From bdb9322ebd4aadeba9c769326bd2381c3b47ef38 Mon Sep 17 00:00:00 2001
From: Nathan J Mehl <70606471+n-oden@users.noreply.github.com>
Date: Tue, 31 Oct 2023 18:02:17 -0400
Subject: [PATCH] Use url.PathUnescape rather than url.QueryUnescape in baggage
 parsing (#4667)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Use url.PathUnescape rather than url.QueryUnescape

I believe this addresses the majority of the cases described in
https://github.com/open-telemetry/opentelemetry-go/issues/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>
---
 CHANGELOG.md            |  1 +
 baggage/baggage.go      |  4 ++--
 baggage/baggage_test.go | 49 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 97b79b2b4..5261ba19d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
diff --git a/baggage/baggage.go b/baggage/baggage.go
index 9e6b3b7b5..84532cb1d 100644
--- a/baggage/baggage.go
+++ b/baggage/baggage.go
@@ -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)
 	}
@@ -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)
 	}
diff --git a/baggage/baggage_test.go b/baggage/baggage_test.go
index 2b98beace..4bac6707e 100644
--- a/baggage/baggage_test.go
+++ b/baggage/baggage_test.go
@@ -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 ",
@@ -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",