Skip to content

Commit 6b55bdc

Browse files
authored
Add check to prevent injection of repeated GRPC headers (#9246)
* adding check for repeated baggage * writing unit tests * pushing removal of error log * adding test to verify allowing repeated non-baggage keys to be set * update GRPC inject to not allow all repeated keys * final updates
1 parent 6c07df1 commit 6b55bdc

File tree

4 files changed

+106
-2
lines changed

4 files changed

+106
-2
lines changed

dd-java-agent/instrumentation/armeria/armeria-grpc-0.84/src/main/java/datadog/trace/instrumentation/armeria/grpc/client/GrpcInjectAdapter.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ public final class GrpcInjectAdapter implements CarrierSetter<Metadata> {
1111

1212
@Override
1313
public void set(final Metadata carrier, final String key, final String value) {
14-
carrier.put(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER), value);
14+
Metadata.Key<String> metadataKey = Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER);
15+
if (carrier.containsKey(metadataKey)) {
16+
carrier.removeAll(
17+
metadataKey); // Remove existing to ensure identical behavior with other carriers
18+
}
19+
carrier.put(metadataKey, value);
1520
}
1621
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import datadog.trace.agent.test.naming.VersionedNamingTestBase
2+
import io.grpc.Metadata
3+
import static datadog.trace.instrumentation.grpc.client.GrpcInjectAdapter.SETTER
4+
5+
class GrpcInjectAdapterTest extends VersionedNamingTestBase {
6+
def "carrier overrides values for duplicate keys"() {
7+
setup:
8+
def carrier = new Metadata()
9+
10+
def baggage = [
11+
["ot-baggage-foo", "v1"],
12+
["ot-baggage-foo", "v2"],
13+
["ot-baggage-bar", "v3"]
14+
]
15+
16+
when:
17+
baggage.each { pair ->
18+
def (key, value) = pair
19+
SETTER.set(carrier, key, value)
20+
}
21+
22+
then:
23+
carrier.headerCount() == 2
24+
carrier.get(getKey("ot-baggage-foo")) == "v2" // overridden value wins
25+
carrier.get(getKey("ot-baggage-bar")) == "v3"
26+
}
27+
28+
Metadata.Key<String> getKey(String key){
29+
Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER)
30+
}
31+
32+
33+
@Override
34+
int version() {
35+
return 0
36+
}
37+
38+
@Override
39+
String service() {
40+
return null
41+
}
42+
43+
@Override
44+
String operation() {
45+
return null
46+
}
47+
}

dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/client/GrpcInjectAdapter.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ public final class GrpcInjectAdapter implements CarrierSetter<Metadata> {
1010

1111
@Override
1212
public void set(final Metadata carrier, final String key, final String value) {
13-
carrier.put(Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER), value);
13+
Metadata.Key<String> metadataKey = Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER);
14+
if (carrier.containsKey(metadataKey)) {
15+
carrier.removeAll(
16+
metadataKey); // Remove existing to ensure identical behavior with other carriers
17+
}
18+
carrier.put(metadataKey, value);
1419
}
1520
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import datadog.trace.agent.test.naming.VersionedNamingTestBase
2+
import io.grpc.Metadata
3+
import static datadog.trace.instrumentation.grpc.client.GrpcInjectAdapter.SETTER
4+
5+
class GrpcInjectAdapterTest extends VersionedNamingTestBase {
6+
def "carrier overrides values for duplicate keys"() {
7+
setup:
8+
def carrier = new Metadata()
9+
10+
def baggage = [
11+
["ot-baggage-foo", "v1"],
12+
["ot-baggage-foo", "v2"],
13+
["ot-baggage-bar", "v3"]
14+
]
15+
16+
when:
17+
baggage.each { pair ->
18+
def (key, value) = pair
19+
SETTER.set(carrier, key, value)
20+
}
21+
22+
then:
23+
carrier.headerCount() == 2
24+
carrier.get(getKey("ot-baggage-foo")) == "v2" // overridden value wins
25+
carrier.get(getKey("ot-baggage-bar")) == "v3"
26+
}
27+
28+
Metadata.Key<String> getKey(String key){
29+
Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER)
30+
}
31+
32+
33+
@Override
34+
int version() {
35+
return 0
36+
}
37+
38+
@Override
39+
String service() {
40+
return null
41+
}
42+
43+
@Override
44+
String operation() {
45+
return null
46+
}
47+
}

0 commit comments

Comments
 (0)