Skip to content

Commit

Permalink
baggage: Fix escaping in Member.String (#4756)
Browse files Browse the repository at this point in the history
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
  • Loading branch information
pellared and MrAlias authored Dec 21, 2023
1 parent e3bf787 commit 885210b
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 12 deletions.
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

0 comments on commit 885210b

Please sign in to comment.