Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

baggage: Fix escaping in Member.String #4756

Merged
merged 11 commits into from
Dec 21, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
### Fixed

- Fix `Parse` in `go.opentelemetry.io/otel/baggage` to validate member value before percent-decoding. (#4755)
- Fix whitespace encoding of `Member.String` in `go.opentelemetry.io/otel/baggage`. (#4756)

## [1.21.0/0.44.0] 2023-11-16

Expand Down
6 changes: 4 additions & 2 deletions baggage/baggage.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,10 @@ func (m Member) Properties() []Property { return m.properties.Copy() }
// String encodes Member into a string compliant with the W3C Baggage
// specification.
func (m Member) String() string {
// A key is just an ASCII string, but a value is URL encoded UTF-8.
s := fmt.Sprintf("%s%s%s", m.key, keyValueDelimiter, url.QueryEscape(m.value))
// A key is just an ASCII string. A value is restricted to be
// US-ASCII characters excluding CTLs, whitespace,
// DQUOTE, comma, semicolon, and backslash.
s := fmt.Sprintf("%s%s%s", m.key, keyValueDelimiter, url.PathEscape(m.value))
if len(m.properties) > 0 {
s = fmt.Sprintf("%s%s%s", s, propertyDelimiter, m.properties.String())
}
Expand Down
52 changes: 44 additions & 8 deletions baggage/baggage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,13 @@ func TestBaggageParse(t *testing.T) {
"foo": {Value: "2"},
},
},
{
name: "= value",
in: "key==",
want: baggage.List{
"key": {Value: "="},
},
},
{
name: "url encoded value",
in: "key1=val%252%2C",
Expand Down Expand Up @@ -486,19 +493,46 @@ func TestBaggageString(t *testing.T) {
},
},
{
name: "URL encoded value",
out: "foo=1%3D1",
name: "Encoded value",
// Allowed value characters are:
//
// %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
//
// Meaning, US-ASCII characters excluding CTLs, whitespace,
// DQUOTE, comma, semicolon, and backslash. All excluded
// characters need to be percent encoded.
//
// Ideally, the want result is:
// out: "foo=%00%01%02%03%04%05%06%07%08%09%0A%0B%0C%0D%0E%0F%10%11%12%13%14%15%16%17%18%19%1A%1B%1C%1D%1E%1F%20!%22#$%25&'()*+%2C-./0123456789:%3B<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[%5C]^_%60abcdefghijklmnopqrstuvwxyz{|}~%7F",
// However, the following characters are escaped:
// !#'()*/<>?[]^{|}
// It is not necessary, but still provides a correct result.
out: "foo=%00%01%02%03%04%05%06%07%08%09%0A%0B%0C%0D%0E%0F%10%11%12%13%14%15%16%17%18%19%1A%1B%1C%1D%1E%1F%20%21%22%23$%25&%27%28%29%2A+%2C-.%2F0123456789:%3B%3C=%3E%3F@ABCDEFGHIJKLMNOPQRSTUVWXYZ%5B%5C%5D%5E_%60abcdefghijklmnopqrstuvwxyz%7B%7C%7D~%7F",
baggage: baggage.List{
"foo": {Value: "1=1"},
"foo": {Value: func() string {
// All US-ASCII characters.
b := [128]byte{}
for i := range b {
b[i] = byte(i)
}
return string(b[:])
}()},
},
},
{
name: "plus",
out: "foo=1%2B1",
out: "foo=1+1",
baggage: baggage.List{
"foo": {Value: "1+1"},
},
},
{
name: "equal",
out: "foo=1=1",
baggage: baggage.List{
"foo": {Value: "1=1"},
},
},
{
name: "single member empty value with properties",
out: "foo=;red;state=on",
Expand Down Expand Up @@ -561,8 +595,10 @@ func TestBaggageString(t *testing.T) {
}

for _, tc := range testcases {
b := Baggage{tc.baggage}
assert.Equal(t, tc.out, orderer(b.String()))
t.Run(tc.name, func(t *testing.T) {
b := Baggage{tc.baggage}
assert.Equal(t, tc.out, orderer(b.String()))
})
}
}

Expand Down Expand Up @@ -863,9 +899,9 @@ func TestMemberString(t *testing.T) {
memberStr := member.String()
assert.Equal(t, memberStr, "key=value")
// encoded key
member, _ = NewMember("key", "%3B")
member, _ = NewMember("key", "%3B%20")
memberStr = member.String()
assert.Equal(t, memberStr, "key=%3B")
assert.Equal(t, memberStr, "key=%3B%20")
}

var benchBaggage Baggage
Expand Down
4 changes: 2 additions & 2 deletions propagation/baggage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,9 @@ func TestInjectBaggageToHTTPReq(t *testing.T) {
{
name: "values with escaped chars",
mems: members{
{Key: "key2", Value: "val3=4"},
{Key: "key2", Value: "val3,4"},
},
wantInHeader: []string{"key2=val3%3D4"},
wantInHeader: []string{"key2=val3%2C4"},
},
{
name: "with properties",
Expand Down