Update the TraceState key validations to more closely match the w3c spec

api/src/main/java/io/opentelemetry/trace/TraceState.java

@@ -46,6 +46,8 @@ public abstract class TraceState {
   private static final int VALUE_MAX_SIZE = 256;
   private static final int MAX_KEY_VALUE_PAIRS = 32;
   private static final TraceState DEFAULT = TraceState.builder().build();
+  private static final int MAX_TENANT_ID_SIZE = 240;
+  public static final int MAX_VENDOR_ID_SIZE = 13;
 
   /**
    * Returns the default {@code TraceState} with no entries.
@@ -235,27 +237,61 @@ public static Entry create(String key, String value) {
   // Key is opaque string up to 256 characters printable. It MUST begin with a lowercase letter, and
   // can only contain lowercase letters a-z, digits 0-9, underscores _, dashes -, asterisks *, and
   // forward slashes /.  For multi-tenant vendor scenarios, an at sign (@) can be used to prefix the
-  // vendor name.
+  // vendor name. The tenant id (before the '@') is limited to 240 characters and the vendor id is
+  // limited to 13 characters. If in the multi-tenant vendor format, then the first character
+  // may additionally be digit.
+  //
   // todo: benchmark this implementation
   private static boolean validateKey(String key) {
-    if (key.length() > KEY_MAX_SIZE || key.isEmpty() || !isNumberOrDigit(key.charAt(0))) {
+    if (key.length() > KEY_MAX_SIZE
+        || key.isEmpty()
+        || isNotLowercaseLetterOrDigit(key.charAt(0))) {
       return false;
     }
-    int atSeenCount = 0;
+    boolean isMultiTenantVendorKey = false;
     for (int i = 1; i < key.length(); i++) {
       char c = key.charAt(i);
-      if (!isNumberOrDigit(c) && c != '_' && c != '-' && c != '@' && c != '*' && c != '/') {
+      if (isNotLegalKeyCharacter(c)) {
         return false;
       }
-      if ((c == '@') && (++atSeenCount > 1)) {
-        return false;
+      if (c == '@') {
+        // you can't have 2 '@' signs
+        if (isMultiTenantVendorKey) {
+          return false;
+        }
+        isMultiTenantVendorKey = true;
+        // tenant id (the part to the left of the '@' sign) must be 240 characters or less
+        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) {
+          return false;
+        }
       }
     }
+    if (!isMultiTenantVendorKey) {
+      // if it's not the vendor format (with an '@' sign), the key must start with a letter.
+      return isNotDigit(key.charAt(0));
+    }
     return true;
   }
 
-  private static boolean isNumberOrDigit(char ch) {
-    return (ch >= 'a' && ch <= 'z') || (ch >= '0' && ch <= '9');
+  private static boolean isNotLegalKeyCharacter(char c) {
+    return isNotLowercaseLetterOrDigit(c)
+        && c != '_'
+        && c != '-'
+        && c != '@'
+        && c != '*'
+        && c != '/';
+  }
+
+  private static boolean isNotLowercaseLetterOrDigit(char ch) {
+    return (ch < 'a' || ch > 'z') && isNotDigit(ch);
+  }
+
+  private static boolean isNotDigit(char ch) {
+    return ch < '0' || ch > '9';
   }
 
   // Value is opaque string up to 256 characters printable ASCII RFC0020 characters (i.e., the range

api/src/test/java/io/opentelemetry/trace/TraceStateTest.java

@@ -72,8 +72,15 @@ void invalidFirstKeyCharacter() {
 
   @Test
   void firstKeyCharacterDigitIsAllowed() {
-    TraceState result = EMPTY.toBuilder().set("1_key", FIRST_VALUE).build();
-    assertThat(result.get("1_key")).isEqualTo(FIRST_VALUE);
+    // note: a digit is only allowed if the key is in the tenant format (with an '@')
+    TraceState result = EMPTY.toBuilder().set("1@tenant", FIRST_VALUE).build();
+    assertThat(result.get("1@tenant")).isEqualTo(FIRST_VALUE);
+  }
+
+  @Test
+  void testValidLongTenantId() {
+    TraceState result = EMPTY.toBuilder().set("12345678901234567890@nr", FIRST_VALUE).build();
+    assertThat(result.get("12345678901234567890@nr")).isEqualTo(FIRST_VALUE);
   }
 
   @Test
@@ -88,6 +95,37 @@ void testValidAtSignVendorNamePrefix() {
     assertThat(result.get("1@nr")).isEqualTo(FIRST_VALUE);
   }
 
+  @Test
+  void testVendorIdLongerThan13Characters() {
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> EMPTY.toBuilder().set("1@nrabcdefghijkl", FIRST_VALUE).build());
+  }
+
+  @Test
+  void testVendorIdLongerThan13Characters_longTenantId() {
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> EMPTY.toBuilder().set("12345678901234567890@nrabcdefghijkl", FIRST_VALUE).build());
+  }
+
+  @Test
+  void tenantIdLongerThan240Characters() {
+    char[] chars = new char[241];
+    Arrays.fill(chars, 'a');
+    String tenantId = new String(chars);
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> EMPTY.toBuilder().set(tenantId + "@nr", FIRST_VALUE).build());
+  }
+
+  @Test
+  void testNonVendorFormatFirstKeyCharacter() {
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> EMPTY.toBuilder().set("1acdfrgs", FIRST_VALUE).build());
+  }
+
   @Test
   void testMultipleAtSignNotAllowed() {
     assertThrows(