Baggage header propagation is dropping property values

[zandt-square]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

latest

Framework Version

No response

Link to Sentry event

No response

Reproduction Example/SDK Setup

test.js file

const Sentry = require('@sentry/node');

Sentry.init({
  dsn: 'https://public@sentry.example.com/1"',
  // ...
});

const http = require('http');

const options = {
  hostname: 'localhost',
  port: 8080,
  headers: {
    baggage: 'key1=value1;property1;property2, key2 = value2, key3=value3; propertyKey=propertyValue'
  }
};

http.get(options);

Steps to Reproduce

Requests containing a baggage header with property values sent from a nodejs app using sentry results in those baggage property values being dropped.

Example baggage header (taken from w3c spec):

baggage: key1=value1;property1;property2, key2 = value2, key3=value3; propertyKey=propertyValue

The resulting baggage header is modified to:

baggage: key1=value1%3Bproperty1%3Bproperty2,key2=value2,key3=value3%3B%20propertyKey,sentry-environment=production,sentry-public_key=public,sentry-trace_id=3c04e02e27e644f2af89b05b02a1b120

The metadata associated with key3 has =propertyValue truncated.

Steps to recreate

nc -l localhost 8080 & node test.js && wait

GET / HTTP/1.1
baggage: key1=value1%3Bproperty1%3Bproperty2,key2=value2,key3=value3%3B%20propertyKey,sentry-environment=production,sentry-public_key=public,sentry-trace_id=3c04e02e27e644f2af89b05b02a1b120
Host: localhost:8080
sentry-trace: 3c04e02e27e644f2af89b05b02a1b120-98a5af2e203aec16
Connection: keep-alive

Expected Result

The baggage header preserves all metadata (incl properties) associated with a baggage key.

baggage: key1=value1%3Bproperty1%3Bproperty2,key2=value2,key3=value3%3B%20propertyKey%3DpropertyValue,sentry-environment=production,sentry-public_key=public,sentry-trace_id=3c04e02e27e644f2af89b05b02a1b120

or unencoded as
key1=value1;property1;property2,key2=value2,key3=value3; propertyKey=propertyValue,sentry-environment=production,sentry-public_key=public,sentry-trace_id=3c04e02e27e644f2af89b05b02a1b120

Actual Result

The actual result is that the propertyValue metadata property value associated with key3 is lost:

baggage: key1=value1%3Bproperty1%3Bproperty2,key2=value2,key3=value3%3B%20propertyKey,sentry-environment=production,sentry-public_key=public,sentry-trace_id=3c04e02e27e644f2af89b05b02a1b120

Additional Context

According to the W3C baggage spec parsers must adhere to the following.

Note, value MAY contain any number of the equal sign (U+003D) code points. Parsers MUST NOT assume that the equal sign is only used to separate key and value.

Property metadata description

Additional metadata MAY be appended to values in the form of property set, represented as semi-colon ; delimited list of keys and/or key-value pairs, e.g. ;k1=v1;k2;k3=v3. Property keys and values are given no specific meaning by this specification. Leading and trailing OWS is allowed and is not considered to be a part of the property key or value.

https://www.w3.org/TR/baggage/#property

Test cases from w3c baggage github.

    def test_parse_kv_property(self):
        entry = BaggageEntry.from_string(
            "SomeKey=SomeValue;SomePropKey=SomePropValue")
        self.assertEqual(entry.key, "SomeKey")
        self.assertEqual(entry.value, "SomeValue")
        self.assertEqual(len(entry.properties), 1)
        self.assertEqual(entry.properties[0].key, "SomePropKey")
        self.assertEqual(entry.properties[0].value, "SomePropValue")

https://github.com/w3c/baggage/blob/main/test/test_baggage.py#L151-L158

It appears the bug is located in this function:

sentry-javascript/packages/core/src/utils/baggage.ts

Lines 113 to 133 in 379b539

	function baggageHeaderToObject(baggageHeader: string): Record<string, string> {
	return baggageHeader
	.split(',')
	.map(baggageEntry =>
	baggageEntry.split('=').map(keyOrValue => {
	try {
	return decodeURIComponent(keyOrValue.trim());
	} catch {
	// We ignore errors here, e.g. if the value cannot be URL decoded.
	// This will then be skipped in the next step
	return;
	}
	}),
	)
	.reduce<Record<string, string>>((acc, [key, value]) => {
	if (key && value) {
	acc[key] = value;
	}
	return acc;
	}, {});
	}

The reduce function is only taking the first two values after splitting on '='. This is problematic as any key metadata property value will be lost. A corrected function would look something like:

function baggageHeaderToObject(baggageHeader) {
  return baggageHeader
    .split(',')
    .map(baggageEntry => {
      const eqIdx = baggageEntry.indexOf('=');
      if (eqIdx === -1) {
        return [];
      }
      const key = baggageEntry.slice(0, eqIdx);
      const value = baggageEntry.slice(eqIdx + 1);
      return [key, value].map(keyOrValue => {
        try {
          return decodeURIComponent(keyOrValue.trim());
        } catch {
          // We ignore errors here, e.g. if the value cannot be URL decoded.
          // This will then be skipped in the next step
          return;
        }
      });
    })
    .reduce((acc, [key, value]) => {
      if (key && value) {
        acc[key] = value;
      }
      return acc;
    }, {});
}

_{Tip: React with 👍 to help prioritize this issue. Please use comments to provide useful context, avoiding +1 or me too, to help us triage it.}

[linear]

JS-1015

[chargome]

@zandt-square thanks for reporting this + the detailed explanation!
The fix will go out with our next release 🙏

[github-actions]

A PR closing this issue has just been released 🚀

This issue was referenced by PR #17847, which was included in the 10.18.0 release.