Fix more tracestate testsuite cases. (open-telemetry#3120)

Anuraag Agrawal · web-flow · commit 531abc0d24ed · 2021-04-06T15:54:57.000-07:00
diff --git a/api/all/src/main/java/io/opentelemetry/api/trace/ArrayBasedTraceStateBuilder.java b/api/all/src/main/java/io/opentelemetry/api/trace/ArrayBasedTraceStateBuilder.java
@@ -126,8 +126,9 @@ private static boolean isKeyValid(@Nullable String key) {
         if (i > MAX_TENANT_ID_SIZE) {
           return false;
         }
-        // vendor id (the part to the right of the '@' sign) must be 13 characters or less
-        if (key.length() - i > MAX_VENDOR_ID_SIZE) {
+        // vendor id (the part to the right of the '@' sign) must be 1-13 characters long
+        int remainingKeyChars = key.length() - i - 1;
+        if (remainingKeyChars > MAX_VENDOR_ID_SIZE || remainingKeyChars == 0) {
           return false;
         }
       }
diff --git a/api/all/src/main/java/io/opentelemetry/api/trace/propagation/W3CTraceContextPropagator.java b/api/all/src/main/java/io/opentelemetry/api/trace/propagation/W3CTraceContextPropagator.java
@@ -245,6 +245,11 @@ private static TraceState extractTraceState(String traceStateHeader) {
       checkArgument(index != -1, "Invalid TraceState list-member format.");
       traceStateBuilder.put(listMember.substring(0, index), listMember.substring(index + 1));
     }
-    return traceStateBuilder.build();
+    TraceState traceState = traceStateBuilder.build();
+    if (traceState.size() != listMembers.length) {
+      // Validation failure, drop the tracestate
+      return TraceState.getDefault();
+    }
+    return traceState;
   }
 }
diff --git a/api/all/src/test/java/io/opentelemetry/api/trace/propagation/W3CTraceContextPropagatorTest.java b/api/all/src/test/java/io/opentelemetry/api/trace/propagation/W3CTraceContextPropagatorTest.java
@@ -24,6 +24,8 @@
 import java.util.Map;
 import javax.annotation.Nullable;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 /** Unit tests for {@link W3CTraceContextPropagator}. */
 class W3CTraceContextPropagatorTest {
@@ -448,10 +450,7 @@ void extract_InvalidTracestate_EmptyValue() {
                 w3cTraceContextPropagator.extract(Context.current(), invalidHeaders, getter)))
         .isEqualTo(
             SpanContext.createFromRemoteParent(
-                TRACE_ID_BASE16,
-                SPAN_ID_BASE16,
-                TraceFlags.getSampled(),
-                TraceState.builder().put("test", "test").build()));
+                TRACE_ID_BASE16, SPAN_ID_BASE16, TraceFlags.getSampled(), TraceState.getDefault()));
   }
 
   @Test
@@ -545,4 +544,18 @@ void extract_nullGetter() {
     assertThat(w3cTraceContextPropagator.extract(context, Collections.emptyMap(), null))
         .isSameAs(context);
   }
+
+  // Tests transplanted from the w3c test suite
+
+  @ParameterizedTest
+  @ValueSource(
+      strings = {"foo@=1,bar=2", "@foo=1,bar=2", "foo@@bar=1,bar=2", "foo@bar@baz=1,bar=2"})
+  void test_tracestate_key_illegal_vendor_format(String traceState) {
+    Map<String, String> invalidHeaders = new HashMap<>();
+    invalidHeaders.put(W3CTraceContextPropagator.TRACE_PARENT, TRACEPARENT_HEADER_SAMPLED);
+    invalidHeaders.put(W3CTraceContextPropagator.TRACE_STATE, traceState);
+    Context context =
+        W3CTraceContextPropagator.getInstance().extract(Context.root(), invalidHeaders, getter);
+    assertThat(Span.fromContext(context).getSpanContext().getTraceState().get("bar")).isNull();
+  }
 }
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -319,6 +319,7 @@ subprojects {
             add(TEST_COMPILE_ONLY_CONFIGURATION_NAME, "com.google.code.findbugs:jsr305")
 
             add(TEST_IMPLEMENTATION_CONFIGURATION_NAME, "org.junit.jupiter:junit-jupiter-api")
+            add(TEST_IMPLEMENTATION_CONFIGURATION_NAME, "org.junit.jupiter:junit-jupiter-params")
             add(TEST_IMPLEMENTATION_CONFIGURATION_NAME, "nl.jqno.equalsverifier:equalsverifier")
             add(TEST_IMPLEMENTATION_CONFIGURATION_NAME, "org.mockito:mockito-core")
             add(TEST_IMPLEMENTATION_CONFIGURATION_NAME, "org.mockito:mockito-junit-jupiter")
diff --git a/integration-tests/tracecontext/src/main/java/io/opentelemetry/Application.java b/integration-tests/tracecontext/src/main/java/io/opentelemetry/Application.java
@@ -25,6 +25,7 @@
 import io.opentelemetry.context.propagation.ContextPropagators;
 import io.opentelemetry.context.propagation.TextMapGetter;
 import io.opentelemetry.context.propagation.TextMapSetter;
+import io.opentelemetry.sdk.OpenTelemetrySdk;
 import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -39,8 +40,9 @@ public final class Application {
 
   static {
     openTelemetry =
-        OpenTelemetry.propagating(
-            ContextPropagators.create(W3CTraceContextPropagator.getInstance()));
+        OpenTelemetrySdk.builder()
+            .setPropagators(ContextPropagators.create(W3CTraceContextPropagator.getInstance()))
+            .build();
   }
 
   private enum ArmeriaGetter implements TextMapGetter<RequestHeaders> {

Original file line number	Diff line number	Diff line change
`@@ -126,8 +126,9 @@ private static boolean isKeyValid(@Nullable String key) {`
`126`	`126`	`if (i > MAX_TENANT_ID_SIZE) {`
`127`	`127`	`return false;`
`128`	`128`	`}`
`129`		`- // vendor id (the part to the right of the '@' sign) must be 13 characters or less`
`130`		`- if (key.length() - i > MAX_VENDOR_ID_SIZE) {`
	`129`	`+ // vendor id (the part to the right of the '@' sign) must be 1-13 characters long`
	`130`	`+ int remainingKeyChars = key.length() - i - 1;`
	`131`	`+ if (remainingKeyChars > MAX_VENDOR_ID_SIZE \|\| remainingKeyChars == 0) {`
`131`	`132`	`return false;`
`132`	`133`	`}`
`133`	`134`	`}`
Original file line number	Diff line number	Diff line change
`@@ -245,6 +245,11 @@ private static TraceState extractTraceState(String traceStateHeader) {`
`245`	`245`	`checkArgument(index != -1, "Invalid TraceState list-member format.");`
`246`	`246`	`traceStateBuilder.put(listMember.substring(0, index), listMember.substring(index + 1));`
`247`	`247`	`}`
`248`		`- return traceStateBuilder.build();`
	`248`	`+ TraceState traceState = traceStateBuilder.build();`
	`249`	`+ if (traceState.size() != listMembers.length) {`
	`250`	`+ // Validation failure, drop the tracestate`
	`251`	`+ return TraceState.getDefault();`
	`252`	`+ }`
	`253`	`+ return traceState;`
`249`	`254`	`}`
`250`	`255`	`}`