From a9063057758e2df8ae5e4a50b118c628717f71a5 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Tue, 10 Oct 2023 22:18:24 +0200 Subject: [PATCH] Escape/Unescape JMS String properties in contexts 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 gh-4202 --- .../jms/JmsProcessObservationContext.java | 11 +++- .../jms/JmsPublishObservationContext.java | 11 +++- .../JmsProcessObservationContextTests.java | 56 +++++++++++++++++++ .../JmsPublishObservationContextTests.java | 55 ++++++++++++++++++ 4 files changed, 129 insertions(+), 4 deletions(-) create mode 100644 micrometer-jakarta9/src/test/java/io/micrometer/jakarta9/instrument/jms/JmsProcessObservationContextTests.java create mode 100644 micrometer-jakarta9/src/test/java/io/micrometer/jakarta9/instrument/jms/JmsPublishObservationContextTests.java diff --git a/micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/jms/JmsProcessObservationContext.java b/micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/jms/JmsProcessObservationContext.java index 4d8d29a892..4353699ccc 100644 --- a/micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/jms/JmsProcessObservationContext.java +++ b/micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/jms/JmsProcessObservationContext.java @@ -26,7 +26,10 @@ * messages}. *

* 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 @@ -36,7 +39,7 @@ public class JmsProcessObservationContext extends ReceiverContext { public JmsProcessObservationContext(Message receivedMessage) { super((message, key) -> { try { - return message.getStringProperty(key); + return message.getStringProperty(escapeKey(key)); } catch (JMSException exc) { return null; @@ -45,4 +48,8 @@ public JmsProcessObservationContext(Message receivedMessage) { setCarrier(receivedMessage); } + private static String escapeKey(String key) { + return key.replace("-", "_HYPHEN_").replace(".", "_DOT_"); + } + } diff --git a/micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/jms/JmsPublishObservationContext.java b/micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/jms/JmsPublishObservationContext.java index f16d2154d9..b73a447b6b 100644 --- a/micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/jms/JmsPublishObservationContext.java +++ b/micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/jms/JmsPublishObservationContext.java @@ -26,7 +26,10 @@ * {@link JmsObservationDocumentation#JMS_MESSAGE_PUBLISH publication of JMS messages}. *

* 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 @@ -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) { @@ -47,4 +50,8 @@ public JmsPublishObservationContext(@Nullable Message sendMessage) { setCarrier(sendMessage); } + private static String escapeKey(String key) { + return key.replace("-", "_HYPHEN_").replace(".", "_DOT_"); + } + } diff --git a/micrometer-jakarta9/src/test/java/io/micrometer/jakarta9/instrument/jms/JmsProcessObservationContextTests.java b/micrometer-jakarta9/src/test/java/io/micrometer/jakarta9/instrument/jms/JmsProcessObservationContextTests.java new file mode 100644 index 0000000000..9c85f6b1ab --- /dev/null +++ b/micrometer-jakarta9/src/test/java/io/micrometer/jakarta9/instrument/jms/JmsProcessObservationContextTests.java @@ -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 argumentCaptor = ArgumentCaptor.forClass(String.class); + verify(sendMessage).getStringProperty(argumentCaptor.capture()); + + assertThat(argumentCaptor.getValue()).isEqualTo(escaped); + } + + static Stream headerValues() { + return Stream.of(Arguments.of("valid", "valid"), Arguments.of("with-hyphen", "with_HYPHEN_hyphen"), + Arguments.of("with.dot", "with_DOT_dot")); + } + +} diff --git a/micrometer-jakarta9/src/test/java/io/micrometer/jakarta9/instrument/jms/JmsPublishObservationContextTests.java b/micrometer-jakarta9/src/test/java/io/micrometer/jakarta9/instrument/jms/JmsPublishObservationContextTests.java new file mode 100644 index 0000000000..b617e3bba8 --- /dev/null +++ b/micrometer-jakarta9/src/test/java/io/micrometer/jakarta9/instrument/jms/JmsPublishObservationContextTests.java @@ -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 argumentCaptor = ArgumentCaptor.forClass(String.class); + verify(sendMessage).setStringProperty(argumentCaptor.capture(), anyString()); + + assertThat(argumentCaptor.getValue()).isEqualTo(escaped); + } + + static Stream headerValues() { + return Stream.of(Arguments.of("valid", "valid"), Arguments.of("with-hyphen", "with_HYPHEN_hyphen"), + Arguments.of("with.dot", "with_DOT_dot")); + } + +}