Fix baggage header escaping of non-ASCII values

dd-trace-core/src/main/java/datadog/trace/core/util/PercentEscaper.java

@@ -70,7 +70,7 @@ public final class PercentEscaper {
   private static final int DEST_PAD = 32;
 
   private static final String UNSAFE_CHARACTERS_KEY = "\",;\\()/:<=>?@[]{} ";
-  private static final String UNSAFE_CHARACTERS_VALUE = "\",;\\ ";
+  private static final String UNSAFE_CHARACTERS_VALUE = "\",;\\ %";
 
   // Percent escapers output upper case hex digits (uri escapers require this).
   private static final char[] UPPER_HEX_DIGITS = "0123456789ABCDEF".toCharArray();
@@ -115,7 +115,7 @@ public Escaped escapeValue(String s) {
     return escape(s, unsafeValOctets);
   }
 
-  private boolean needsEncoding(char c, boolean[] unsafeOctets) {
+  private static boolean needsEncoding(char c, boolean[] unsafeOctets) {
     if (c > '~' || c <= ' ' || c < unsafeOctets.length && unsafeOctets[c]) {
       return true;
     }
@@ -236,8 +236,7 @@ private static Escaped escapeSlow(String s, int index, boolean[] unsafeOctets) {
 
   private static int nextEscapeIndex(CharSequence csq, int index, int end, boolean[] unsafeOctets) {
     for (; index < end; index++) {
-      char c = csq.charAt(index);
-      if (c < unsafeOctets.length && unsafeOctets[c]) {
+      if (needsEncoding(csq.charAt(index), unsafeOctets)) {
         break;
       }
     }
@@ -250,7 +249,7 @@ private static int nextEscapeIndex(CharSequence csq, int index, int end, boolean
   private static char[] escape(int cp, Escaped escaped, boolean[] unsafeOctets) {
     // We should never get negative values here but if we do it will throw an
     // IndexOutOfBoundsException, so at least it will get spotted.
-    if (cp < unsafeOctets.length && !unsafeOctets[cp]) {
+    if (cp <= '~' && !needsEncoding((char) cp, unsafeOctets)) {
       return null;
     } else if (cp <= 0x7F) {
       // Single byte UTF-8 characters

dd-trace-core/src/test/java/datadog/trace/core/util/PercentEscaperTest.java

@@ -0,0 +1,74 @@
+package datadog.trace.core.util;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.UnsupportedEncodingException;
+import java.net.URLDecoder;
+import org.junit.jupiter.api.Test;
+import org.tabletest.junit.TableTest;
+
+class PercentEscaperTest {
+
+  private final PercentEscaper escaper = PercentEscaper.create();
+
+  @TableTest({
+    "scenario                        | value        | expected                       ",
+    "safe ascii is untouched         | 'plainvalue' | 'plainvalue'                   ",
+    "unsafe ascii is escaped         | 'a,b'        | 'a%2Cb'                        ",
+    "non-ascii at the start          | '你好'       | '%E4%BD%A0%E5%A5%BD'           ",
+    "non-ascii after an escaped char | 'a,b中'      | 'a%2Cb%E4%B8%AD'               ",
+    "non-ascii interleaved with safe | '中a中a中'   | '%E4%B8%ADa%E4%B8%ADa%E4%B8%AD'",
+    "two byte non-ascii              | 'café crème' | 'caf%C3%A9%20cr%C3%A8me'       ",
+    "four byte supplementary         | 'a,b😀'      | 'a%2Cb%F0%9F%98%80'            ",
+    "percent is escaped              | '50%'        | '50%25'                        ",
+    "percent-like sequence preserved | 'a%20b'      | 'a%2520b'                      "
+  })
+  void escapeValueEncodesEveryNonAsciiCharacter(String value, String expected) {
+    assertEquals(expected, this.escaper.escapeValue(value).data);
+  }
+
+  /** Keys use a separate, longer octet table than values, so cover that path too. */
+  @TableTest({
+    "scenario                        | key     | expected            ",
+    "non-ascii at the start          | '你好'  | '%E4%BD%A0%E5%A5%BD'",
+    "non-ascii after an escaped char | 'a/b中' | 'a%2Fb%E4%B8%AD'    "
+  })
+  void escapeKeyEncodesEveryNonAsciiCharacter(String key, String expected) {
+    assertEquals(expected, this.escaper.escapeKey(key).data);
+  }
+
+  /**
+   * Independently verifies the escaped bytes are correct, rather than relying on the hand-written
+   * expectations above, and that nothing non-ASCII reaches the header.
+   */
+  @TableTest({
+    "scenario                | value       ",
+    "three byte non-ascii    | '你好世界'  ",
+    "two byte non-ascii      | 'café crème'",
+    "four byte supplementary | '😀🎉'      ",
+    "percent sign            | 'a%20b%'    "
+  })
+  void escapedValueIsAsciiOnlyAndDecodesBackToTheInput(String value)
+      throws UnsupportedEncodingException {
+    String escaped = this.escaper.escapeValue(value).data;
+    for (int i = 0; i < escaped.length(); i++) {
+      char c = escaped.charAt(i);
+      assertTrue(c <= '~', "expected pure ASCII output but got '" + c + "' in " + escaped);
+    }
+    assertEquals(value, URLDecoder.decode(escaped, "UTF-8"));
+  }
+
+  @Test
+  void escapeValueEncodesControlCharacters() {
+    assertEquals("a%0Ab", this.escaper.escapeValue("a\nb").data);
+    assertEquals("a%09b", this.escaper.escapeValue("a\tb").data);
+    assertEquals("a%0D%0Ab", this.escaper.escapeValue("a\r\nb").data);
+    assertEquals("a%2Cb%0A", this.escaper.escapeValue("a,b\n").data);
+  }
+
+  @Test
+  void escapeKeyEncodesControlCharacters() {
+    assertEquals("a%0Ab", this.escaper.escapeKey("a\nb").data);
+  }
+}