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 14 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:** Change space character encoding from `+` to `%20`
for baggage item values when propagating baggage.
lachmatt marked this conversation as resolved.
Show resolved Hide resolved
([#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]);
}
}