Skip to content

Commit

Permalink
Escape/Unescape JMS String properties in contexts
Browse files Browse the repository at this point in the history
Prior to this commit, both `JmsPublishObservationContext` and
`JmsProcessObservationContext` would set propagation headers as String
properties on the JMS `Message`.
This would cause issues with JMS implementations, as the JMS
specification only allows valid Java identifiers as keys. Typically, a
`X-B3-TraceId` header fails this requirement as it contains "-"
characters.

This commit ensures that the following characters are escaped when
publishing messages, and unescaped when receiving messages:

* "-" becomes "_HYPHEN_"
* "." becomes "_DOT_"

Closes micrometer-metricsgh-4202
  • Loading branch information
bclozel committed Oct 11, 2023
1 parent 1f2b1de commit a906305
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@
* messages}.
* <p>
* The inbound tracing information is propagated by looking it up in
* {@link Message#getStringProperty(String) incoming JMS message headers}.
* {@link Message#getStringProperty(String) incoming JMS message headers}. As the JMS spec
* only allows valid Java identifiers as String properties, we must escape "-" and "."
* characters, assuming that they are escaped as {@code "_HYPHEN_"} and {@code "_DOT_"} on
* the sending side.
*
* @author Brian Clozel
* @since 1.12.0
Expand All @@ -36,7 +39,7 @@ public class JmsProcessObservationContext extends ReceiverContext<Message> {
public JmsProcessObservationContext(Message receivedMessage) {
super((message, key) -> {
try {
return message.getStringProperty(key);
return message.getStringProperty(escapeKey(key));
}
catch (JMSException exc) {
return null;
Expand All @@ -45,4 +48,8 @@ public JmsProcessObservationContext(Message receivedMessage) {
setCarrier(receivedMessage);
}

private static String escapeKey(String key) {
return key.replace("-", "_HYPHEN_").replace(".", "_DOT_");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@
* {@link JmsObservationDocumentation#JMS_MESSAGE_PUBLISH publication of JMS messages}.
* <p>
* This propagates the tracing information with the message sent by
* {@link Message#setStringProperty(String, String) setting a message header}.
* {@link Message#setStringProperty(String, String) setting a message header}. As the JMS
* spec only allows valid Java identifiers as String properties, we must escape "-" and
* "." characters as {@code "_HYPHEN_"} and {@code "_DOT_"}, assuming that they are
* escaped on the receiving side.
*
* @author Brian Clozel
* @since 1.12.0
Expand All @@ -37,7 +40,7 @@ public JmsPublishObservationContext(@Nullable Message sendMessage) {
super((message, key, value) -> {
try {
if (message != null) {
message.setStringProperty(key, value);
message.setStringProperty(escapeKey(key), value);
}
}
catch (JMSException exc) {
Expand All @@ -47,4 +50,8 @@ public JmsPublishObservationContext(@Nullable Message sendMessage) {
setCarrier(sendMessage);
}

private static String escapeKey(String key) {
return key.replace("-", "_HYPHEN_").replace(".", "_DOT_");
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.micrometer.jakarta9.instrument.jms;

import jakarta.jms.Message;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.ArgumentCaptor;

import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;

/**
* Tests for {@link JmsProcessObservationContext}.
*
* @author Brian Clozel
*/
class JmsProcessObservationContextTests {

@ParameterizedTest
@MethodSource("headerValues")
void propagatorShouldEscapeHeader(String value, String escaped) throws Exception {

Message sendMessage = mock(Message.class);
JmsProcessObservationContext context = new JmsProcessObservationContext(sendMessage);
context.getGetter().get(sendMessage, value);

ArgumentCaptor<String> argumentCaptor = ArgumentCaptor.forClass(String.class);
verify(sendMessage).getStringProperty(argumentCaptor.capture());

assertThat(argumentCaptor.getValue()).isEqualTo(escaped);
}

static Stream<Arguments> headerValues() {
return Stream.of(Arguments.of("valid", "valid"), Arguments.of("with-hyphen", "with_HYPHEN_hyphen"),
Arguments.of("with.dot", "with_DOT_dot"));
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright 2002-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.micrometer.jakarta9.instrument.jms;

import jakarta.jms.Message;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.ArgumentCaptor;

import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.*;

/**
* Tests for {@link JmsPublishObservationContext}.
*
* @author Brian Clozel
*/
class JmsPublishObservationContextTests {

@ParameterizedTest
@MethodSource("headerValues")
void propagatorShouldEscapeHeader(String value, String escaped) throws Exception {

Message sendMessage = mock(Message.class);
JmsPublishObservationContext context = new JmsPublishObservationContext(sendMessage);
context.getSetter().set(sendMessage, value, "testValue");

ArgumentCaptor<String> argumentCaptor = ArgumentCaptor.forClass(String.class);
verify(sendMessage).setStringProperty(argumentCaptor.capture(), anyString());

assertThat(argumentCaptor.getValue()).isEqualTo(escaped);
}

static Stream<Arguments> headerValues() {
return Stream.of(Arguments.of("valid", "valid"), Arguments.of("with-hyphen", "with_HYPHEN_hyphen"),
Arguments.of("with.dot", "with_DOT_dot"));
}

}

0 comments on commit a906305

Please sign in to comment.