Error while parsing Baggage header with percent-encoded whitespace

[tonyo]

Description

At the moment baggage.Parse fails when trying to decode a value with percent-encoded whitespace "%20" (and potentially others?).

Related issue in "opentelemetry-python" that has a pending fix: https://github.com/open-telemetry/opentelemetry-python/issues/2934

A test case in "opentelemetry-js" that hints that whitespaces should be properly handled when encoding and decoding baggage: https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-core/test/baggage/W3CBaggagePropagator.test.ts#L60

Might be also related to https://github.com/open-telemetry/opentelemetry-go/issues/3467.

Environment

• Go Version: 1.19
• opentelemetry-go version: v1.11.2

Steps To Reproduce

The following code returns the error: invalid value: "val1 val2"

val, err := baggage.Parse("key1=val1%20val2")

https://go.dev/play/p/2kLO3RFzH_R

Expected behavior

key1=val1%20val2 should be properly parsed into a baggage item with key "key1" and value "val1 val2" (at least that's my understanding).

[tonyo]

After looking into this a bit more, I believe here are some issues with the current implementation:

1. Baggage string value one%20two should be properly parsed as "one two"
2. Baggage string value one+two should be parsed as "one+two"
3. Go string value "one two" should be encoded as one%20two (percent encoding), and NOT as one+two (URL query encoding).
4. Go string value "1=1" might be encoded as 1=1 (the spec says: "Note, value MAY contain any number of the equal sign (=) characters. Parsers MUST NOT assume that the equal sign is only used to separate key and value."). 1%3D1 is also valid, but to simplify the implementation we don't have to do extra encoding.

This is our take on fixing these issues: https://github.com/getsentry/sentry-go/pull/568

[n-oden]

@tonyo I believe I've figured out at least the decoding side of this: https://github.com/open-telemetry/opentelemetry-go/pull/4667

I suspect you're correct about the encoding side as well, but swapping out url.QueryEncode with url.PathEncode broke a bunch of the unit tests, and I'm leery of breaking existing behavior.

[pellared]

@tonyo Can you please double-check if releases starting from https://github.com/open-telemetry/opentelemetry-go/releases/tag/v1.20.0 are solving the issue you described?