From fb8a0485b8353aabe3ce9ed601f8ca8ac9f93075 Mon Sep 17 00:00:00 2001 From: arimu1 <19286898+arimu1@users.noreply.github.com> Date: Sat, 1 Aug 2026 11:09:09 +0700 Subject: [PATCH 1/2] fix(baggage): do not percent-encode or decode W3C baggage metadata Neither the W3C Baggage spec nor the OTel baggage API require percent encoding of metadata (W3C properties). Encoding breaks property key-value shape for other implementations on extract/inject round-trips. Only baggage entry values remain percent-encoded/decoded. Fixes #6771 --- .../api/baggage/propagation/Parser.java | 3 +- .../propagation/W3CBaggagePropagator.java | 25 ++++---- .../W3CBaggagePropagatorFuzzTest.java | 38 +++++++++--- .../propagation/W3CBaggagePropagatorTest.java | 59 ++++++++++++++++++- 4 files changed, 102 insertions(+), 23 deletions(-) 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..3690df2c07c 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,9 @@ private void putBaggage( } String decodedValue; try { + // Only baggage entry values are percent-decoded. Metadata is an opaque string and must not + // be percent-decoded (W3C baggage properties / OTel metadata). 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..6cf7c988911 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,22 @@ private static String baggageToString(Baggage baggage) { return; } String encodedValue = encodeValue(baggageEntry.getValue()); + // Metadata is an opaque string (OTel baggage API) / W3C properties blob. Neither the + // W3C baggage nor OTel specs require percent-encoding metadata; leave it untouched so + // property structure (e.g. "name = value") is preserved for other implementations. 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 +114,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 not percent-encoded. */ 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 From 3a146dddb471a1b8344313334856824c0e5181ec Mon Sep 17 00:00:00 2001 From: arimu1 <19286898+arimu1@users.noreply.github.com> Date: Thu, 6 Aug 2026 07:57:32 +0700 Subject: [PATCH 2/2] docs(baggage): clarify W3C property-value vs opaque metadata encoding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review: cite OTel opaque metadata + Propagation links, and note that W3C percent-encoding applies to list-member values and property *values* only — not to the entire OTel metadata blob as one unit. --- .../opentelemetry/api/baggage/propagation/Parser.java | 8 ++++++-- .../api/baggage/propagation/W3CBaggagePropagator.java | 11 +++++++---- 2 files changed, 13 insertions(+), 6 deletions(-) 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 3690df2c07c..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,12 @@ private void putBaggage( } String decodedValue; try { - // Only baggage entry values are percent-decoded. Metadata is an opaque string and must not - // be percent-decoded (W3C baggage properties / OTel metadata). + // 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); } catch (IllegalArgumentException e) { LOGGER.log(Level.WARNING, "Skipping invalid baggage member", e); 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 6cf7c988911..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,9 +77,12 @@ private static String baggageToString(Baggage baggage) { return; } String encodedValue = encodeValue(baggageEntry.getValue()); - // Metadata is an opaque string (OTel baggage API) / W3C properties blob. Neither the - // W3C baggage nor OTel specs require percent-encoding metadata; leave it untouched so - // property structure (e.g. "name = value") is preserved for other implementations. + // 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 metadata = (metadataValue != null && !metadataValue.isEmpty()) ? metadataValue : null; @@ -115,7 +118,7 @@ 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 ";metadata"} when metadata is present. - * Metadata is not percent-encoded. + * 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 metadata) {