diff --git a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/BaggageCodec.java b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/BaggageCodec.java new file mode 100644 index 0000000000..ab2a66becc --- /dev/null +++ b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/BaggageCodec.java @@ -0,0 +1,88 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.api.baggage.propagation; + +import java.io.ByteArrayOutputStream; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; + +/** + * Note: This class is based on code from Apache Commons Codec. It is comprised of code from these + * classes: + * + * + * + *

Implements baggage-octet decoding in accordance with th Baggage header content specification. All + * US-ASCII characters excluding CTLs, whitespace, DQUOTE, comma, semicolon and backslash are + * encoded in `www-form-urlencoded` encoding scheme. + */ +class BaggageCodec { + + private static final byte ESCAPE_CHAR = '%'; + private static final int RADIX = 16; + + private BaggageCodec() {} + + /** + * Decodes an array of URL safe 7-bit characters into an array of original bytes. Escaped + * characters are converted back to their original representation. + * + * @param bytes array of URL safe characters + * @return array of original bytes + */ + private static byte[] decode(byte[] bytes) { + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + for (int i = 0; i < bytes.length; i++) { + int b = bytes[i]; + if (b == ESCAPE_CHAR) { + try { + int u = digit16(bytes[++i]); + int l = digit16(bytes[++i]); + buffer.write((char) ((u << 4) + l)); + } catch (ArrayIndexOutOfBoundsException e) { // FIXME + throw new IllegalArgumentException("Invalid URL encoding: ", e); + } + } else { + buffer.write(b); + } + } + return buffer.toByteArray(); + } + + /** + * Decodes an array of URL safe 7-bit characters into an array of original bytes. Escaped + * characters are converted back to their original representation. + * + * @param value string of URL safe characters + * @param charset encoding of given string + * @return decoded value + */ + static String decode(String value, Charset charset) { + byte[] bytes = decode(value.getBytes(StandardCharsets.US_ASCII)); + return new String(bytes, charset); + } + + /** + * Returns the numeric value of the character {@code b} in radix 16. + * + * @param b The byte to be converted. + * @return The numeric value represented by the character in radix 16. + */ + private static int digit16(byte b) { + int i = Character.digit((char) b, RADIX); + if (i == -1) { + throw new IllegalArgumentException( // FIXME + "Invalid URL encoding: not a valid digit (radix " + RADIX + "): " + b); + } + return i; + } +} 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 345fb177a5..d6c26fcef2 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 @@ -7,8 +7,6 @@ package io.opentelemetry.api.baggage.propagation; import io.opentelemetry.api.baggage.BaggageBuilder; import io.opentelemetry.api.baggage.BaggageEntryMetadata; -import java.io.UnsupportedEncodingException; -import java.net.URLDecoder; import java.nio.charset.StandardCharsets; import javax.annotation.Nullable; @@ -64,6 +62,8 @@ class Parser { } else { skipToNext = true; } + } else if (state == State.VALUE) { + skipToNext = !value.tryNextChar(current, i); } break; } @@ -146,11 +146,7 @@ class Parser { if (value == null) { return null; } - try { - return URLDecoder.decode(value, StandardCharsets.UTF_8.name()); - } catch (UnsupportedEncodingException e) { - return null; - } + return BaggageCodec.decode(value, StandardCharsets.UTF_8); } /** diff --git a/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/BaggageCodecTest.java b/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/BaggageCodecTest.java new file mode 100644 index 0000000000..5de8fe6763 --- /dev/null +++ b/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/BaggageCodecTest.java @@ -0,0 +1,42 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.api.baggage.propagation; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.nio.charset.StandardCharsets; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +class BaggageCodecTest { + + @ParameterizedTest + @CsvSource( + quoteCharacter = ';', // default is a quote character "'", which collides with %27 character. + value = { + "%21,!", "%23,#", "%24,$", "%25,%", "%26,&", "%27,'", "%28,(", "%29,)", "%2A,*", "%2B,+", + "%2D,-", "%2E,.", "%2F,/", "%30,0", "%31,1", "%32,2", "%33,3", "%34,4", "%35,5", "%36,6", + "%37,7", "%38,8", "%39,9", "%3A,:", "%3C,<", "%3D,=", "%3E,>", "%3F,?", "%40,@", "%41,A", + "%42,B", "%43,C", "%44,D", "%45,E", "%46,F", "%47,G", "%48,H", "%49,I", "%4A,J", "%4B,K", + "%4C,L", "%4D,M", "%4E,N", "%4F,O", "%50,P", "%51,Q", "%52,R", "%53,S", "%54,T", "%55,U", + "%56,V", "%57,W", "%58,X", "%59,Y", "%5A,Z", "%5B,[", "%5D,]", "%5E,^", "%5F,_", "%60,`", + "%61,a", "%62,b", "%63,c", "%64,d", "%65,e", "%66,f", "%67,g", "%68,h", "%69,i", "%6A,j", + "%6B,k", "%6C,l", "%6D,m", "%6E,n", "%6F,o", "%70,p", "%71,q", "%72,r", "%73,s", "%74,t", + "%75,u", "%76,v", "%77,w", "%78,x", "%79,y", "%7A,z", "%7B,{", "%7C,|", "%7D,}", "%7E,~", + }) + void shouldDecodePercentEncodedValues(String percentEncoded, String expectedDecoded) { + assertThat(BaggageCodec.decode(percentEncoded, StandardCharsets.UTF_8)) + .isEqualTo(expectedDecoded); + } + + @Test + void shouldThrowIfMalformedData() { + assertThatThrownBy(() -> BaggageCodec.decode("%1", StandardCharsets.UTF_8)) + .isInstanceOf(IllegalArgumentException.class); + } +} 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 83e78ca0ec..0a95e27e83 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 @@ -18,8 +18,11 @@ import io.opentelemetry.api.baggage.Baggage; import io.opentelemetry.api.baggage.BaggageEntryMetadata; import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.TextMapGetter; +import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import javax.annotation.Nullable; import org.junit.jupiter.api.Test; import org.junit.runner.Result; @@ -62,6 +65,17 @@ class W3CBaggagePropagatorFuzzTest { Baggage extractedBaggage = Baggage.fromContext(extractedContext); assertThat(extractedBaggage).isEqualTo(baggage); } + + @Fuzz + public void baggageOctet(@From(BaggageOctetGenerator.class) String baggageValue) { + Map carrier = new HashMap<>(); + carrier.put("baggage", "key=" + baggageValue); + Context context = + baggagePropagator.extract(Context.current(), carrier, new MapTextMapGetter()); + Baggage baggage = Baggage.fromContext(context); + String value = baggage.getEntryValue("key"); + assertThat(value).isEqualTo(baggageValue); + } } // driver methods to avoid having to use the vintage junit engine, and to enable increasing the @@ -79,9 +93,15 @@ class W3CBaggagePropagatorFuzzTest { assertThat(result.wasSuccessful()).isTrue(); } - private static Result runTestCase(String roundTripRandomValues) { + @Test + void baggageOctetFuzzing() { + Result result = runTestCase("baggageOctet"); + assertThat(result.wasSuccessful()).isTrue(); + } + + private static Result runTestCase(String testCaseName) { return GuidedFuzzing.run( - TestCases.class, roundTripRandomValues, new NoGuidance(10000, System.out), System.out); + TestCases.class, testCaseName, new NoGuidance(10000, System.out), System.out); } public static class AsciiGenerator extends AbstractStringGenerator { @@ -109,4 +129,25 @@ class W3CBaggagePropagatorFuzzTest { return carrier.get(key); } } + + public static class BaggageOctetGenerator extends AbstractStringGenerator { + + private static final Set excluded = + new HashSet<>(Arrays.asList(' ', '"', ',', ';', '\\', '%')); + + @Override + protected int nextCodePoint(SourceOfRandomness random) { + while (true) { + char c = random.nextChar(' ', '~'); + if (!excluded.contains(c)) { + return c; + } + } + } + + @Override + protected boolean codePointInRange(int codePoint) { + return !excluded.contains((char) codePoint); + } + } }