diff --git a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/Parser.java b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/Parser.java index a96d2aebf7d..3772fe607e5 100644 --- a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/Parser.java +++ b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/Parser.java @@ -146,8 +146,13 @@ private void putBaggage( } String decodedValue; try { + // Only list-member values are percent-decoded. OTel metadata is an opaque string + // (https://opentelemetry.io/docs/specs/otel/baggage/api/#set-value) stored as a single + // instance on extract (https://opentelemetry.io/docs/specs/otel/baggage/api/#propagation); + // do not percent-decode the properties blob. W3C decoding rules apply to list-member + // values and to property *values* only (https://w3c.github.io/baggage/#property), not to + // the entire metadata string as one unit. decodedValue = decodeValue(value); - metadataValue = decodeValue(metadataValue); } catch (IllegalArgumentException e) { LOGGER.log(Level.WARNING, "Skipping invalid baggage member", e); return; diff --git a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java index 2b589315435..e224a9d1cfb 100644 --- a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java +++ b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java @@ -77,21 +77,25 @@ private static String baggageToString(Baggage baggage) { return; } String encodedValue = encodeValue(baggageEntry.getValue()); + // OTel metadata is an opaque string (baggage API Set Value / Propagation): append as-is + // so W3C property structure (keys, '=', OWS) is preserved. Do not percent-encode the + // whole blob - that would break property key-value form (see #6771). W3C requires + // percent-encoding of list-member values and of property *values* only + // (https://w3c.github.io/baggage/#property); callers must supply wire-correct metadata + // because OTel does not parse property structure. String metadataValue = baggageEntry.getMetadata().getValue(); - String encodedMetadata = - (metadataValue != null && !metadataValue.isEmpty()) - ? encodeValue(metadataValue) - : null; + String metadata = + (metadataValue != null && !metadataValue.isEmpty()) ? metadataValue : null; // Exit early if adding this entry causes the total length to exceed the limit // encodedEntryLength includes a trailing comma; the final string trims exactly one, // so the net contribution to the final length is entryLength - 1. - if (headerContent.length() + encodedEntryLength(key, encodedValue, encodedMetadata) - 1 + if (headerContent.length() + encodedEntryLength(key, encodedValue, metadata) - 1 > MAX_BAGGAGE_BYTES) { return; } headerContent.append(key).append("=").append(encodedValue); - if (encodedMetadata != null) { - headerContent.append(";").append(encodedMetadata); + if (metadata != null) { + headerContent.append(";").append(metadata); } headerContent.append(","); entryCount[0]++; @@ -113,14 +117,14 @@ private static String encodeValue(String value) { /** * Returns the length of the serialized entry as it would appear in the baggage header, including * the trailing comma used by the trailing-comma pattern in {@link #baggageToString}. The length - * accounts for {@code "key=encodedValue,"} plus {@code ";encodedMetadata"} when metadata is - * present. + * accounts for {@code "key=encodedValue,"} plus {@code ";metadata"} when metadata is present. + * Metadata is treated as an opaque properties blob and is not percent-encoded as a unit. */ private static int encodedEntryLength( - String key, String encodedValue, @Nullable String encodedMetadata) { + String key, String encodedValue, @Nullable String metadata) { int length = key.length() + 1 + encodedValue.length() + 1; // "key=value," - if (encodedMetadata != null) { - length += 1 + encodedMetadata.length(); // ";metadata" + if (metadata != null) { + length += 1 + metadata.length(); // ";metadata" } return length; } diff --git a/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorFuzzTest.java b/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorFuzzTest.java index 0a95e27e83c..2a87b88a05f 100644 --- a/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorFuzzTest.java +++ b/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorFuzzTest.java @@ -36,11 +36,12 @@ public static class TestCases { private final W3CBaggagePropagator baggagePropagator = W3CBaggagePropagator.getInstance(); @Fuzz - public void roundTripRandomValues(String baggageValue, String metadataBlob) { + public void roundTripRandomValues( + String baggageValue, @From(MetadataGenerator.class) String metadataBlob) { + // Extract trims OWS around the metadata blob; match that so round-trip compares equal. + String metadata = metadataBlob.trim(); Baggage baggage = - Baggage.builder() - .put("b", baggageValue, BaggageEntryMetadata.create(metadataBlob)) - .build(); + Baggage.builder().put("b", baggageValue, BaggageEntryMetadata.create(metadata)).build(); Map carrier = new HashMap<>(); baggagePropagator.inject(Context.root().with(baggage), carrier, Map::put); Context extractedContext = @@ -53,11 +54,10 @@ public void roundTripRandomValues(String baggageValue, String metadataBlob) { @Fuzz public void roundTripAsciiValues( @From(AsciiGenerator.class) String baggageValue, - @From(AsciiGenerator.class) String metadataBlob) { + @From(MetadataGenerator.class) String metadataBlob) { + String metadata = metadataBlob.trim(); Baggage baggage = - Baggage.builder() - .put("b", baggageValue, BaggageEntryMetadata.create(metadataBlob)) - .build(); + Baggage.builder().put("b", baggageValue, BaggageEntryMetadata.create(metadata)).build(); Map carrier = new HashMap<>(); baggagePropagator.inject(Context.root().with(baggage), carrier, Map::put); Context extractedContext = @@ -117,6 +117,28 @@ protected boolean codePointInRange(int codePoint) { } } + /** + * Generates opaque metadata that is safe to round-trip without percent-encoding. Excludes {@code + * ','} which is the W3C list-member separator and would split the header on extract. + */ + public static class MetadataGenerator extends AbstractStringGenerator { + + @Override + protected int nextCodePoint(SourceOfRandomness random) { + while (true) { + char c = random.nextChar(' ', '~'); + if (c != ',') { + return c; + } + } + } + + @Override + protected boolean codePointInRange(int codePoint) { + return codePoint >= ' ' && codePoint <= '~' && codePoint != ','; + } + } + private static class MapTextMapGetter implements TextMapGetter> { @Override public Iterable keys(Map carrier) { diff --git a/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java b/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java index 00847063952..373636c4215 100644 --- a/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java +++ b/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java @@ -476,7 +476,12 @@ static Stream extract_member_invalidPercentEncoding_preservesValidMem Arguments.argumentSet( "multiple invalid entries", "bad1=va%lue,key1=value1,bad2=value%GG,encoded=value%202,bad3=value;meta=%GG", - Baggage.builder().put("key1", "value1").put("encoded", "value 2").build())); + // metadata is not percent-decoded, so "meta=%GG" is kept as an opaque string + Baggage.builder() + .put("key1", "value1") + .put("encoded", "value 2") + .put("bad3", "value", BaggageEntryMetadata.create("meta=%GG")) + .build())); } @Test @@ -625,7 +630,57 @@ void inject() { .containsExactlyInAnyOrderEntriesOf( singletonMap( "baggage", - "meta=meta-value;somemetadata%3B%20someother%3Dfoo,needsEncoding=blah%20blah%20blah,nometa=nometa-value")); + // Values are percent-encoded; metadata is left opaque (not percent-encoded). + "meta=meta-value;somemetadata; someother=foo,needsEncoding=blah%20blah%20blah,nometa=nometa-value")); + } + + @Test + void inject_doesNotPercentEncodeMetadata() { + // Regression for #6771: W3C property key-value shape (spaces, '=', tabs) must survive inject. + Baggage baggage = + Baggage.builder() + .put("SomeKey", "SomeValue", BaggageEntryMetadata.create("ValueProp \t = \t PropVal")) + .build(); + Map carrier = new HashMap<>(); + W3CBaggagePropagator.getInstance().inject(Context.root().with(baggage), carrier, Map::put); + assertThat(carrier) + .containsExactlyInAnyOrderEntriesOf( + singletonMap("baggage", "SomeKey=SomeValue;ValueProp \t = \t PropVal")); + } + + @Test + void extract_metadataNotPercentDecoded() { + // Metadata containing percent sequences must be kept as-is (opaque string). + W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance(); + Context result = + propagator.extract( + Context.root(), + ImmutableMap.of("baggage", "SomeKey=SomeValue;ValueProp%20%09%20%3D%20%09%20PropVal"), + getter); + + assertThat(Baggage.fromContext(result)) + .isEqualTo( + Baggage.builder() + .put( + "SomeKey", + "SomeValue", + BaggageEntryMetadata.create("ValueProp%20%09%20%3D%20%09%20PropVal")) + .build()); + } + + @Test + void roundTrip_metadataPreservedOpaque() { + W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance(); + Baggage baggage = + Baggage.builder() + .put("SomeKey", "SomeValue", BaggageEntryMetadata.create("ValueProp \t = \t PropVal")) + .build(); + Map carrier = new HashMap<>(); + propagator.inject(baggage.storeInContext(Context.root()), carrier, Map::put); + assertThat(carrier.get("baggage")).isEqualTo("SomeKey=SomeValue;ValueProp \t = \t PropVal"); + + Baggage extracted = Baggage.fromContext(propagator.extract(Context.root(), carrier, getter)); + assertThat(extracted).isEqualTo(baggage); } @Test