W3C Baggage propagator uses form encoding instead of percent encoding

.changelog/5561.fixed

@@ -0,0 +1 @@
+`opentelemetry-api`: percent-encode W3C Baggage instead of form-encoding it, so a literal `+` round trips and spaces are sent as `%20`

opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py

@@ -4,7 +4,7 @@
 from collections.abc import Iterable, Iterator, Mapping
 from logging import getLogger
 from re import split
-from urllib.parse import quote_plus, unquote_plus
+from urllib.parse import quote, unquote
 
 from opentelemetry.baggage import _is_valid_pair, get_all, set_baggage
 from opentelemetry.context import get_current
@@ -115,8 +115,8 @@ def extract(
                 _logger.warning("Invalid baggage entry: `%s`", entry)
                 continue
 
-            name = unquote_plus(name).strip()
-            value = unquote_plus(value).strip()
+            name = unquote(name).strip()
+            value = unquote(value).strip()
 
             context = set_baggage(
                 name,
@@ -162,9 +162,14 @@ def fields(self) -> set[str]:
 def _encode_baggage_pairs(
     baggage_entries: Mapping[str, object],
 ) -> Iterator[str]:
-    """Yield URL-encoded 'key=value' pairs from baggage entries."""
+    """Yield percent-encoded 'key=value' pairs from baggage entries.
+
+    Percent-encoding (RFC 3986), not form encoding: the W3C Baggage grammar
+    lists "+" as an ordinary baggage-octet, so it must not be used to stand in
+    for a space. https://www.w3.org/TR/baggage/
+    """
     for key, value in baggage_entries.items():
-        yield quote_plus(str(key)) + "=" + quote_plus(str(value))
+        yield quote(str(key), safe="") + "=" + quote(str(value), safe="")
 
 
 def _extract_first_element(

opentelemetry-api/tests/propagators/test_w3cbaggagepropagator.py

@@ -6,10 +6,12 @@
 from logging import WARNING
 from unittest import TestCase
 from unittest.mock import Mock, patch
-from urllib.parse import quote_plus
 
 from opentelemetry.baggage import get_all, set_baggage
-from opentelemetry.baggage.propagation import W3CBaggagePropagator
+from opentelemetry.baggage.propagation import (
+    W3CBaggagePropagator,
+    _encode_baggage_pairs,
+)
 from opentelemetry.context import get_current
 
 
@@ -186,7 +188,7 @@ def test_inject_no_baggage_entries(self):
         self.assertEqual(None, output)
 
     def test_inject_space_entries(self):
-        self.assertEqual("key=val+ue", self._inject({"key": "val ue"}))
+        self.assertEqual("key=val%20ue", self._inject({"key": "val ue"}))
 
     def test_inject(self):
         values = {
@@ -232,14 +234,50 @@ def test_fields(self, mock_baggage):
 
     def test_encode_baggage_pairs(self):
         def _format_baggage(entries):
-            return ",".join(quote_plus(str(k)) + "=" + quote_plus(str(v)) for k, v in entries.items())
+            return ",".join(_encode_baggage_pairs(entries))
 
-        self.assertEqual(_format_baggage({"key key": "value value"}), "key+key=value+value")
+        self.assertEqual(
+            _format_baggage({"key key": "value value"}),
+            "key%20key=value%20value",
+        )
         self.assertEqual(
             _format_baggage({"key/key": "value/value"}),
             "key%2Fkey=value%2Fvalue",
         )
 
+    # The W3C Baggage grammar defines
+    #   baggage-octet = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
+    # "+" is %x2B, an ordinary literal that must survive a round trip, and
+    # SP (%x20) is not a baggage-octet so it must be percent-encoded.
+    # https://www.w3.org/TR/baggage/
+
+    def test_extract_preserves_literal_plus(self):
+        """A "+" on the wire is a literal "+", never a space."""
+        self.assertEqual(self._extract("key=a+b"), {"key": "a+b"})
+
+    def test_extract_preserves_plus_only_value(self):
+        """A value that is only "+" must not decode to an empty string."""
+        self.assertEqual(self._extract("key=+"), {"key": "+"})
+
+    def test_extract_preserves_repeated_plus(self):
+        self.assertEqual(self._extract("key=c++"), {"key": "c++"})
+
+    def test_extract_preserves_plus_in_key(self):
+        self.assertEqual(self._extract("a+b=value"), {"a+b": "value"})
+
+    def test_extract_percent_encoded_space(self):
+        """%20 is the spec-correct wire form for a space."""
+        self.assertEqual(self._extract("key=a%20b"), {"key": "a b"})
+
+    def test_inject_percent_encodes_space(self):
+        self.assertEqual("key=a%20b", self._inject({"key": "a b"}))
+
+    def test_inject_extract_round_trips_plus(self):
+        self.assertEqual(self._extract(self._inject({"key": "a+b"})), {"key": "a+b"})
+
+    def test_inject_extract_round_trips_space(self):
+        self.assertEqual(self._extract(self._inject({"key": "a b"})), {"key": "a b"})
+
     def test_inject_too_many_entries(self):
         """Inject should drop entries exceeding _MAX_PAIRS."""
         values = {f"key{i}": f"val{i}" for i in range(self.propagator._MAX_PAIRS + 10)}
@@ -320,6 +358,6 @@ def test_inject_extract(self):
 
         context = self.propagator.extract(carrier)
 
-        self.assertEqual(carrier, {"baggage": "transaction=string+with+spaces"})
+        self.assertEqual(carrier, {"baggage": "transaction=string%20with%20spaces"})
 
         self.assertEqual(context, {"abc": {"transaction": "string with spaces"}})