fix(baggage): ignore legacy baggage keys if not a valid header (#7214)

The legacy baggage implementation is using headers for transporting
the entries. That is causing issues in case the used key is not
a valid header. Thus, ignore those while still keeping the baggage
entries as before for other usage.

Fixes: #7043

Author: BridgeAR

packages/dd-trace/src/opentracing/propagation/text_map.js

@@ -36,6 +36,9 @@ const b3HeaderExpr = /^(([0-9a-f]{16}){1,2}-[0-9a-f]{16}(-[01d](-[0-9a-f]{16})?)
 const baggageExpr = new RegExp(`^${baggagePrefix}(.+)$`)
 const tagKeyExpr = /^_dd\.p\.[\x21-\x2B\x2D-\x7E]+$/ // ASCII minus spaces and commas
 const tagValueExpr = /^[\x20-\x2B\x2D-\x7E]*$/ // ASCII minus commas
+// RFC7230 token (used by HTTP header field-name) and compatible with Node's header name validation.
+// See https://www.rfc-editor.org/rfc/rfc7230#section-3.2.6
+const httpHeaderNameExpr = /^[0-9A-Za-z!#$%&'*+\-.^_`|~]+$/
 const traceparentExpr = /^([a-f0-9]{2})-([a-f0-9]{32})-([a-f0-9]{16})-([a-f0-9]{2})(-.*)?$/i
 const traceparentKey = 'traceparent'
 const tracestateKey = 'tracestate'
@@ -130,9 +133,21 @@ class TextMapPropagator {
 
   _injectBaggageItems (spanContext, carrier) {
     if (this._config.legacyBaggageEnabled) {
-      spanContext?._baggageItems && Object.keys(spanContext._baggageItems).forEach(key => {
-        carrier[baggagePrefix + key] = String(spanContext._baggageItems[key])
-      })
+      const baggageItems = spanContext?._baggageItems
+      if (baggageItems) {
+        for (const key of Object.keys(baggageItems)) {
+          const headerName = baggagePrefix + key
+
+          // Legacy OpenTracing baggage is propagated as individual headers (ot-baggage-*),
+          // so it must always be representable as a valid HTTP header name.
+          if (!httpHeaderNameExpr.test(headerName)) {
+            tracerMetrics.count('context_header_style.malformed', ['header_style:baggage']).inc()
+            continue
+          }
+
+          carrier[headerName] = String(baggageItems[key])
+        }
+      }
     }
     if (this._hasPropagationStyle('inject', 'baggage')) {
       let baggage = ''
@@ -646,9 +661,10 @@ class TextMapPropagator {
 
   _extractBaggageItems (carrier, spanContext) {
     if (!this._hasPropagationStyle('extract', 'baggage')) return
-    if (!carrier || !carrier.baggage) return
+    if (!carrier?.baggage) return
     const baggages = carrier.baggage.split(',')
-    const keysToSpanTag = this._config.baggageTagKeys === '*'
+    const tagAllKeys = this._config.baggageTagKeys === '*'
+    const keysToSpanTag = tagAllKeys
       ? undefined
       : new Set(this._config.baggageTagKeys.split(','))
     for (const keyValue of baggages) {
@@ -665,7 +681,7 @@ class TextMapPropagator {
         removeAllBaggageItems()
         return
       }
-      if (spanContext && (this._config.baggageTagKeys === '*' || keysToSpanTag.has(key))) {
+      if (spanContext && (tagAllKeys || keysToSpanTag?.has(key))) {
         spanContext._trace.tags['baggage.' + key] = value
       }
       setBaggageItem(key, value)

packages/dd-trace/test/opentracing/propagation/text_map.spec.js

@@ -126,6 +126,25 @@ describe('TextMapPropagator', () => {
       assert.strictEqual(carrier.baggage, undefined)
     })
 
+    it('should skip legacy baggage items that cannot be encoded as a valid HTTP header name', () => {
+      const carrier = {}
+      const tracerMetrics = telemetryMetrics.manager.namespace('tracers')
+      const spanContext = createContext({
+        baggageItems: {
+          ok: 'yes',
+          'not ok': 'no'
+        }
+      })
+
+      propagator.inject(spanContext, carrier)
+
+      assert.strictEqual(carrier['ot-baggage-ok'], 'yes')
+      assert.strictEqual(carrier['ot-baggage-not ok'], undefined)
+
+      sinon.assert.calledWith(tracerMetrics.count, 'context_header_style.malformed', ['header_style:baggage'])
+      sinon.assert.called(tracerMetrics.count().inc)
+    })
+
     it('should handle special characters in baggage', () => {
       const carrier = {}
       setBaggageItem('",;\\()/:<=>?@[]{}🐶é我', '",;\\🐶é我')
@@ -135,12 +154,17 @@ describe('TextMapPropagator', () => {
     })
 
     it('should drop excess baggage items when there are too many pairs', () => {
+      /** @type {Record<string, unknown>} */
       const carrier = {}
       for (let i = 0; i < config.baggageMaxItems + 1; i++) {
-        setBaggageItem(`key-${i}`, i)
+        setBaggageItem(`key-${i}`, String(i))
       }
       propagator.inject(undefined, carrier)
-      assert.strictEqual(carrier.baggage.split(',').length, config.baggageMaxItems)
+      const baggage = carrier.baggage
+      if (typeof baggage !== 'string') {
+        throw new Error('Expected baggage header to be a string')
+      }
+      assert.strictEqual(baggage.split(',').length, config.baggageMaxItems)
     })
 
     it('should drop excess baggage items when the resulting baggage header contains many bytes', () => {