From 91b89a2ad58aa3e4bd719f1a71ab53a6b1c0401f Mon Sep 17 00:00:00 2001 From: Mateusz Lach Date: Thu, 1 Feb 2024 12:54:48 +0100 Subject: [PATCH 1/8] fix encoding of baggage item values --- .../Context/Propagation/BaggagePropagator.cs | 6 ++-- .../Propagation/BaggagePropagatorTest.cs | 28 ++++++++++++++++--- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs b/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs index 57bdb453b8f..1aec08c2bd0 100644 --- a/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs +++ b/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs @@ -94,7 +94,7 @@ public override void Inject(PropagationContext context, T carrier, Action> { new KeyValuePair(BaggagePropagator.BaggageHeaderName, initialBaggage), @@ -142,11 +144,11 @@ public void ValidateSpecialCharsBaggageExtraction() Assert.False(propagationContext == default); Assert.True(propagationContext.ActivityContext == default); - Assert.Equal(3, propagationContext.Baggage.Count); + Assert.Equal(6, propagationContext.Baggage.Count); var actualBaggage = propagationContext.Baggage.GetBaggage(); - Assert.Equal(3, actualBaggage.Count); + Assert.Equal(6, actualBaggage.Count); Assert.True(actualBaggage.ContainsKey("key 1")); Assert.Equal("value 1", actualBaggage["key 1"]); @@ -156,6 +158,18 @@ public void ValidateSpecialCharsBaggageExtraction() Assert.True(actualBaggage.ContainsKey("key()3")); Assert.Equal("value()!&;:", actualBaggage["key()3"]); + + // x20-x7E range + Assert.True(actualBaggage.ContainsKey("key4")); + Assert.Equal(" !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~", actualBaggage["key4"]); + + // non-ASCII characters + Assert.True(actualBaggage.ContainsKey("key5")); + Assert.Equal("ąść", actualBaggage["key5"]); + + // value contains '=' character + Assert.True(actualBaggage.ContainsKey("key6")); + Assert.Equal("1=1", actualBaggage["key6"]); } [Fact] @@ -195,11 +209,17 @@ public void ValidateSpecialCharsBaggageInjection() { { "key 1", "value 1" }, { "key2", "!x_x,x-x&x(x\");:" }, + // x20-x7E range + { "key3", " !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~" }, + // non-ASCII + { "key4", "ąść" }, + // '=' char in value + { "key5", "1=1" }, })); this.baggage.Inject(propagationContext, carrier, Setter); Assert.Single(carrier); - Assert.Equal("key+1=value+1,key2=!x_x%2Cx-x%26x(x%22)%3B%3A", carrier[BaggagePropagator.BaggageHeaderName]); + Assert.Equal("key%201=value%201,key2=%21x_x%2Cx-x%26x%28x%22%29%3B%3A,key3=%20%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F0123456789%3A%3B%3C%3D%3E%3F%40ABCDEFGHIJKLMNOPQRSTUVWXYZ%5B%5C%5D%5E_%60abcdefghijklmnopqrstuvwxyz%7B%7C%7D~,key4=%C4%85%C5%9B%C4%87,key5=1%3D1", carrier[BaggagePropagator.BaggageHeaderName]); } } From 0680ceb4eefcead800c2bb03d044329e655c79bf Mon Sep 17 00:00:00 2001 From: Mateusz Lach Date: Thu, 1 Feb 2024 13:24:13 +0100 Subject: [PATCH 2/8] remove redundant using --- src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs b/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs index 1aec08c2bd0..e9dc29d950d 100644 --- a/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs +++ b/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs @@ -1,7 +1,6 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -using System.Net; using System.Text; using OpenTelemetry.Internal; From b15ef6f09ca47205173475d74dfa3ea9df19c464 Mon Sep 17 00:00:00 2001 From: Mateusz Lach Date: Thu, 1 Feb 2024 13:30:05 +0100 Subject: [PATCH 3/8] add required blanklines --- .../Trace/Propagation/BaggagePropagatorTest.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/OpenTelemetry.Api.Tests/Trace/Propagation/BaggagePropagatorTest.cs b/test/OpenTelemetry.Api.Tests/Trace/Propagation/BaggagePropagatorTest.cs index 70a893b426e..bdbf9bfa1cc 100644 --- a/test/OpenTelemetry.Api.Tests/Trace/Propagation/BaggagePropagatorTest.cs +++ b/test/OpenTelemetry.Api.Tests/Trace/Propagation/BaggagePropagatorTest.cs @@ -209,10 +209,13 @@ public void ValidateSpecialCharsBaggageInjection() { { "key 1", "value 1" }, { "key2", "!x_x,x-x&x(x\");:" }, + // x20-x7E range { "key3", " !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~" }, + // non-ASCII { "key4", "ąść" }, + // '=' char in value { "key5", "1=1" }, })); From d244e4d85b6db6701dc88cba6a6f1f4414acb7af Mon Sep 17 00:00:00 2001 From: Mateusz Lach Date: Mon, 5 Feb 2024 12:29:42 +0100 Subject: [PATCH 4/8] drop value with nonascii chars --- .../Propagation/BaggagePropagatorTest.cs | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/test/OpenTelemetry.Api.Tests/Trace/Propagation/BaggagePropagatorTest.cs b/test/OpenTelemetry.Api.Tests/Trace/Propagation/BaggagePropagatorTest.cs index bdbf9bfa1cc..1ddf5c0ee14 100644 --- a/test/OpenTelemetry.Api.Tests/Trace/Propagation/BaggagePropagatorTest.cs +++ b/test/OpenTelemetry.Api.Tests/Trace/Propagation/BaggagePropagatorTest.cs @@ -132,7 +132,7 @@ public void ValidateSpecialCharsBaggageExtraction() Assert.Equal("value%28%29%21%26%3B%3A", escapedValue); var initialBaggage = - $"key%201=value%201,{encodedKey}={encodedValue},{escapedKey}={escapedValue},key4=%20%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F0123456789%3A%3B%3C%3D%3E%3F%40ABCDEFGHIJKLMNOPQRSTUVWXYZ%5B%5C%5D%5E_%60abcdefghijklmnopqrstuvwxyz%7B%7C%7D~,key5=%C4%85%C5%9B%C4%87,key6=1%3D1"; + $"key%201=value%201,{encodedKey}={encodedValue},{escapedKey}={escapedValue},key4=%20%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F0123456789%3A%3B%3C%3D%3E%3F%40ABCDEFGHIJKLMNOPQRSTUVWXYZ%5B%5C%5D%5E_%60abcdefghijklmnopqrstuvwxyz%7B%7C%7D~,key5=1%3D1"; var carrier = new List> { @@ -144,11 +144,11 @@ public void ValidateSpecialCharsBaggageExtraction() Assert.False(propagationContext == default); Assert.True(propagationContext.ActivityContext == default); - Assert.Equal(6, propagationContext.Baggage.Count); + Assert.Equal(5, propagationContext.Baggage.Count); var actualBaggage = propagationContext.Baggage.GetBaggage(); - Assert.Equal(6, actualBaggage.Count); + Assert.Equal(5, actualBaggage.Count); Assert.True(actualBaggage.ContainsKey("key 1")); Assert.Equal("value 1", actualBaggage["key 1"]); @@ -163,13 +163,9 @@ public void ValidateSpecialCharsBaggageExtraction() Assert.True(actualBaggage.ContainsKey("key4")); Assert.Equal(" !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~", actualBaggage["key4"]); - // non-ASCII characters - Assert.True(actualBaggage.ContainsKey("key5")); - Assert.Equal("ąść", actualBaggage["key5"]); - // value contains '=' character - Assert.True(actualBaggage.ContainsKey("key6")); - Assert.Equal("1=1", actualBaggage["key6"]); + Assert.True(actualBaggage.ContainsKey("key5")); + Assert.Equal("1=1", actualBaggage["key5"]); } [Fact] @@ -213,16 +209,13 @@ public void ValidateSpecialCharsBaggageInjection() // x20-x7E range { "key3", " !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~" }, - // non-ASCII - { "key4", "ąść" }, - // '=' char in value - { "key5", "1=1" }, + { "key4", "1=1" }, })); this.baggage.Inject(propagationContext, carrier, Setter); Assert.Single(carrier); - Assert.Equal("key%201=value%201,key2=%21x_x%2Cx-x%26x%28x%22%29%3B%3A,key3=%20%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F0123456789%3A%3B%3C%3D%3E%3F%40ABCDEFGHIJKLMNOPQRSTUVWXYZ%5B%5C%5D%5E_%60abcdefghijklmnopqrstuvwxyz%7B%7C%7D~,key4=%C4%85%C5%9B%C4%87,key5=1%3D1", carrier[BaggagePropagator.BaggageHeaderName]); + Assert.Equal("key%201=value%201,key2=%21x_x%2Cx-x%26x%28x%22%29%3B%3A,key3=%20%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F0123456789%3A%3B%3C%3D%3E%3F%40ABCDEFGHIJKLMNOPQRSTUVWXYZ%5B%5C%5D%5E_%60abcdefghijklmnopqrstuvwxyz%7B%7C%7D~,key4=1%3D1", carrier[BaggagePropagator.BaggageHeaderName]); } } From 5373ae2575efc064281605e34e3bf4b471c27062 Mon Sep 17 00:00:00 2001 From: Mateusz Lach Date: Mon, 5 Feb 2024 13:05:41 +0100 Subject: [PATCH 5/8] tests simplification --- .../Trace/Propagation/BaggagePropagatorTest.cs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/test/OpenTelemetry.Api.Tests/Trace/Propagation/BaggagePropagatorTest.cs b/test/OpenTelemetry.Api.Tests/Trace/Propagation/BaggagePropagatorTest.cs index 1ddf5c0ee14..9d8a8972586 100644 --- a/test/OpenTelemetry.Api.Tests/Trace/Propagation/BaggagePropagatorTest.cs +++ b/test/OpenTelemetry.Api.Tests/Trace/Propagation/BaggagePropagatorTest.cs @@ -132,7 +132,7 @@ public void ValidateSpecialCharsBaggageExtraction() Assert.Equal("value%28%29%21%26%3B%3A", escapedValue); var initialBaggage = - $"key%201=value%201,{encodedKey}={encodedValue},{escapedKey}={escapedValue},key4=%20%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F0123456789%3A%3B%3C%3D%3E%3F%40ABCDEFGHIJKLMNOPQRSTUVWXYZ%5B%5C%5D%5E_%60abcdefghijklmnopqrstuvwxyz%7B%7C%7D~,key5=1%3D1"; + $"key%201=value%201,{encodedKey}={encodedValue},{escapedKey}={escapedValue},key4=%20%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F0123456789%3A%3B%3C%3D%3E%3F%40ABCDEFGHIJKLMNOPQRSTUVWXYZ%5B%5C%5D%5E_%60abcdefghijklmnopqrstuvwxyz%7B%7C%7D~"; var carrier = new List> { @@ -144,11 +144,11 @@ public void ValidateSpecialCharsBaggageExtraction() Assert.False(propagationContext == default); Assert.True(propagationContext.ActivityContext == default); - Assert.Equal(5, propagationContext.Baggage.Count); + Assert.Equal(4, propagationContext.Baggage.Count); var actualBaggage = propagationContext.Baggage.GetBaggage(); - Assert.Equal(5, actualBaggage.Count); + Assert.Equal(4, actualBaggage.Count); Assert.True(actualBaggage.ContainsKey("key 1")); Assert.Equal("value 1", actualBaggage["key 1"]); @@ -162,10 +162,6 @@ public void ValidateSpecialCharsBaggageExtraction() // x20-x7E range Assert.True(actualBaggage.ContainsKey("key4")); Assert.Equal(" !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~", actualBaggage["key4"]); - - // value contains '=' character - Assert.True(actualBaggage.ContainsKey("key5")); - Assert.Equal("1=1", actualBaggage["key5"]); } [Fact] @@ -208,14 +204,11 @@ public void ValidateSpecialCharsBaggageInjection() // x20-x7E range { "key3", " !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~" }, - - // '=' char in value - { "key4", "1=1" }, })); this.baggage.Inject(propagationContext, carrier, Setter); Assert.Single(carrier); - Assert.Equal("key%201=value%201,key2=%21x_x%2Cx-x%26x%28x%22%29%3B%3A,key3=%20%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F0123456789%3A%3B%3C%3D%3E%3F%40ABCDEFGHIJKLMNOPQRSTUVWXYZ%5B%5C%5D%5E_%60abcdefghijklmnopqrstuvwxyz%7B%7C%7D~,key4=1%3D1", carrier[BaggagePropagator.BaggageHeaderName]); + Assert.Equal("key%201=value%201,key2=%21x_x%2Cx-x%26x%28x%22%29%3B%3A,key3=%20%21%22%23%24%25%26%27%28%29%2A%2B%2C-.%2F0123456789%3A%3B%3C%3D%3E%3F%40ABCDEFGHIJKLMNOPQRSTUVWXYZ%5B%5C%5D%5E_%60abcdefghijklmnopqrstuvwxyz%7B%7C%7D~", carrier[BaggagePropagator.BaggageHeaderName]); } } From e47218aa049c26f96502bcb1bfebb70ba830cd95 Mon Sep 17 00:00:00 2001 From: Mateusz Lach Date: Fri, 5 Apr 2024 08:38:56 +0200 Subject: [PATCH 6/8] changelog.md update --- src/OpenTelemetry.Api/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index 63046fa589d..b30b35ae044 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* **Breaking change:** Fix space character encoding for baggage item values + when propagating baggage. + ([#5303](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5303)) + ## 1.8.0 Released 2024-Apr-02 From 50ed9209edea6547f0b5af3716ac160da74a7718 Mon Sep 17 00:00:00 2001 From: Mateusz Lach Date: Fri, 5 Apr 2024 11:16:26 +0200 Subject: [PATCH 7/8] pr feedback --- src/OpenTelemetry.Api/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index b30b35ae044..1b58f82709a 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -2,8 +2,8 @@ ## Unreleased -* **Breaking change:** Fix space character encoding for baggage item values - when propagating baggage. +* **Breaking change:** Change space character encoding from `+` to `%20` + for baggage item values when propagating baggage. ([#5303](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5303)) ## 1.8.0 From db34fd953cd00b1fdc863dd7659ed26b1ef50753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20=C5=81ach?= Date: Fri, 5 Apr 2024 11:42:05 +0200 Subject: [PATCH 8/8] Update src/OpenTelemetry.Api/CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert Pająk --- src/OpenTelemetry.Api/CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index 1b58f82709a..5dfd8dc48e2 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -2,8 +2,9 @@ ## Unreleased -* **Breaking change:** Change space character encoding from `+` to `%20` - for baggage item values when propagating baggage. +* **Breaking change:** Fix space character encoding from `+` to `%20` + for baggage item values when propagating baggage as defined in + [W3C Baggage propagation format specification](https://www.w3.org/TR/baggage/). ([#5303](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5303)) ## 1.8.0