Fixes length bug in b3 single format

Thanks to @zyfjeff for finding this! See envoyproxy/envoy#4712 (comment)
openzipkin · Oct 17, 2018 · 8ab16a9 · 8ab16a9
1 parent 9155e14
commit 8ab16a9
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 2 deletions.
diff --git a/brave/src/main/java/brave/propagation/B3SingleFormat.java b/brave/src/main/java/brave/propagation/B3SingleFormat.java
@@ -42,7 +42,7 @@
  * <p>See <a href="https://github.com/openzipkin/b3-propagation">B3 Propagation</a>
  */
 public final class B3SingleFormat {
-  static final int FORMAT_MAX_LENGTH = 32 + 1 + 16 + 2 + 16; // traceid128-spanid-1-parentid
+  static final int FORMAT_MAX_LENGTH = 32 + 1 + 16 + 3 + 16; // traceid128-spanid-1-parentid
 
   /**
    * Writes all B3 defined fields in the trace context, except {@link TraceContext#parentIdAsLong()

diff --git a/brave/src/test/java/brave/propagation/B3SingleFormatTest.java b/brave/src/test/java/brave/propagation/B3SingleFormatTest.java
@@ -11,6 +11,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 public class B3SingleFormatTest {
+  String traceIdHigh = "0000000000000009";
   String traceId = "0000000000000001";
   String parentId = "0000000000000002";
   String spanId = "0000000000000003";
@@ -27,7 +28,7 @@ public class B3SingleFormatTest {
     TraceContext context = TraceContext.newBuilder().traceIdHigh(9).traceId(1).spanId(3).build();
 
     assertThat(writeB3SingleFormat(context))
-        .isEqualTo("0000000000000009" + traceId + "-" + spanId)
+        .isEqualTo(traceIdHigh + traceId + "-" + spanId)
         .isEqualTo(new String(writeB3SingleFormatAsBytes(context), UTF_8));
   }
 
@@ -64,6 +65,35 @@ public class B3SingleFormatTest {
         .isEqualTo(new String(writeB3SingleFormatAsBytes(context), UTF_8));
   }
 
+  @Test public void writeB3SingleFormat_largest() {
+    TraceContext context =
+        TraceContext.newBuilder()
+            .traceIdHigh(9)
+            .traceId(1)
+            .parentId(2)
+            .spanId(3)
+            .sampled(true)
+            .build();
+
+    assertThat(writeB3SingleFormat(context))
+        .isEqualTo(traceIdHigh + traceId + "-" + spanId + "-1-" + parentId)
+        .isEqualTo(new String(writeB3SingleFormatAsBytes(context), UTF_8));
+  }
+
+  @Test public void parseB3SingleFormat_largest() {
+    assertThat(
+        parseB3SingleFormat(traceIdHigh + traceId + "-" + spanId + "-1-" + parentId)
+    ).extracting(TraceContextOrSamplingFlags::context).isEqualToComparingFieldByField(
+        TraceContext.newBuilder()
+            .traceIdHigh(9)
+            .traceId(1)
+            .parentId(2)
+            .spanId(3)
+            .sampled(true)
+            .build()
+    );
+  }
+
   @Test public void writeB3SingleFormatWithoutParent_notYetSampled() {
     TraceContext context = TraceContext.newBuilder().traceId(1).spanId(3).build();
 
@@ -244,5 +274,9 @@ public class B3SingleFormatTest {
     // overall length is not ok
     assertThat(parseB3SingleFormat(traceId + traceId + traceId + "-" + spanId + "-" + traceId))
         .isNull();
+    // one character too long is not ok
+    assertThat(
+        parseB3SingleFormat(traceIdHigh + traceId + "-" + spanId + "-1-" + parentId + "a")
+    ).isNull();
   }
 }