fix(baggage): do not percent-encode W3C baggage metadata

api/all/src/main/java/io/opentelemetry/api/baggage/propagation/Parser.java

@@ -147,7 +147,6 @@ private void putBaggage(
     String decodedValue;
     try {
       decodedValue = decodeValue(value);
-      metadataValue = decodeValue(metadataValue);
     } catch (IllegalArgumentException e) {
       LOGGER.log(Level.WARNING, "Skipping invalid baggage member", e);
       return;

api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java

@@ -78,20 +78,18 @@ private static String baggageToString(Baggage baggage) {
           }
           String encodedValue = encodeValue(baggageEntry.getValue());
           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 +111,13 @@ 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.
    */
   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;
   }
@@ -179,7 +176,9 @@ private static int extractEntries(
   }
 
   private static boolean baggageIsInvalid(String key, BaggageEntry baggageEntry) {
-    return !isValidBaggageKey(key) || !isValidBaggageValue(baggageEntry.getValue());
+    return !isValidBaggageKey(key)
+        || !isValidBaggageValue(baggageEntry.getValue())
+        || !isValidBaggageMetadata(baggageEntry.getMetadata().getValue());
   }
 
   /**
@@ -202,6 +201,29 @@ private static boolean isValidBaggageValue(String value) {
     return value != null;
   }
 
+  /**
+   * Determines whether the given {@code String} is valid W3C baggage metadata.
+   *
+   * @param metadata the metadata to be validated.
+   * @return whether the metadata is valid.
+   */
+  private static boolean isValidBaggageMetadata(@Nullable String metadata) {
+    if (metadata == null || metadata.isEmpty()) {
+      return true;
+    }
+    for (int i = 0; i < metadata.length(); i++) {
+      if (!isValidMetadataChar(metadata.charAt(i))) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  // tchar / baggage-octet / OWS / '=' / ';'
+  private static boolean isValidMetadataChar(char c) {
+    return c == '\t' || (c >= ' ' && c <= '~' && c != '"' && c != ',' && c != '\\');
+  }
+
   @Override
   public String toString() {
     return "W3CBaggagePropagator";

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<String, String> 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<String, String> carrier = new HashMap<>();
       baggagePropagator.inject(Context.root().with(baggage), carrier, Map::put);
       Context extractedContext =
@@ -117,6 +117,28 @@ protected boolean codePointInRange(int codePoint) {
     }
   }
 
+  public static class MetadataGenerator extends AbstractStringGenerator {
+
+    @Override
+    protected int nextCodePoint(SourceOfRandomness random) {
+      while (true) {
+        char c = random.nextChar(' ', '~');
+        if (c != '"' && c != ',' && c != '\\') {
+          return c;
+        }
+      }
+    }
+
+    @Override
+    protected boolean codePointInRange(int codePoint) {
+      return codePoint >= ' '
+          && codePoint <= '~'
+          && codePoint != '"'
+          && codePoint != ','
+          && codePoint != '\\';
+    }
+  }
+
   private static class MapTextMapGetter implements TextMapGetter<Map<String, String>> {
     @Override
     public Iterable<String> keys(Map<String, String> carrier) {

api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java

@@ -476,7 +476,11 @@ static Stream<Arguments> 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()));
+            Baggage.builder()
+                .put("key1", "value1")
+                .put("encoded", "value 2")
+                .put("bad3", "value", BaggageEntryMetadata.create("meta=%GG"))
+                .build()));
   }
 
   @Test
@@ -625,7 +629,88 @@ void inject() {
         .containsExactlyInAnyOrderEntriesOf(
             singletonMap(
                 "baggage",
-                "meta=meta-value;somemetadata%3B%20someother%3Dfoo,needsEncoding=blah%20blah%20blah,nometa=nometa-value"));
+                "meta=meta-value;somemetadata; someother=foo,needsEncoding=blah%20blah%20blah,nometa=nometa-value"));
+  }
+
+  @Test
+  void inject_doesNotPercentEncodeMetadata() {
+    Baggage baggage =
+        Baggage.builder()
+            .put("SomeKey", "SomeValue", BaggageEntryMetadata.create("ValueProp \t = \t PropVal"))
+            .build();
+    Map<String, String> carrier = new HashMap<>();
+    W3CBaggagePropagator.getInstance().inject(Context.root().with(baggage), carrier, Map::put);
+    assertThat(carrier)
+        .containsExactlyInAnyOrderEntriesOf(
+            singletonMap("baggage", "SomeKey=SomeValue;ValueProp \t = \t PropVal"));
+  }
+
+  @ParameterizedTest
+  @MethodSource
+  void inject_invalidMetadata_skipsEntry(String metadata) {
+    Baggage baggage =
+        Baggage.builder()
+            .put("keep", "yes")
+            .put("drop", "no", BaggageEntryMetadata.create(metadata))
+            .build();
+    Map<String, String> carrier = new HashMap<>();
+    W3CBaggagePropagator.getInstance().inject(Context.root().with(baggage), carrier, Map::put);
+    assertThat(carrier).containsExactlyInAnyOrderEntriesOf(singletonMap("baggage", "keep=yes"));
+  }
+
+  static Stream<Arguments> inject_invalidMetadata_skipsEntry() {
+    return Stream.of(
+        Arguments.argumentSet("comma", "a,b"),
+        Arguments.argumentSet("dquote", "a\"b"),
+        Arguments.argumentSet("backslash", "a\\b"),
+        Arguments.argumentSet("cr", "a\rb"),
+        Arguments.argumentSet("lf", "a\nb"),
+        Arguments.argumentSet("nul", "a\0b"),
+        Arguments.argumentSet("del", "a\u007fb"),
+        Arguments.argumentSet("non-ascii", "café"),
+        Arguments.argumentSet("obs-text", "a\u0080b"));
+  }
+
+  @Test
+  void inject_invalidMetadataOnly_omitsHeader() {
+    Baggage baggage = Baggage.builder().put("k", "v", BaggageEntryMetadata.create("a,b")).build();
+    Map<String, String> carrier = new HashMap<>();
+    W3CBaggagePropagator.getInstance().inject(Context.root().with(baggage), carrier, Map::put);
+    assertThat(carrier).isEmpty();
+  }
+
+  @Test
+  void extract_metadataNotPercentDecoded() {
+    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<String, String> 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