fix: enforce w3c trace context value validation (#777)

* fix: enforce w3c trace context value validation

validate length and characters of the w3c trace context value.
see https://www.w3.org/TR/trace-context/#traceparent-header-field-values
refactor unit testing to bring invalid w3c trace context tests to separate file

Fixes #774

* 🦉 Updates from OwlBot

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* chore: fix bug in formatting validation

change length of the span field from 12 to 16 according to the standard

* chore: refactor w3c format regex

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

Author: minherz

README.md

@@ -58,13 +58,13 @@ implementation 'com.google.cloud:google-cloud-logging'
 If you are using Gradle without BOM, add this to your dependencies
 
 ```Groovy
-implementation 'com.google.cloud:google-cloud-logging:3.5.0'
+implementation 'com.google.cloud:google-cloud-logging:3.5.1'
 ```
 
 If you are using SBT, add this to your dependencies
 
 ```Scala
-libraryDependencies += "com.google.cloud" % "google-cloud-logging" % "3.5.0"
+libraryDependencies += "com.google.cloud" % "google-cloud-logging" % "3.5.1"
 ```
 
 ## Authentication

google-cloud-logging/src/main/java/com/google/cloud/logging/Context.java

@@ -18,11 +18,17 @@
 
 import com.google.cloud.logging.HttpRequest.RequestMethod;
 import com.google.common.base.MoreObjects;
-import com.google.common.base.Strings;
 import java.util.Objects;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /** Class to hold context attributes including information about {@see HttpRequest} and tracing. */
 public class Context {
+  // validate W3C trace context value on load according to the existing version format.
+  // see https://www.w3.org/TR/trace-context/#traceparent-header-field-values for details.
+  private static final Pattern W3C_TRACE_CONTEXT_FORMAT =
+      Pattern.compile(
+          "^00-(?!00000000000000000000000000000000)[0-9a-f]{32}-(?!0000000000000000)[0-9a-f]{16}-[0-9a-f]{2}$");
   private final HttpRequest request;
   private final String traceId;
   private final String spanId;
@@ -142,26 +148,15 @@ public Builder loadCloudTraceContext(String cloudTrace) {
      */
     public Builder loadW3CTraceParentContext(String traceParent) throws IllegalArgumentException {
       if (traceParent != null) {
-        String[] fields = traceParent.split("-");
-        if (fields.length > 3) {
-          String versionFormat = fields[0];
-          if (!versionFormat.equals("00")) {
-            throw new IllegalArgumentException("Not supporting versionFormat other than \"00\"");
-          }
-        } else {
+        Matcher validator = W3C_TRACE_CONTEXT_FORMAT.matcher(traceParent.toLowerCase());
+        if (!validator.matches()) {
           throw new IllegalArgumentException(
-              "Invalid format of the header value. Expected \"00-traceid-spanid-arguments\"");
-        }
-        String traceId = fields[1];
-        if (!traceId.isEmpty()) {
-          setTraceId(traceId);
-        }
-        if (!Strings.isNullOrEmpty(traceId)) {
-          String spanId = fields[2];
-          if (!spanId.isEmpty()) {
-            setSpanId(spanId);
-          }
+              "Invalid format of the header value. The value does not match W3C Trace Context version \"00\"");
         }
+        String[] fields = traceParent.split("-");
+        setTraceId(fields[1]);
+        setSpanId(fields[2]);
+        // fields[3] contains flag(s)
       }
       return this;
     }

google-cloud-logging/src/test/java/com/google/cloud/logging/ContextTest.java

@@ -132,50 +132,16 @@ public void testParsingCloudTraceContext() {
 
   @Test
   public void testParsingW3CTraceParent() {
-    final String TRACEPARENT_NO_TRACE = "00--SPAN_ID-FLAGS";
-    final String TRACEPARENT_TRACE_ONLY = "00-" + TEST_TRACE_ID + "--SPAN_ID-FLAGS";
-    final String TRACEPARENT_TRACE_FULL = "00-" + TEST_TRACE_ID + "-" + TEST_SPAN_ID + "-FLAGS";
+    final String W3C_TEST_TRACE_ID = "12345678901234567890123456789012";
+    final String W3C_TEST_SPAN_ID = "1234567890123456";
+    final String W3C_TRACE_CONTEXT = "00-" + W3C_TEST_TRACE_ID + "-" + W3C_TEST_SPAN_ID + "-00";
 
     Context.Builder builder = Context.newBuilder();
 
     builder.loadW3CTraceParentContext(null);
     assertTraceAndSpan(builder.build(), null, null);
-    builder.loadW3CTraceParentContext(TRACEPARENT_NO_TRACE);
-    assertTraceAndSpan(builder.build(), null, null);
-    builder.loadW3CTraceParentContext(TRACEPARENT_TRACE_ONLY);
-    assertTraceAndSpan(builder.build(), TEST_TRACE_ID, null);
-    builder.loadW3CTraceParentContext(TRACEPARENT_TRACE_FULL);
-    assertTraceAndSpan(builder.build(), TEST_TRACE_ID, TEST_SPAN_ID);
-  }
-
-  @Test(expected = IllegalArgumentException.class)
-  public void testEmptyW3CTraceParent() {
-    Context.Builder builder = Context.newBuilder();
-    builder.loadW3CTraceParentContext("");
-  }
-
-  @Test(expected = IllegalArgumentException.class)
-  public void testInvalidFormatW3CTraceParent() {
-    Context.Builder builder = Context.newBuilder();
-    builder.loadW3CTraceParentContext("TRACE_ID/SPAN_ID;o=TRACE_TRUE");
-  }
-
-  @Test(expected = IllegalArgumentException.class)
-  public void testInvalidFormatW3CTraceParent1Dash() {
-    Context.Builder builder = Context.newBuilder();
-    builder.loadW3CTraceParentContext("00-TRACE_ID");
-  }
-
-  @Test(expected = IllegalArgumentException.class)
-  public void testInvalidFormatW3CTraceParentWithoutFlag() {
-    Context.Builder builder = Context.newBuilder();
-    builder.loadW3CTraceParentContext("00-TRACE_ID-SPAN_ID-");
-  }
-
-  @Test(expected = IllegalArgumentException.class)
-  public void testInvalidVersionW3CTraceParent() {
-    Context.Builder builder = Context.newBuilder();
-    builder.loadW3CTraceParentContext("01-TRACE_ID-SPAN_ID-FLAGS");
+    builder.loadW3CTraceParentContext(W3C_TRACE_CONTEXT);
+    assertTraceAndSpan(builder.build(), W3C_TEST_TRACE_ID, W3C_TEST_SPAN_ID);
   }
 
   private void assertTraceAndSpan(Context context, String expectedTraceId, String expectedSpanId) {

google-cloud-logging/src/test/java/com/google/cloud/logging/InvalidContextTest.java

@@ -0,0 +1,57 @@
+/*
+ * Copyright 2021 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.cloud.logging;
+
+import java.util.Arrays;
+import java.util.Collection;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class InvalidContextTest {
+  @Parameters
+  public static Collection<String> data() {
+    final String[] INVALID_W3C_TRACE_CONTEXTS = {
+      "",
+      "abc/efg",
+      "01-something",
+      "00-123456789012345678901234567890",
+      "00-12345678901234567890123456789012",
+      "00-12345678901234567890123456789012345",
+      "00-12345678901234567890123456789012-123456789012345",
+      "00-12345678901234567890123456789012-1234567890123456",
+      "00-12345678901234567890123456789012-12345678901234567",
+      "00-12345678901234567890123456789012-1234567890123456-1",
+      "00-12345678901234567890123456789012-1234567890123456-123"
+    };
+    return Arrays.asList(INVALID_W3C_TRACE_CONTEXTS);
+  }
+
+  private final String traceContext;
+
+  public InvalidContextTest(String traceContext) {
+    this.traceContext = traceContext;
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testAssertionInvalidContext() {
+    Context.Builder builder = Context.newBuilder();
+    builder.loadW3CTraceParentContext(traceContext);
+  }
+}