diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/propagation/EnvironmentGetter.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/propagation/EnvironmentGetter.java index 457ce198a9a..33179389d12 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/propagation/EnvironmentGetter.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/propagation/EnvironmentGetter.java @@ -9,8 +9,8 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Locale; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -46,7 +46,8 @@ */ public final class EnvironmentGetter implements TextMapGetter> { - private static final Logger logger = Logger.getLogger(EnvironmentGetter.class.getName()); + private static final AtomicBoolean LOG_KEYS_CALLED = new AtomicBoolean(false); + private static final Logger LOGGER = Logger.getLogger(EnvironmentGetter.class.getName()); private static final EnvironmentGetter INSTANCE = new EnvironmentGetter(); private EnvironmentGetter() {} @@ -58,14 +59,21 @@ public static EnvironmentGetter getInstance() { @Override public Iterable keys(Map carrier) { + if (LOG_KEYS_CALLED.compareAndSet(false, true)) { + LOGGER.log( + Level.WARNING, + "keys() called on EnvironmentGetter. " + + "This may produce unexpected results with propagators which depend on case sensitivity or special characters in keys.", + new Throwable()); + } if (carrier == null) { return Collections.emptyList(); } List result = new ArrayList<>(carrier.size()); for (String key : carrier.keySet()) { - result.add(key.toLowerCase(Locale.ROOT)); + result.add(EnvironmentSetter.normalizeKey(key)); } - return result; + return Collections.unmodifiableList(result); } @Nullable @@ -74,20 +82,15 @@ public String get(@Nullable Map carrier, String key) { if (carrier == null || key == null) { return null; } - // Spec recommends using uppercase and underscores for environment variable - // names for maximum - // cross-platform compatibility. - String sanitizedKey = key.replace('.', '_').replace('-', '_').toUpperCase(Locale.ROOT); - String value = carrier.get(sanitizedKey); - if (value != null && !EnvironmentSetter.isValidHttpHeaderValue(value)) { - logger.log( - Level.FINE, - "Ignoring environment variable '{0}': " - + "value contains characters not valid in HTTP header fields per RFC 9110.", - sanitizedKey); - return null; - } - return value; + String normalizedKey = EnvironmentSetter.normalizeKey(key); + String[] result = new String[] {null}; + carrier.forEach( + (entryKey, entryValue) -> { + if (EnvironmentSetter.normalizeKey(entryKey).equals(normalizedKey)) { + result[0] = entryValue; + } + }); + return result[0]; } @Override diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/propagation/EnvironmentSetter.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/propagation/EnvironmentSetter.java index a4dd5974751..bb754b3f616 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/propagation/EnvironmentSetter.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/propagation/EnvironmentSetter.java @@ -6,10 +6,7 @@ package io.opentelemetry.api.incubator.propagation; import io.opentelemetry.context.propagation.TextMapSetter; -import java.util.Locale; import java.util.Map; -import java.util.logging.Level; -import java.util.logging.Logger; import javax.annotation.Nullable; /** @@ -30,8 +27,10 @@ * conventions: * *
    - *
  • Converts keys to uppercase (e.g., {@code traceparent} becomes {@code TRACEPARENT}) - *
  • Replaces {@code .} and {@code -} with underscores + *
  • ASCII letters are converted to uppercase + *
  • Any character that is not an ASCII letter, digit, or underscore is replaced with an + * underscore + *
  • If the result would start with a digit, an underscore is prepended *
* *

Values are validated to contain only characters valid in HTTP header fields per > { - private static final Logger logger = Logger.getLogger(EnvironmentSetter.class.getName()); private static final EnvironmentSetter INSTANCE = new EnvironmentSetter(); private EnvironmentSetter() {} @@ -64,35 +62,36 @@ public void set(@Nullable Map carrier, String key, String value) if (carrier == null || key == null || value == null) { return; } - if (!isValidHttpHeaderValue(value)) { - logger.log( - Level.FINE, - "Skipping environment variable injection for key ''{0}'': " - + "value contains characters not valid in HTTP header fields per RFC 9110.", - key); - return; - } - // Spec recommends using uppercase and underscores for environment variable - // names for maximum - // cross-platform compatibility. - String sanitizedKey = key.replace('.', '_').replace('-', '_').toUpperCase(Locale.ROOT); - carrier.put(sanitizedKey, value); + String normalizedKey = normalizeKey(key); + carrier.put(normalizedKey, value); } /** - * Checks whether a string contains only characters valid in HTTP header field values per RFC 9110 Section 5.5. - * Valid characters are: visible ASCII (0x21-0x7E), space (0x20), and horizontal tab (0x09). + * Normalizes a key to be a valid environment variable name. + * + *

    + *
  • ASCII letters are converted to uppercase + *
  • Any character that is not an ASCII letter, digit, or underscore is replaced with an + * underscore (including {@code .}, {@code -}, whitespace, and control characters) + *
  • If the result would start with a digit, an underscore is prepended + *
*/ - static boolean isValidHttpHeaderValue(String value) { - for (int i = 0; i < value.length(); i++) { - char ch = value.charAt(i); - // VCHAR (0x21-0x7E), SP (0x20), HTAB (0x09) - if (ch != '\t' && (ch < ' ' || ch > '~')) { - return false; + static String normalizeKey(String key) { + StringBuilder sb = new StringBuilder(key.length() + 1); + for (int i = 0; i < key.length(); i++) { + char ch = key.charAt(i); + if (ch >= 'a' && ch <= 'z') { + sb.append((char) (ch - 32)); + } else if ((ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') || ch == '_') { + sb.append(ch); + } else { + sb.append('_'); } } - return true; + if (sb.length() > 0 && sb.charAt(0) >= '0' && sb.charAt(0) <= '9') { + sb.insert(0, '_'); + } + return sb.toString(); } @Override diff --git a/api/incubator/src/test/java/io/opentelemetry/api/incubator/propagation/EnvironmentGetterTest.java b/api/incubator/src/test/java/io/opentelemetry/api/incubator/propagation/EnvironmentGetterTest.java index 6d9b1786548..91fed7a6943 100644 --- a/api/incubator/src/test/java/io/opentelemetry/api/incubator/propagation/EnvironmentGetterTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/api/incubator/propagation/EnvironmentGetterTest.java @@ -7,13 +7,19 @@ import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; +import io.github.netmikey.logunit.api.LogCapturer; +import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import java.util.Collections; import java.util.HashMap; import java.util.Map; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; class EnvironmentGetterTest { + @RegisterExtension + LogCapturer logCapturer = LogCapturer.create().captureForType(EnvironmentGetter.class); + @Test void get() { Map carrier = new HashMap<>(); @@ -29,10 +35,10 @@ void get() { } @Test - void get_sanitization() { + void get_normalization() { Map carrier = new HashMap<>(); carrier.put("OTEL_TRACE_ID", "val1"); - carrier.put("OTEL_BAGGAGE_KEY", "val2"); + carrier.put("otel-baggage-key", "val2"); assertThat(EnvironmentGetter.getInstance().get(carrier, "otel.trace.id")).isEqualTo("val1"); assertThat(EnvironmentGetter.getInstance().get(carrier, "otel-baggage-key")).isEqualTo("val2"); @@ -45,37 +51,45 @@ void get_null() { } @Test - void keys() { + @SuppressLogger(EnvironmentGetter.class) + void keys_valuesAreNormalized() { Map carrier = new HashMap<>(); - carrier.put("K1", "V1"); - carrier.put("K2", "V2"); - - assertThat(EnvironmentGetter.getInstance().keys(carrier)).containsExactlyInAnyOrder("k1", "k2"); + carrier.put("otel.trace.id", "V1"); + carrier.put("otel-baggage-key", "V2"); + carrier.put("OTEL_FOO", "V2"); + + // For a carrier containing keys that are both normalized and not normalized, verify all results + // from keys() return values for get. + assertThat(EnvironmentGetter.getInstance().keys(carrier)) + .containsExactlyInAnyOrder("OTEL_TRACE_ID", "OTEL_BAGGAGE_KEY", "OTEL_FOO"); + for (String key : EnvironmentGetter.getInstance().keys(carrier)) { + assertThat(EnvironmentGetter.getInstance().get(carrier, key)).isNotNull(); + } assertThat(EnvironmentGetter.getInstance().keys(null)).isEmpty(); + + assertThat(logCapturer.size()).isEqualTo(1); + logCapturer.assertContains("keys() called on EnvironmentGetter"); } @Test - void get_validHeaderValues() { + void get_valuesAreUnmodified() { Map carrier = new HashMap<>(); carrier.put("KEY1", "simple-value"); carrier.put("KEY2", "value with spaces"); carrier.put("KEY3", "value\twith\ttabs"); + carrier.put("KEY4", "value\u0000with\u0001control"); + carrier.put("KEY5", "value\nwith\nnewlines"); + carrier.put("KEY6", "value\u0080non-ascii"); assertThat(EnvironmentGetter.getInstance().get(carrier, "key1")).isEqualTo("simple-value"); assertThat(EnvironmentGetter.getInstance().get(carrier, "key2")).isEqualTo("value with spaces"); assertThat(EnvironmentGetter.getInstance().get(carrier, "key3")).isEqualTo("value\twith\ttabs"); - } - - @Test - void get_invalidHeaderValues() { - Map carrier = new HashMap<>(); - carrier.put("KEY1", "value\u0000with\u0001control"); - carrier.put("KEY2", "value\nwith\nnewlines"); - carrier.put("KEY3", "value\u0080non-ascii"); - - assertThat(EnvironmentGetter.getInstance().get(carrier, "key1")).isNull(); - assertThat(EnvironmentGetter.getInstance().get(carrier, "key2")).isNull(); - assertThat(EnvironmentGetter.getInstance().get(carrier, "key3")).isNull(); + assertThat(EnvironmentGetter.getInstance().get(carrier, "key4")) + .isEqualTo("value\u0000with\u0001control"); + assertThat(EnvironmentGetter.getInstance().get(carrier, "key5")) + .isEqualTo("value\nwith\nnewlines"); + assertThat(EnvironmentGetter.getInstance().get(carrier, "key6")) + .isEqualTo("value\u0080non-ascii"); } @Test diff --git a/api/incubator/src/test/java/io/opentelemetry/api/incubator/propagation/EnvironmentSetterTest.java b/api/incubator/src/test/java/io/opentelemetry/api/incubator/propagation/EnvironmentSetterTest.java index de166401af6..c401c02d28b 100644 --- a/api/incubator/src/test/java/io/opentelemetry/api/incubator/propagation/EnvironmentSetterTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/api/incubator/propagation/EnvironmentSetterTest.java @@ -45,28 +45,21 @@ void set_null() { } @Test - void set_validHeaderValues() { + void set_valuesAreUnmodified() { Map carrier = new HashMap<>(); - // Printable ASCII and tab are valid per RFC 9110 EnvironmentSetter.getInstance().set(carrier, "key1", "simple-value"); EnvironmentSetter.getInstance().set(carrier, "key2", "value with spaces"); EnvironmentSetter.getInstance().set(carrier, "key3", "value\twith\ttabs"); + EnvironmentSetter.getInstance().set(carrier, "key4", "value\u0000with\u0001control"); + EnvironmentSetter.getInstance().set(carrier, "key5", "value\nwith\nnewlines"); + EnvironmentSetter.getInstance().set(carrier, "key6", "value\u0080non-ascii"); assertThat(carrier).containsEntry("KEY1", "simple-value"); assertThat(carrier).containsEntry("KEY2", "value with spaces"); assertThat(carrier).containsEntry("KEY3", "value\twith\ttabs"); - } - - @Test - void set_invalidHeaderValues() { - Map carrier = new HashMap<>(); - // Control characters and non-ASCII are invalid per RFC 9110 - EnvironmentSetter.getInstance().set(carrier, "key1", "value\u0000with\u0001control"); - EnvironmentSetter.getInstance().set(carrier, "key2", "value\nwith\nnewlines"); - EnvironmentSetter.getInstance().set(carrier, "key3", "value\rwith\rcarriage"); - EnvironmentSetter.getInstance().set(carrier, "key4", "value\u0080non-ascii"); - - assertThat(carrier).isEmpty(); + assertThat(carrier).containsEntry("KEY4", "value\u0000with\u0001control"); + assertThat(carrier).containsEntry("KEY5", "value\nwith\nnewlines"); + assertThat(carrier).containsEntry("KEY6", "value\u0080non-ascii"); } @Test