Fix more tracestate testsuite cases. (#3120)

open-telemetry · Apr 6, 2021 · 531abc0 · 531abc0
1 parent 5bad869
commit 531abc0
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 9 deletions.
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/...l/src/test/java/io/opentelemetry/api/trace/propagation/W3CTraceContextPropagatorTest.java b/...l/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> {