From 304e2eee764a23e4c5c0a53b8e8a54ded50d4646 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 30 Aug 2021 14:29:05 +0200 Subject: [PATCH 1/3] Escape reserved characters in baggage keys Fixes #2072 --- .../opentelemetry/baggage/propagation/__init__.py | 8 ++++---- .../tests/baggage/test_baggage_propagation.py | 14 +++++++++++++- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py index 04d896baa3..bc169bd6e8 100644 --- a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py +++ b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py @@ -13,7 +13,7 @@ # limitations under the License. # import typing -import urllib.parse +from urllib.parse import quote_plus, unquote from opentelemetry import baggage from opentelemetry.context import get_current @@ -64,8 +64,8 @@ def extract( except Exception: # pylint: disable=broad-except continue context = baggage.set_baggage( - urllib.parse.unquote(name).strip(), - urllib.parse.unquote(value).strip(), + unquote(name).strip(), + unquote(value).strip(), context=context, ) @@ -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..ebac00d467 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 @@ -154,3 +157,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", + ) From 86a0dd649329e3f662d44ac21e1cde0465d519ec Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 31 Aug 2021 12:24:13 +0200 Subject: [PATCH 2/3] Add extract tests --- .../opentelemetry/baggage/propagation/__init__.py | 12 ++++++------ .../tests/baggage/test_baggage_propagation.py | 12 +++++++++++- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py index bc169bd6e8..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 -from urllib.parse import quote_plus, unquote +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( - unquote(name).strip(), - 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 diff --git a/opentelemetry-api/tests/baggage/test_baggage_propagation.py b/opentelemetry-api/tests/baggage/test_baggage_propagation.py index ebac00d467..205d066b15 100644 --- a/opentelemetry-api/tests/baggage/test_baggage_propagation.py +++ b/opentelemetry-api/tests/baggage/test_baggage_propagation.py @@ -109,6 +109,16 @@ 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) @@ -143,7 +153,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): From 189fc5c32ebe1712a3653354cdf2aa587cc03ac1 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 31 Aug 2021 12:40:50 +0200 Subject: [PATCH 3/3] Fix lint --- opentelemetry-api/tests/baggage/test_baggage_propagation.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/opentelemetry-api/tests/baggage/test_baggage_propagation.py b/opentelemetry-api/tests/baggage/test_baggage_propagation.py index 205d066b15..009598db9f 100644 --- a/opentelemetry-api/tests/baggage/test_baggage_propagation.py +++ b/opentelemetry-api/tests/baggage/test_baggage_propagation.py @@ -111,12 +111,11 @@ def test_header_contains_pair_too_long(self): def test_extract_unquote_plus(self): self.assertEqual( - self._extract("key+key=value+value"), - {"key key": "value value"} + self._extract("key+key=value+value"), {"key key": "value value"} ) self.assertEqual( self._extract("key%2Fkey=value%2Fvalue"), - {"key/key": "value/value"} + {"key/key": "value/value"}, ) def test_inject_no_baggage_entries(self):