Do not count skipped baggage entries

Fixes open-telemetry#2071
ocelotl · Aug 30, 2021 · e39f80b · e39f80b
1 parent c3dfa36
commit e39f80b
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.5.0-0.24b0...HEAD)
 
+- Fix propagation bug caused by counting skipped entries
+  ([#2071](https://github.com/open-telemetry/opentelemetry-python/pull/2071))
+
 ## [1.5.0-0.24b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.5.0-0.24b0) - 2021-08-26
 
 

diff --git a/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/baggage/propagation/__init__.py
@@ -54,9 +54,6 @@ def extract(
         baggage_entries = header.split(",")
         total_baggage_entries = self._MAX_PAIRS
         for entry in baggage_entries:
-            if total_baggage_entries <= 0:
-                return context
-            total_baggage_entries -= 1
             if len(entry) > self._MAX_PAIR_LENGTH:
                 continue
             try:
@@ -68,6 +65,9 @@ def extract(
                 urllib.parse.unquote(value).strip(),
                 context=context,
             )
+            total_baggage_entries -= 1
+            if total_baggage_entries == 0:
+                break
 
         return context
 

diff --git a/opentelemetry-api/tests/baggage/test_baggage_propagation.py b/opentelemetry-api/tests/baggage/test_baggage_propagation.py
@@ -106,6 +106,29 @@ def test_header_contains_pair_too_long(self):
         expected = {"key1": "value1", "key3": "value3"}
         self.assertEqual(self._extract(header), expected)
 
+    def test_header_max_entries_skip_invalid_entry(self):
+
+        self.assertEqual(
+            self._extract(
+                ",".join(
+                    [
+                        f"key{index}=value{index}"
+                        if index != 2
+                        else (
+                            f"key{index}="
+                            f"value{'s' * (W3CBaggagePropagator._MAX_PAIR_LENGTH + 1)}"
+                        )
+                        for index in range(W3CBaggagePropagator._MAX_PAIRS + 1)
+                    ]
+                )
+            ),
+            {
+                f"key{index}": f"value{index}"
+                for index in range(W3CBaggagePropagator._MAX_PAIRS + 1)
+                if index != 2
+            },
+        )
+
     def test_inject_no_baggage_entries(self):
         values = {}
         output = self._inject(values)