Skip to content

Commit 69d26c1

Browse files
jkwatsonjkwatson
authored
Properly encode baggage values and metadata (open-telemetry#3740)
* URL encode & decode baggage values Resolves open-telemetry#3442 * Vendor in the guava url escaping code, and use it, rather than URLEncoder * add fuzz testing for the escaper and propagator * formatting * Remove strict value validation from the baggage implementation And, ensure we encode baggage metadata as well as the baggage value. * Add additional fuzz iterations, manually. * remove the usage of the vintage engine and use driver test methods instead
1 parent 3b5c48f commit 69d26c1

8 files changed

Lines changed: 561 additions & 49 deletions

File tree

‎api/all/src/main/java/io/opentelemetry/api/baggage/ImmutableBaggage.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,6 @@ private static boolean isKeyValid(String name) {
105105
* @return whether the value is valid.
106106
*/
107107
private static boolean isValueValid(String value) {
108-
return value != null && StringUtils.isPrintableString(value);
108+
return value != null;
109109
}
110110
}

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

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77

88
import io.opentelemetry.api.baggage.BaggageBuilder;
99
import io.opentelemetry.api.baggage.BaggageEntryMetadata;
10+
import java.io.UnsupportedEncodingException;
11+
import java.net.URLDecoder;
12+
import java.nio.charset.StandardCharsets;
1013
import javax.annotation.Nullable;
1114

1215
/**
@@ -83,11 +86,7 @@ void parseInto(BaggageBuilder baggageBuilder) {
8386
break;
8487
case KEY: // none
8588
}
86-
putBaggage(
87-
baggageBuilder,
88-
key.getValue(),
89-
value.getValue(),
90-
BaggageEntryMetadata.create(meta));
89+
putBaggage(baggageBuilder, key.getValue(), value.getValue(), meta);
9190
reset(i + 1);
9291
break;
9392
}
@@ -112,16 +111,14 @@ void parseInto(BaggageBuilder baggageBuilder) {
112111
case META:
113112
{
114113
String rest = baggageHeader.substring(metaStart).trim();
115-
putBaggage(
116-
baggageBuilder, key.getValue(), value.getValue(), BaggageEntryMetadata.create(rest));
114+
putBaggage(baggageBuilder, key.getValue(), value.getValue(), rest);
117115
break;
118116
}
119117
case VALUE:
120118
{
121119
if (!skipToNext) {
122120
value.tryTerminating(baggageHeader.length(), baggageHeader);
123-
putBaggage(
124-
baggageBuilder, key.getValue(), value.getValue(), BaggageEntryMetadata.empty());
121+
putBaggage(baggageBuilder, key.getValue(), value.getValue(), null);
125122
break;
126123
}
127124
}
@@ -132,9 +129,27 @@ private static void putBaggage(
132129
BaggageBuilder baggage,
133130
@Nullable String key,
134131
@Nullable String value,
135-
BaggageEntryMetadata metadata) {
136-
if (key != null && value != null) {
137-
baggage.put(key, value, metadata);
132+
@Nullable String metadataValue) {
133+
String decodedValue = decodeValue(value);
134+
metadataValue = decodeValue(metadataValue);
135+
BaggageEntryMetadata baggageEntryMetadata =
136+
metadataValue != null
137+
? BaggageEntryMetadata.create(metadataValue)
138+
: BaggageEntryMetadata.empty();
139+
if (key != null && decodedValue != null) {
140+
baggage.put(key, decodedValue, baggageEntryMetadata);
141+
}
142+
}
143+
144+
@Nullable
145+
private static String decodeValue(@Nullable String value) {
146+
if (value == null) {
147+
return null;
148+
}
149+
try {
150+
return URLDecoder.decode(value, StandardCharsets.UTF_8.name());
151+
} catch (UnsupportedEncodingException e) {
152+
return null;
138153
}
139154
}
140155

0 commit comments

Comments
 (0)