Skip to content

Commit

Permalink
Adds '+' character to allowed characters in baggage value (#4898)
Browse files Browse the repository at this point in the history
* Adds '+' character to allowed characters in baggade value

* Formatting fix

* Adda BaggageCodec implementation

* Additional cleanup

* Removal of Nullable method parameters.

* Additional tests for baggage decoding
  • Loading branch information
lmonkiewicz authored Nov 10, 2022
1 parent 4b6df42 commit e814602
Show file tree
Hide file tree
Showing 4 changed files with 176 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -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:
*
* <ul>
* <li><a
* href="https://github.com/apache/commons-codec/blob/482df6cabfb288acb6ab3e4a732fdb93aecfa7c2/src/main/java/org/apache/commons/codec/net/URLCodec.java">org.apache.commons.codec.net.URLCodec</a>
* <li><a
* href="https://github.com/apache/commons-codec/blob/482df6cabfb288acb6ab3e4a732fdb93aecfa7c2/src/main/java/org/apache/commons/codec/net/Utils.java">org.apache.commons.codec.net.Utils</a>
* </ul>
*
* <p>Implements baggage-octet decoding in accordance with th <a
* href="https://w3c.github.io/baggage/#definition">Baggage header content</a> 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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@

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;

Expand Down Expand Up @@ -64,6 +62,8 @@ void parseInto(BaggageBuilder baggageBuilder) {
} else {
skipToNext = true;
}
} else if (state == State.VALUE) {
skipToNext = !value.tryNextChar(current, i);
}
break;
}
Expand Down Expand Up @@ -146,11 +146,7 @@ private static String decodeValue(@Nullable String value) {
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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@
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;
Expand Down Expand Up @@ -62,6 +65,17 @@ public void roundTripAsciiValues(
Baggage extractedBaggage = Baggage.fromContext(extractedContext);
assertThat(extractedBaggage).isEqualTo(baggage);
}

@Fuzz
public void baggageOctet(@From(BaggageOctetGenerator.class) String baggageValue) {
Map<String, String> 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
Expand All @@ -79,9 +93,15 @@ void roundTripFuzzing() {
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 {
Expand Down Expand Up @@ -109,4 +129,25 @@ public String get(Map<String, String> carrier, String key) {
return carrier.get(key);
}
}

public static class BaggageOctetGenerator extends AbstractStringGenerator {

private static final Set<Character> 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);
}
}
}

0 comments on commit e814602

Please sign in to comment.