Skip to content

Commit

Permalink
Use url.PathUnescape rather than url.QueryUnescape in baggage parsing (
Browse files Browse the repository at this point in the history
…#4667)

* Use url.PathUnescape rather than url.QueryUnescape

I believe this addresses the majority of the cases described in
#3601

Golang's url.QueryUnescape will render url _path_ elements (e.g. /, +)
as spaces: `foo+bar` is rendered as `foo bar`.  Path elements are (as I
read the spec) legal W3C baggage values, and replacing them with spaces
fails the value validation regex.

url.PathEscape allows path elements through unmolested.

Signed-off-by: Nathan J. Mehl <n@oden.io>

* Update CHANGELOG.md

address comments

Co-authored-by: Robert Pająk <pellared@hotmail.com>

---------

Signed-off-by: Nathan J. Mehl <n@oden.io>
Co-authored-by: Robert Pająk <pellared@hotmail.com>
  • Loading branch information
n-oden and pellared committed Oct 31, 2023
1 parent ce7b40a commit bdb9322
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### Fixed

- Fix improper parsing of characters such us `+`, `\` by `Parse` in `go.opentelemetry.io/otel/baggage` as they were rendered as a whitespace. (#4667)
- In `go.opentelemetry.op/otel/exporters/prometheus`, the exporter no longer `Collect`s metrics after `Shutdown` is invoked. (#4648)

## [1.19.0/0.42.0/0.0.7] 2023-09-28
Expand Down
4 changes: 2 additions & 2 deletions baggage/baggage.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ func NewMember(key, value string, props ...Property) (Member, error) {
if err := m.validate(); err != nil {
return newInvalidMember(), err
}
decodedValue, err := url.QueryUnescape(value)
decodedValue, err := url.PathUnescape(value)
if err != nil {
return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidValue, value)
}
Expand Down Expand Up @@ -301,7 +301,7 @@ func parseMember(member string) (Member, error) {
// when converting the header into a data structure."
key = strings.TrimSpace(k)
var err error
value, err = url.QueryUnescape(strings.TrimSpace(v))
value, err = url.PathUnescape(strings.TrimSpace(v))
if err != nil {
return newInvalidMember(), fmt.Errorf("%w: %q", err, value)
}
Expand Down
49 changes: 49 additions & 0 deletions baggage/baggage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,48 @@ func TestBaggageParse(t *testing.T) {
"foo": {Value: "1"},
},
},
{
name: "single member no properties plus",
in: "foo=1+1",
want: baggage.List{
"foo": {Value: "1+1"},
},
},
{
name: "single member no properties plus encoded",
in: "foo=1%2B1",
want: baggage.List{
"foo": {Value: "1+1"},
},
},
{
name: "single member no properties slash",
in: "foo=1/1",
want: baggage.List{
"foo": {Value: "1/1"},
},
},
{
name: "single member no properties slash encoded",
in: "foo=1%2F1",
want: baggage.List{
"foo": {Value: "1/1"},
},
},
{
name: "single member no properties equals",
in: "foo=1=1",
want: baggage.List{
"foo": {Value: "1=1"},
},
},
{
name: "single member no properties equals encoded",
in: "foo=1%3D1",
want: baggage.List{
"foo": {Value: "1=1"},
},
},
{
name: "single member with spaces",
in: " foo \t= 1\t\t ",
Expand Down Expand Up @@ -440,6 +482,13 @@ func TestBaggageString(t *testing.T) {
"foo": {Value: "1=1"},
},
},
{
name: "plus",
out: "foo=1%2B1",
baggage: baggage.List{
"foo": {Value: "1+1"},
},
},
{
name: "single member empty value with properties",
out: "foo=;red;state=on",
Expand Down

0 comments on commit bdb9322

Please sign in to comment.