Skip to content

Commit

Permalink
Escape reserved characters in baggage keys (open-telemetry#2080)
Browse files Browse the repository at this point in the history
* Escape reserved characters in baggage keys

Fixes open-telemetry#2072

* Add extract tests

* Fix lint
  • Loading branch information
ocelotl committed Aug 31, 2021
1 parent b4ebef9 commit a36a615
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
)

Expand All @@ -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

Expand All @@ -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()
)

Expand Down
25 changes: 23 additions & 2 deletions opentelemetry-api/tests/baggage/test_baggage_propagation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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):

Expand All @@ -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",
)

0 comments on commit a36a615

Please sign in to comment.