B3: Add Additional Validity Checks and tests for Trace and Span ID (#1457)

extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3Propagator.java

@@ -38,6 +38,7 @@ public class B3Propagator implements HttpTextFormat {
   static final String FALSE_INT = "0";
   static final String COMBINED_HEADER = "b3";
   static final String COMBINED_HEADER_DELIMITER = "-";
+  static final int MIN_TRACE_ID_LENGTH = TraceId.getSize();
   static final int MAX_TRACE_ID_LENGTH = 2 * TraceId.getSize();
   static final int MAX_SPAN_ID_LENGTH = 2 * SpanId.getSize();
 

extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractor.java

@@ -18,6 +18,7 @@ package io.opentelemetry.extensions.trace.propagation;
 
 import static io.opentelemetry.extensions.trace.propagation.B3Propagator.MAX_SPAN_ID_LENGTH;
 import static io.opentelemetry.extensions.trace.propagation.B3Propagator.MAX_TRACE_ID_LENGTH;
+import static io.opentelemetry.extensions.trace.propagation.B3Propagator.MIN_TRACE_ID_LENGTH;
 import static io.opentelemetry.extensions.trace.propagation.B3Propagator.TRUE_INT;
 
 import io.grpc.Context;
@@ -63,12 +64,25 @@ interface B3PropagatorExtractor {
       }
     }
 
+    private static boolean isHex(String value) {
+      for (int i = 0; i < value.length(); i++) {
+        if (Character.digit(value.charAt(i), 16) == -1) {
+          return false;
+        }
+      }
+      return true;
+    }
+
     static boolean isTraceIdValid(String value) {
-      return !(StringUtils.isNullOrEmpty(value) || value.length() > MAX_TRACE_ID_LENGTH);
+      return !(StringUtils.isNullOrEmpty(value)
+          || (value.length() != MIN_TRACE_ID_LENGTH && value.length() != MAX_TRACE_ID_LENGTH)
+          || !isHex(value));
     }
 
     static boolean isSpanIdValid(String value) {
-      return !(StringUtils.isNullOrEmpty(value) || value.length() > MAX_SPAN_ID_LENGTH);
+      return !(StringUtils.isNullOrEmpty(value)
+          || value.length() != MAX_SPAN_ID_LENGTH
+          || !isHex(value));
     }
   }
 }

extensions/trace_propagators/src/main/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorExtractorMultipleHeaders.java

@@ -51,16 +51,13 @@ final class B3PropagatorExtractorMultipleHeaders implements B3PropagatorExtracto
     String traceId = getter.get(carrier, TRACE_ID_HEADER);
     if (!Util.isTraceIdValid(traceId)) {
       logger.info(
-          "Invalid TraceId in B3 header: "
+          "Invalid TraceId in B3 header: " + traceId + "'. Returning INVALID span context.");
-              + TRACE_ID_HEADER
-              + "'. Returning INVALID span context.");
       return SpanContext.getInvalid();
     }
 
     String spanId = getter.get(carrier, SPAN_ID_HEADER);
     if (!Util.isSpanIdValid(spanId)) {
-      logger.info(
+      logger.info("Invalid SpanId in B3 header: " + spanId + "'. Returning INVALID span context.");
-          "Invalid SpanId in B3 header: " + SPAN_ID_HEADER + "'. Returning INVALID span context.");
       return SpanContext.getInvalid();
     }
 

extensions/trace_propagators/src/test/java/io/opentelemetry/extensions/trace/propagation/B3PropagatorTest.java

