fix(w3c): fix traceparent validation

ddtrace/propagation/http.py

@@ -73,6 +73,10 @@ def _possible_header(header):
 _POSSIBLE_HTTP_HEADER_TRACESTATE = _possible_header(_HTTP_HEADER_TRACESTATE)
 
 
+# https://www.w3.org/TR/trace-context/#traceparent-header-field-values
+_TRACEPARENT_HEX_REGEX = "^[a-f0-9]{2}-[a-f0-9]{32}-[a-f0-9]{16}-[a-f0-9]{2}$"
+
+
 def _extract_header_value(possible_header_names, headers, default=None):
     # type: (FrozenSet[str], Dict[str, str], Optional[str]) -> Optional[str]
     for header in possible_header_names:
@@ -550,35 +554,29 @@ def _get_traceparent_values(tp):
         Otherwise we extract the trace-id, span-id, and sampling priority from the
         traceparent header.
         """
+        tp = tp.strip()
+        if not re.match(_TRACEPARENT_HEX_REGEX, tp) or tp.startswith("ff"):
+            # ff is an invalid traceparent version: https://www.w3.org/TR/trace-context/#version
+            raise ValueError("W3C traceparent hex is invalid: %s" % tp)
 
         version, trace_id_hex, span_id_hex, trace_flags_hex = tp.strip().split("-")
-        # check version is a valid hexadecimal, if not it's invalid we will move on to the next prop method
-        int(version, 16)
-        # https://www.w3.org/TR/trace-context/#version
-        if version == "ff":
-            raise ValueError("'ff' is an invalid traceparent version")
-        # currently 00 is the only version format, but if future versions come up we may need to add changes
         if version != "00":
             log.warning("unsupported traceparent version:%r, still attempting to parse", version)
 
-        if len(trace_id_hex) == 32 and len(span_id_hex) == 16 and len(trace_flags_hex) >= 2:
-            trace_id = _hex_id_to_dd_id(trace_id_hex)
-            span_id = _hex_id_to_dd_id(span_id_hex)
+        trace_id = _hex_id_to_dd_id(trace_id_hex)
+        span_id = _hex_id_to_dd_id(span_id_hex)
 
-            # All 0s are invalid values
-            if trace_id == 0:
-                raise ValueError("0 value for trace_id is invalid")
-            if span_id == 0:
-                raise ValueError("0 value for span_id is invalid")
+        # All 0s are invalid values
+        if trace_id == 0:
+            raise ValueError("0 value for trace_id is invalid")
+        if span_id == 0:
+            raise ValueError("0 value for span_id is invalid")
 
-            trace_flags = _hex_id_to_dd_id(trace_flags_hex)
-            # there's currently only one trace flag, which denotes sampling priority
-            # was set to keep "01" or drop "00"
-            # trace flags is a bit field: https://www.w3.org/TR/trace-context/#trace-flags
-            sampling_priority = trace_flags & 0x1
-
-        else:
-            raise ValueError("W3C traceparent hex length incorrect: %s" % tp)
+        trace_flags = _hex_id_to_dd_id(trace_flags_hex)
+        # there's currently only one trace flag, which denotes sampling priority
+        # was set to keep "01" or drop "00"
+        # trace flags is a bit field: https://www.w3.org/TR/trace-context/#trace-flags
+        sampling_priority = trace_flags & 0x1
 
         return trace_id, span_id, sampling_priority
 
@@ -644,10 +642,6 @@ def _extract(headers):
             if tp is None:
                 log.debug("no traceparent header")
                 return None
-            # uppercase char in tp makes it invalid:
-            # https://www.w3.org/TR/trace-context/#traceparent-header-field-values
-            if not tp.islower():
-                raise ValueError("uppercase characters are not allowed in traceparent")
             trace_id, span_id, sampling_priority = _TraceContext._get_traceparent_values(tp)
         except (ValueError, AssertionError):
             log.exception("received invalid w3c traceparent: %s ", tp)

tests/tracer/test_propagation.py

@@ -448,17 +448,21 @@ def test_tracecontext_get_sampling_priority(sampling_priority_tp, sampling_prior
             "0-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01",
             # tp, trace_id, span_id, sampling_priority
             (11803532876627986230, 67667974448284343, 1),
-            ["unsupported traceparent version:'0', still attempting to parse"],
-            None,
+            [],
+            ValueError,
+        ),
+        (
+            "ff-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01",
+            # tp, trace_id, span_id, sampling_priority
+            (11803532876627986230, 67667974448284343, 1),
+            [],
+            ValueError,
         ),
         (
             "00-f92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01",
             # tp, trace_id, span_id, sampling_priority
             None,
-            [
-                "received invalid w3c traceparent: 00-f92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01.",
-                "W3C traceparent hex length incorrect: 00-f92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01",
-            ],
+            [],
             ValueError,
         ),
         (  # we still parse the trace flag and analyze the it as a bit field
@@ -476,6 +480,7 @@ def test_tracecontext_get_sampling_priority(sampling_priority_tp, sampling_prior
         "traceflag_00",
         "unsupported_version",
         "short_version",
+        "invalid_version",
         "short_trace_id",
         "unknown_trace_flag",
     ],
@@ -489,9 +494,10 @@ def test_extract_traceparent(caplog, headers, expected_tuple, expected_logging,
         else:
             traceparent_values = _TraceContext._get_traceparent_values(headers)
             assert traceparent_values == expected_tuple
-            if caplog.text or expected_logging:
-                for expected_log in expected_logging:
-                    assert expected_log in caplog.text
+
+        if caplog.text or expected_logging:
+            for expected_log in expected_logging:
+                assert expected_log in caplog.text
 
 
 @pytest.mark.parametrize(