diff --git a/dd-trace-core/src/main/java/datadog/trace/core/util/PercentEscaper.java b/dd-trace-core/src/main/java/datadog/trace/core/util/PercentEscaper.java index cfe1f02b4a9..0d13be6ce32 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/util/PercentEscaper.java +++ b/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 diff --git a/dd-trace-core/src/test/java/datadog/trace/core/util/PercentEscaperTest.java b/dd-trace-core/src/test/java/datadog/trace/core/util/PercentEscaperTest.java new file mode 100644 index 00000000000..54114ab561f --- /dev/null +++ b/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); + } +}