@@ -45,11 +45,13 @@ public class B3PropagatorTest {
 
   private static final TraceState TRACE_STATE_DEFAULT = TraceState.builder().build();
   private static final String TRACE_ID_BASE16 = "ff000000000000000000000000000041";
+  private static final String TRACE_ID_ALL_ZERO = "00000000000000000000000000000000";
   private static final TraceId TRACE_ID = TraceId.fromLowerBase16(TRACE_ID_BASE16, 0);
   private static final String SHORT_TRACE_ID_BASE16 = "ff00000000000000";
   private static final TraceId SHORT_TRACE_ID =
       TraceId.fromLowerBase16(StringUtils.padLeft(SHORT_TRACE_ID_BASE16, 32), 0);
   private static final String SPAN_ID_BASE16 = "ff00000000000041";
+  private static final String SPAN_ID_ALL_ZERO = "0000000000000000";
   private static final SpanId SPAN_ID = SpanId.fromLowerBase16(SPAN_ID_BASE16, 0);
   private static final byte SAMPLED_TRACE_OPTIONS_BYTES = 1;
   private static final TraceFlags SAMPLED_TRACE_OPTIONS =
@@ -221,9 +223,9 @@ public class B3PropagatorTest {
   }
 
   @Test
-  public void extract_InvalidTraceId() {
+  public void extract_InvalidTraceId_NotHex() {
     Map<String, String> invalidHeaders = new LinkedHashMap<>();
-    invalidHeaders.put(B3Propagator.TRACE_ID_HEADER, "abcdefghijklmnopabcdefghijklmnop");
+    invalidHeaders.put(B3Propagator.TRACE_ID_HEADER, "g" + TRACE_ID_BASE16.substring(1));
     invalidHeaders.put(B3Propagator.SPAN_ID_HEADER, SPAN_ID_BASE16);
     invalidHeaders.put(B3Propagator.SAMPLED_HEADER, B3Propagator.TRUE_INT);
     assertThat(getSpanContext(b3Propagator.extract(Context.current(), invalidHeaders, getter)))
@@ -231,7 +233,17 @@ public class B3PropagatorTest {
   }
 
   @Test
-  public void extract_InvalidTraceId_Size() {
+  public void extract_InvalidTraceId_TooShort() {
+    Map<String, String> invalidHeaders = new LinkedHashMap<>();
+    invalidHeaders.put(B3Propagator.TRACE_ID_HEADER, TRACE_ID_BASE16.substring(2));
+    invalidHeaders.put(B3Propagator.SPAN_ID_HEADER, SPAN_ID_BASE16);
+    invalidHeaders.put(B3Propagator.SAMPLED_HEADER, B3Propagator.TRUE_INT);
+    assertThat(getSpanContext(b3Propagator.extract(Context.current(), invalidHeaders, getter)))
+        .isSameInstanceAs(SpanContext.getInvalid());
+  }
+
+  @Test
+  public void extract_InvalidTraceId_TooLong() {
     Map<String, String> invalidHeaders = new LinkedHashMap<>();
     invalidHeaders.put(B3Propagator.TRACE_ID_HEADER, TRACE_ID_BASE16 + "00");
     invalidHeaders.put(B3Propagator.SPAN_ID_HEADER, SPAN_ID_BASE16);
@@ -241,20 +253,50 @@ public class B3PropagatorTest {
   }
 
   @Test
-  public void extract_InvalidSpanId() {
+  public void extract_InvalidTraceId_AllZero() {
     Map<String, String> invalidHeaders = new LinkedHashMap<>();
-    invalidHeaders.put(B3Propagator.TRACE_ID_HEADER, TRACE_ID_BASE16);
+    invalidHeaders.put(B3Propagator.TRACE_ID_HEADER, TRACE_ID_ALL_ZERO);
-    invalidHeaders.put(B3Propagator.SPAN_ID_HEADER, "abcdefghijklmnop");
+    invalidHeaders.put(B3Propagator.SPAN_ID_HEADER, SPAN_ID_BASE16);
     invalidHeaders.put(B3Propagator.SAMPLED_HEADER, B3Propagator.TRUE_INT);
     assertThat(getSpanContext(b3Propagator.extract(Context.current(), invalidHeaders, getter)))
         .isSameInstanceAs(SpanContext.getInvalid());
   }
 
   @Test
-  public void extract_InvalidSpanId_Size() {
+  public void extract_InvalidSpanId_NotHex() {
     Map<String, String> invalidHeaders = new LinkedHashMap<>();
     invalidHeaders.put(B3Propagator.TRACE_ID_HEADER, TRACE_ID_BASE16);
-    invalidHeaders.put(B3Propagator.SPAN_ID_HEADER, "abcdefghijklmnop" + "00");
+    invalidHeaders.put(B3Propagator.SPAN_ID_HEADER, "g" + SPAN_ID_BASE16.substring(1));
+    invalidHeaders.put(B3Propagator.SAMPLED_HEADER, B3Propagator.TRUE_INT);
+    assertThat(getSpanContext(b3Propagator.extract(Context.current(), invalidHeaders, getter)))
+        .isSameInstanceAs(SpanContext.getInvalid());
+  }
+
+  @Test
+  public void extract_InvalidSpanId_TooShort() {
+    Map<String, String> invalidHeaders = new LinkedHashMap<>();
+    invalidHeaders.put(B3Propagator.TRACE_ID_HEADER, TRACE_ID_BASE16);
+    invalidHeaders.put(B3Propagator.SPAN_ID_HEADER, SPAN_ID_BASE16.substring(2));
+    invalidHeaders.put(B3Propagator.SAMPLED_HEADER, B3Propagator.TRUE_INT);
+    assertThat(getSpanContext(b3Propagator.extract(Context.current(), invalidHeaders, getter)))
+        .isSameInstanceAs(SpanContext.getInvalid());
+  }
+
+  @Test
+  public void extract_InvalidSpanId_TooLong() {
+    Map<String, String> invalidHeaders = new LinkedHashMap<>();
+    invalidHeaders.put(B3Propagator.TRACE_ID_HEADER, TRACE_ID_BASE16);
+    invalidHeaders.put(B3Propagator.SPAN_ID_HEADER, SPAN_ID_BASE16 + "00");
+    invalidHeaders.put(B3Propagator.SAMPLED_HEADER, B3Propagator.TRUE_INT);
+    assertThat(getSpanContext(b3Propagator.extract(Context.current(), invalidHeaders, getter)))
+        .isSameInstanceAs(SpanContext.getInvalid());
+  }
+
+  @Test
+  public void extract_InvalidSpanId_AllZeros() {
+    Map<String, String> invalidHeaders = new LinkedHashMap<>();
+    invalidHeaders.put(B3Propagator.TRACE_ID_HEADER, TRACE_ID_BASE16);
+    invalidHeaders.put(B3Propagator.SPAN_ID_HEADER, SPAN_ID_ALL_ZERO);
     invalidHeaders.put(B3Propagator.SAMPLED_HEADER, B3Propagator.TRUE_INT);
     assertThat(getSpanContext(b3Propagator.extract(Context.current(), invalidHeaders, getter)))
         .isSameInstanceAs(SpanContext.getInvalid());