Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[api-baggage] fix encoding of space chars in baggage item value #5303

Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* **Breaking change:** Fix space character encoding for baggage item values
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest writing explicitly that space was encoded to + and now it is encoded to %20.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed in 50ed920

when propagating baggage.
([#5303](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5303))

## 1.8.0

Released 2024-Apr-02
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

using System.Net;
using System.Text;
using OpenTelemetry.Internal;

Expand Down Expand Up @@ -94,7 +93,7 @@ public override void Inject<T>(PropagationContext context, T carrier, Action<T,
continue;
}

baggage.Append(WebUtility.UrlEncode(item.Key)).Append('=').Append(WebUtility.UrlEncode(item.Value)).Append(',');
baggage.Append(Uri.EscapeDataString(item.Key)).Append('=').Append(Uri.EscapeDataString(item.Value)).Append(',');
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
}
while (e.MoveNext() && ++itemCount < MaxBaggageItems && baggage.Length < MaxBaggageLength);
baggage.Remove(baggage.Length - 1, 1);
Expand Down Expand Up @@ -141,8 +140,8 @@ internal static bool TryExtractBaggage(string[] baggageCollection, out Dictionar
continue;
}

var key = WebUtility.UrlDecode(parts[0]);
var value = WebUtility.UrlDecode(parts[1]);
var key = Uri.UnescapeDataString(parts[0]);
var value = Uri.UnescapeDataString(parts[1]);

if (string.IsNullOrEmpty(key) || string.IsNullOrEmpty(value))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ public void ValidateSpecialCharsBaggageExtraction()
Assert.Equal("key%28%293", escapedKey);
Assert.Equal("value%28%29%21%26%3B%3A", escapedValue);

var initialBaggage = $"key+1=value+1,{encodedKey}={encodedValue},{escapedKey}={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~";

var carrier = new List<KeyValuePair<string, string>>
{
new KeyValuePair<string, string>(BaggagePropagator.BaggageHeaderName, initialBaggage),
Expand All @@ -142,11 +144,11 @@ public void ValidateSpecialCharsBaggageExtraction()
Assert.False(propagationContext == default);
Assert.True(propagationContext.ActivityContext == default);

Assert.Equal(3, propagationContext.Baggage.Count);
Assert.Equal(4, propagationContext.Baggage.Count);

var actualBaggage = propagationContext.Baggage.GetBaggage();

Assert.Equal(3, actualBaggage.Count);
Assert.Equal(4, actualBaggage.Count);

Assert.True(actualBaggage.ContainsKey("key 1"));
Assert.Equal("value 1", actualBaggage["key 1"]);
Expand All @@ -156,6 +158,10 @@ 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"]);
}

[Fact]
Expand Down Expand Up @@ -195,11 +201,14 @@ public void ValidateSpecialCharsBaggageInjection()
{
{ "key 1", "value 1" },
{ "key2", "!x_x,x-x&x(x\");:" },

// x20-x7E range
{ "key3", " !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~" },
}));

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~", carrier[BaggagePropagator.BaggageHeaderName]);
}
}