From a36a6152691c37eb2a9bf5d5db182188e2b5017f Mon Sep 17 00:00:00 2001
From: Diego Hurtado <ocelotl@users.noreply.github.com>
Date: Tue, 31 Aug 2021 14:07:45 +0200
Subject: [PATCH] Escape reserved characters in baggage keys (#2080)

* Escape reserved characters in baggage keys

Fixes #2072

* Add extract tests

* Fix lint
---
 .../baggage/propagation/__init__.py           | 14 +++++------
 .../tests/baggage/test_baggage_propagation.py | 25 +++++++++++++++++--
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py
index 04d896baa3..b484d9b2e8 100644
--- a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py
+++ b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py
@@ -13,9 +13,9 @@
 # limitations under the License.
 #
 import typing
-import urllib.parse
+from urllib.parse import quote_plus, unquote_plus
 
-from opentelemetry import baggage
+from opentelemetry.baggage import get_all, set_baggage
 from opentelemetry.context import get_current
 from opentelemetry.context.context import Context
 from opentelemetry.propagators import textmap
@@ -63,9 +63,9 @@ def extract(
                 name, value = entry.split("=", 1)
             except Exception:  # pylint: disable=broad-except
                 continue
-            context = baggage.set_baggage(
-                urllib.parse.unquote(name).strip(),
-                urllib.parse.unquote(value).strip(),
+            context = set_baggage(
+                unquote_plus(name).strip(),
+                unquote_plus(value).strip(),
                 context=context,
             )
 
@@ -82,7 +82,7 @@ def inject(
         See
         `opentelemetry.propagators.textmap.TextMapPropagator.inject`
         """
-        baggage_entries = baggage.get_all(context=context)
+        baggage_entries = get_all(context=context)
         if not baggage_entries:
             return
 
@@ -97,7 +97,7 @@ def fields(self) -> typing.Set[str]:
 
 def _format_baggage(baggage_entries: typing.Mapping[str, object]) -> str:
     return ",".join(
-        key + "=" + urllib.parse.quote_plus(str(value))
+        quote_plus(str(key)) + "=" + quote_plus(str(value))
         for key, value in baggage_entries.items()
     )
 
diff --git a/opentelemetry-api/tests/baggage/test_baggage_propagation.py b/opentelemetry-api/tests/baggage/test_baggage_propagation.py
index 9084bb778e..009598db9f 100644
--- a/opentelemetry-api/tests/baggage/test_baggage_propagation.py
+++ b/opentelemetry-api/tests/baggage/test_baggage_propagation.py
@@ -18,7 +18,10 @@
 from unittest.mock import Mock, patch
 
 from opentelemetry import baggage
-from opentelemetry.baggage.propagation import W3CBaggagePropagator
+from opentelemetry.baggage.propagation import (
+    W3CBaggagePropagator,
+    _format_baggage,
+)
 from opentelemetry.context import get_current
 
 
@@ -106,6 +109,15 @@ def test_header_contains_pair_too_long(self):
         expected = {"key1": "value1", "key3": "value3"}
         self.assertEqual(self._extract(header), expected)
 
+    def test_extract_unquote_plus(self):
+        self.assertEqual(
+            self._extract("key+key=value+value"), {"key key": "value value"}
+        )
+        self.assertEqual(
+            self._extract("key%2Fkey=value%2Fvalue"),
+            {"key/key": "value/value"},
+        )
+
     def test_inject_no_baggage_entries(self):
         values = {}
         output = self._inject(values)
@@ -140,7 +152,7 @@ def test_inject_non_string_values(self):
         self.assertIn("key2=123", output)
         self.assertIn("key3=123.567", output)
 
-    @patch("opentelemetry.baggage.propagation.baggage")
+    @patch("opentelemetry.baggage.propagation.get_all")
     @patch("opentelemetry.baggage.propagation._format_baggage")
     def test_fields(self, mock_format_baggage, mock_baggage):
 
@@ -154,3 +166,12 @@ def test_fields(self, mock_format_baggage, mock_baggage):
             inject_fields.add(mock_call[1][1])
 
         self.assertEqual(inject_fields, self.propagator.fields)
+
+    def test__format_baggage(self):
+        self.assertEqual(
+            _format_baggage({"key key": "value value"}), "key+key=value+value"
+        )
+        self.assertEqual(
+            _format_baggage({"key/key": "value/value"}),
+            "key%2Fkey=value%2Fvalue",
+        )