diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index 92535998fc0..66c497594db 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -10,6 +10,10 @@ Notes](../../RELEASENOTES.md). Add support for using environment variables as context propagation carriers. ([#7174](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7174)) +* Fix `BaggagePropagator` to correctly follow Key and Value Encoding rules as mentioned + the [W3C Baggage specification](https://www.w3.org/TR/baggage/#key-and-value-encoding). + [#7051](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7051) + * Update `TraceContextPropagator` to support the W3C randomness flag. ([#7301](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7301)) diff --git a/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs b/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs index 669c67c3f82..ea17c4e721a 100644 --- a/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs +++ b/src/OpenTelemetry.Api/Context/Propagation/BaggagePropagator.cs @@ -2,12 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 #if NET -#if NET9_0_OR_GREATER using System.Buffers; -#endif using System.Diagnostics.CodeAnalysis; #endif -using System.Net; using System.Text; using OpenTelemetry.Internal; @@ -19,12 +16,44 @@ namespace OpenTelemetry.Context.Propagation; public class BaggagePropagator : TextMapPropagator { internal const string BaggageHeaderName = "baggage"; + private const string Hex = "0123456789ABCDEF"; private const int MaxBaggageLength = 8192; private const int MaxBaggageItems = 180; -#if NET9_0_OR_GREATER - private static readonly SearchValues DecodeHints = SearchValues.Create('%', '+'); +#if NET + private static readonly SearchValues DecodeHints = SearchValues.Create("%"); + + private static readonly SearchValues ValidKeySearcher = SearchValues.Create( + "!#$%&'*+-.^_`|~0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"); + + // W3C Baggage 3.3 baggage-octet, '%' excluded so raw '%' is always encoded as %25 + private static readonly SearchValues ValidValueSearcher = SearchValues.Create( + "!#$&'()*+-./:<=>?@[]^_`|~0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz{}"); + +#else + + private static readonly char[] ValidKeyChars = + [ + '!', '#', '$', '%', '&', '\'', '*', '+', '-', '.', '^', '_', '`', '|', '~', + '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', + 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', + 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', + 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', + 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', + ]; + + // baggage-octet minus %, so raw % is always encoded as %25 + private static readonly char[] ValidValueChars = + [ + '!', '#', '$', '&', '\'', '(', ')', '*', '+', '-', '.', '/', ':', + '<', '=', '>', '?', '@', '[', ']', '^', '_', '`', '{', '|', '}', '~', + '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', + 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', + 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', + 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', + 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', + ]; #endif /// @@ -108,8 +137,13 @@ public override void Inject(PropagationContext context, T carrier, Action 0) @@ -167,7 +201,7 @@ internal static bool TryExtractBaggage( while (!remaining.IsEmpty) { var pair = ReadNextSegment(ref remaining, ','); - baggageLength += pair.Length + 1; // pair and comma + baggageLength += pair.Length + 1; if (baggageLength >= MaxBaggageLength || baggageDictionary?.Count >= MaxBaggageItems) { @@ -183,6 +217,13 @@ internal static bool TryExtractBaggage( var rawKey = pair.Slice(0, separatorIndex).Trim(); + if (!IsValidKey(rawKey)) + { + continue; + } + + var key = rawKey.ToString(); + var rawValue = pair.Slice(separatorIndex + 1); var semicolonIndex = rawValue.IndexOf(';'); @@ -193,7 +234,6 @@ internal static bool TryExtractBaggage( rawValue = rawValue.Trim(); - var key = DecodeIfNeeded(rawKey); var value = DecodeIfNeeded(rawValue); if (string.IsNullOrEmpty(key) || string.IsNullOrEmpty(value)) @@ -225,10 +265,212 @@ private static ReadOnlySpan ReadNextSegment(ref ReadOnlySpan remaini return result; } - private static string DecodeIfNeeded(ReadOnlySpan value) => -#if NET9_0_OR_GREATER - value.ContainsAny(DecodeHints) ? WebUtility.UrlDecode(value.ToString()) : value.ToString(); + private static string EncodeKey(ReadOnlySpan key) => Encode(key, isKey: true); + + private static string EncodeValue(ReadOnlySpan value) => Encode(value, isKey: false); + + private static string Encode(ReadOnlySpan value, bool isKey) + { +#if NET + if (!value.ContainsAnyExcept(isKey ? ValidKeySearcher : ValidValueSearcher)) + { + return value.ToString(); + } +#else + var validChars = isKey ? ValidKeyChars : ValidValueChars; + var allValid = true; + foreach (var c in value) + { + if (Array.IndexOf(validChars, c) < 0) + { + allValid = false; + break; + } + } + + if (allValid) + { + return value.ToString(); + } +#endif + + var sb = new StringBuilder(value.Length); + +#if NET + Span utf8Buffer = stackalloc byte[4]; + foreach (var rune in value.EnumerateRunes()) + { + if (rune.IsAscii) + { + var c = (char)rune.Value; + if (isKey ? !IsValidKey(c) : !IsValidValueChar(c)) + { + AppendPercentEncoded(sb, (byte)c); + } + else + { + sb.Append(c); + } + } + else + { + // Non-ASCII rune: always encode as UTF-8 bytes. + // This correctly handles non-BMP scalar values (emoji, etc.) + // because Rune represents the full codepoint, not a surrogate half. + var byteCount = rune.EncodeToUtf8(utf8Buffer); + foreach (var b in utf8Buffer[..byteCount]) + { + AppendPercentEncoded(sb, b); + } + } + } +#else + var i = 0; + while (i < value.Length) + { + var c = value[i]; + + if (char.IsHighSurrogate(c) && i + 1 < value.Length && char.IsLowSurrogate(value[i + 1])) + { + // Non-BMP pair: encode both chars as one UTF-8 sequence. + // Passing the pair to Encoding.UTF8 produces the correct 4-byte result + // rather than two replacement characters. + foreach (var b in Encoding.UTF8.GetBytes(new string(new[] { c, value[i + 1] }))) + { + AppendPercentEncoded(sb, b); + } + + i += 2; + } + else + { + var shouldEncode = isKey ? !IsValidKey(c) : !IsValidValueChar(c); + if (shouldEncode) + { + if (c > '\x7F') + { + foreach (var b in Encoding.UTF8.GetBytes(c.ToString())) + { + AppendPercentEncoded(sb, b); + } + } + else + { + AppendPercentEncoded(sb, (byte)c); + } + } + else + { + sb.Append(c); + } + + i++; + } + } +#endif + + return sb.ToString(); + } + + private static bool IsValidValueChar(char c) => +#if NET + ValidValueSearcher.Contains(c); +#else + Array.IndexOf(ValidValueChars, c) >= 0; +#endif + + private static bool IsValidKey(char c) => +#if NET + ValidKeySearcher.Contains(c); +#else + Array.IndexOf(ValidKeyChars, c) >= 0; +#endif + + private static bool IsValidKey(ReadOnlySpan key) + { +#if NET + return !key.ContainsAnyExcept(ValidKeySearcher); #else - value.IndexOfAny('%', '+') < 0 ? value.ToString() : WebUtility.UrlDecode(value.ToString()); + foreach (var c in key) + { + if (Array.IndexOf(ValidKeyChars, c) < 0) + { + return false; + } + } + + return true; #endif + } + + private static string DecodeIfNeeded(ReadOnlySpan value) + { +#if NET + if (!value.ContainsAny(DecodeHints)) + { + return value.ToString(); + } +#else + if (value.IndexOf('%') < 0) + { + return value.ToString(); + } +#endif + + var sb = new StringBuilder(value.Length); + + var byteBuffer = new byte[value.Length]; + var byteCount = 0; + var i = 0; + + while (i < value.Length) + { + if (value[i] == '%') + { + if (i + 2 < value.Length && IsHexDigit(value[i + 1]) && IsHexDigit(value[i + 2])) + { + byteBuffer[byteCount++] = (byte)((HexDigitValue(value[i + 1]) << 4) | HexDigitValue(value[i + 2])); + i += 3; + } + else + { + FlushByteBuffer(sb, byteBuffer, ref byteCount); + sb.Append('\uFFFD'); + i += Math.Min(3, value.Length - i); + } + } + else + { + FlushByteBuffer(sb, byteBuffer, ref byteCount); + sb.Append(value[i]); + i++; + } + } + + FlushByteBuffer(sb, byteBuffer, ref byteCount); + + return sb.ToString(); + } + + private static void FlushByteBuffer(StringBuilder sb, byte[] buffer, ref int count) + { + if (count == 0) + { + return; + } + + sb.Append(Encoding.UTF8.GetString(buffer, 0, count)); + count = 0; + } + + private static bool IsHexDigit(char c) => + char.IsAsciiDigit(c) || c is (>= 'A' and <= 'F') or (>= 'a' and <= 'f'); + + private static int HexDigitValue(char c) => + c <= '9' ? c - '0' : (c & 0x0f) + 9; + + private static void AppendPercentEncoded(StringBuilder sb, byte b) => + sb.Append('%') + .Append(Hex[(b >> 4) & 0xF]) + .Append(Hex[b & 0xF]); } diff --git a/test/OpenTelemetry.Api.FuzzTests/Context/Propagation/BaggagePropagatorFuzzTests.cs b/test/OpenTelemetry.Api.FuzzTests/Context/Propagation/BaggagePropagatorFuzzTests.cs index d1008b63d13..48096b29b23 100644 --- a/test/OpenTelemetry.Api.FuzzTests/Context/Propagation/BaggagePropagatorFuzzTests.cs +++ b/test/OpenTelemetry.Api.FuzzTests/Context/Propagation/BaggagePropagatorFuzzTests.cs @@ -16,24 +16,21 @@ public class BaggagePropagatorFuzzTests private const int MaxBaggageItems = 180; [Property(MaxTest = MaxTests)] - public Property InjectExtractRoundTripPreservesSafeBaggage() => Prop.ForAll(Generators.SafeBaggageDictionaryArbitrary(), (baggageItems) => + public Property ExtractNeverThrowsOnArbitraryInput() => Prop.ForAll(Generators.BaggageCarrierArbitrary(), (carrier) => { - try - { - var propagator = new BaggagePropagator(); - var carrier = new Dictionary(StringComparer.Ordinal); - var propagationContext = new PropagationContext(default, Baggage.Create(baggageItems)); - - propagator.Inject(propagationContext, carrier, FuzzTestHelpers.Setter); - - var extracted = propagator.Extract(default, carrier, FuzzTestHelpers.Getter); + var propagator = new BaggagePropagator(); + propagator.Extract(default, carrier, FuzzTestHelpers.ArrayGetter); + return true; + }); - return DictionariesEqual(baggageItems, extracted.Baggage.GetBaggage()); - } - catch (Exception ex) when (FuzzTestHelpers.IsAllowedException(ex)) - { - return true; - } + [Property(MaxTest = MaxTests)] + public Property InjectNeverThrowsOnArbitraryInput() => Prop.ForAll(Generators.BaggageDictionaryArbitrary(), (baggageItems) => + { + var propagator = new BaggagePropagator(); + var carrier = new Dictionary(StringComparer.Ordinal); + var propagationContext = new PropagationContext(default, Baggage.Create(baggageItems)); + propagator.Inject(propagationContext, carrier, FuzzTestHelpers.Setter); + return true; }); [Property(MaxTest = MaxTests)] diff --git a/test/OpenTelemetry.Api.FuzzTests/Context/Propagation/EnvironmentVariableCarrierFuzzTests.cs b/test/OpenTelemetry.Api.FuzzTests/Context/Propagation/EnvironmentVariableCarrierFuzzTests.cs index d8b4375a456..d1c7c0f3770 100644 --- a/test/OpenTelemetry.Api.FuzzTests/Context/Propagation/EnvironmentVariableCarrierFuzzTests.cs +++ b/test/OpenTelemetry.Api.FuzzTests/Context/Propagation/EnvironmentVariableCarrierFuzzTests.cs @@ -51,23 +51,15 @@ public Property TraceContextRoundTripWorksWithEnvironmentVariableCarrier() => Pr }); [Property(MaxTest = MaxTests)] - public Property BaggageRoundTripWorksWithEnvironmentVariableCarrier() => Prop.ForAll(Generators.SafeBaggageDictionaryArbitrary(), baggageItems => + public Property EnvironmentVariableCarrierNeverThrowsWithBaggageRoundTrip() => Prop.ForAll(Generators.SafeBaggageDictionaryArbitrary(), baggageItems => { - try - { - var carrier = new Dictionary(StringComparer.Ordinal); - var propagator = new BaggagePropagator(); + var carrier = new Dictionary(StringComparer.Ordinal); + var propagator = new BaggagePropagator(); - propagator.Inject(new PropagationContext(default, Baggage.Create(baggageItems)), carrier, EnvironmentVariableCarrier.Set); - - var extracted = propagator.Extract(default, EnvironmentVariableCarrier.Capture(carrier), EnvironmentVariableCarrier.Get); + propagator.Inject(new PropagationContext(default, Baggage.Create(baggageItems)), carrier, EnvironmentVariableCarrier.Set); + propagator.Extract(default, EnvironmentVariableCarrier.Capture(carrier), EnvironmentVariableCarrier.Get); - return DictionariesEqual(baggageItems, extracted.Baggage.GetBaggage()); - } - catch (Exception ex) when (FuzzTestHelpers.IsAllowedException(ex)) - { - return true; - } + return true; }); [Property(MaxTest = MaxTests)] diff --git a/test/OpenTelemetry.Api.Tests/Context/Propagation/BaggagePropagatorTests.cs b/test/OpenTelemetry.Api.Tests/Context/Propagation/BaggagePropagatorTests.cs index 0cd1f2dae7f..e62b7f2e07b 100644 --- a/test/OpenTelemetry.Api.Tests/Context/Propagation/BaggagePropagatorTests.cs +++ b/test/OpenTelemetry.Api.Tests/Context/Propagation/BaggagePropagatorTests.cs @@ -1,7 +1,5 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 - -using System.Net; using Xunit; namespace OpenTelemetry.Context.Propagation.Tests; @@ -113,46 +111,6 @@ public void ValidateLongBaggageExtraction() Assert.Equal(new string('x', 8186), array[0].Value); } - [Fact] - public void ValidateSpecialCharsBaggageExtraction() - { - var encodedKey = WebUtility.UrlEncode("key2"); - var encodedValue = WebUtility.UrlEncode("!x_x,x-x&x(x\");:"); - var escapedKey = Uri.EscapeDataString("key()3"); - var escapedValue = Uri.EscapeDataString("value()!&;:"); - - Assert.Equal("key2", encodedKey); - Assert.Equal("!x_x%2Cx-x%26x(x%22)%3B%3A", encodedValue); - 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 carrier = new List> - { - new(BaggagePropagator.BaggageHeaderName, initialBaggage), - }; - - var propagationContext = this.baggage.Extract(default, carrier, GetterList); - - Assert.False(propagationContext == default); - Assert.True(propagationContext.ActivityContext == default); - - Assert.Equal(3, propagationContext.Baggage.Count); - - var actualBaggage = propagationContext.Baggage.GetBaggage(); - - Assert.Equal(3, actualBaggage.Count); - - Assert.True(actualBaggage.ContainsKey("key 1")); - Assert.Equal("value 1", actualBaggage["key 1"]); - - Assert.True(actualBaggage.ContainsKey("key2")); - Assert.Equal("!x_x,x-x&x(x\");:", actualBaggage["key2"]); - - Assert.True(actualBaggage.ContainsKey("key()3")); - Assert.Equal("value()!&;:", actualBaggage["key()3"]); - } - [Fact] public void ValidateEmptyBaggageInjection() { @@ -180,24 +138,6 @@ public void ValidateBaggageInjection() Assert.Equal("key1=value1,key2=value2", carrier[BaggagePropagator.BaggageHeaderName]); } - [Fact] - public void ValidateSpecialCharsBaggageInjection() - { - var carrier = new Dictionary(); - var propagationContext = new PropagationContext( - default, - new Baggage(new Dictionary - { - { "key 1", "value 1" }, - { "key2", "!x_x,x-x&x(x\");:" }, - })); - - 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]); - } - [Fact] public void ValidateMultipleEqualsInValue() { @@ -232,7 +172,7 @@ public void ValidateOWSOnExtraction() { var carrier = new Dictionary { - { BaggagePropagator.BaggageHeaderName, "SomeKey \t = \t SomeValue \t , \t SomeKey2 \t = \t SomeValue2" }, + { BaggagePropagator.BaggageHeaderName, "SomeKey = \t SomeValue \t , SomeKey2 = \t SomeValue2" }, }; var propagationContext = this.baggage.Extract(default, carrier, Getter); @@ -271,7 +211,7 @@ public void ValidateOptionalWhiteSpaceExtractionDoesNotCorruptOnReinjection() // Simulates a header emitted by .NET 10's W3C propagator var carrier = new Dictionary { - { BaggagePropagator.BaggageHeaderName, "correlationId = 12345, userId = user-abc" }, + { BaggagePropagator.BaggageHeaderName, "correlationId= 12345,userId= user-abc" }, }; var extractedContext = this.baggage.Extract(default, carrier, Getter); @@ -298,26 +238,6 @@ public void ValidateOptionalWhiteSpaceBeforeSemicolonIgnored() Assert.Equal("SomeValue", baggage.Value); } - [Fact] - public void ValidatePercentEncoding() - { - var originalValue = "\t \"\';=asdf!@#$%^&*()"; - var encodedValue = Uri.EscapeDataString(originalValue); - - var carrier = new Dictionary - { - { BaggagePropagator.BaggageHeaderName, $"SomeKey={encodedValue}" }, - }; - - var propagationContext = this.baggage.Extract(default, carrier, Getter); - Assert.Single(propagationContext.Baggage.GetBaggage()); - - var baggage = propagationContext.Baggage.GetBaggage().FirstOrDefault(); - - Assert.Equal("SomeKey", baggage.Key); - Assert.Equal(originalValue, baggage.Value); - } - [Fact] public void ValidateInvalidFormatSkipped() { @@ -540,8 +460,8 @@ public void ValidateOversizedBaggageExtractionHonorsLimits() Assert.False(baggage.ContainsKey("k0180")); } - [Fact(Skip = "Fails due to spec mismatch, tracked in https://github.com/open-telemetry/opentelemetry-dotnet/issues/5210")] - public void ValidateSpecialCharactersInjection() + [Fact] + public void ValidateSpecialCharactersInjectionForValue() { var propagationContext = new PropagationContext( default, @@ -556,13 +476,530 @@ public void ValidateSpecialCharactersInjection() var baggageHeader = carrier[BaggagePropagator.BaggageHeaderName]; - Assert.Contains("%09", baggageHeader, StringComparison.Ordinal); // Tab - Assert.Contains("%20", baggageHeader, StringComparison.Ordinal); // Space - Assert.Contains("%22", baggageHeader, StringComparison.Ordinal); // Quote + Assert.Contains("%09", baggageHeader, StringComparison.Ordinal); // Tab + Assert.Contains("%20", baggageHeader, StringComparison.Ordinal); // Space + Assert.Contains("%22", baggageHeader, StringComparison.Ordinal); // Quote var extractedContext = this.baggage.Extract(default, carrier, Getter); var extractedBaggage = extractedContext.Baggage.GetBaggage(); Assert.Equal("\t \"';=asdf!@#$%^&*()", extractedBaggage["key"]); } + + [Fact] + public void KeyValidTokenCharSymbolInjectedUnchanged() + { + var propagationContext = new PropagationContext( + default, + new Baggage(new Dictionary + { + { "my.key-name_here", "value" }, + })); + + var carrier = new Dictionary(); + this.baggage.Inject(propagationContext, carrier, Setter); + + Assert.Equal("my.key-name_here=value", carrier[BaggagePropagator.BaggageHeaderName]); + } + + [Fact] + public void ValidateNonAsciiEncodingIsCountedAgainstByteLimitNotCharCount() + { + // '\u00E9' encodes to %C3%A9 (6 wire bytes per char, not 1). + // 1000 '\u00E9' chars = 6000 wire bytes for the value alone. + // With key "k=" (2 bytes) + 6000 = 6002 bytes, well within 8192 chars + // but the CHARACTER count (1002) would have appeared safe under the old code. + // We construct a case where purely char-count-based accounting would allow + // a second entry through that pushes the header over 8192 bytes. + // key "a" (1) + "=" (1) + 1365 x "\u00E9" -> 1365 x 6 = 8190 wire bytes = 8192 total + + var nonAsciiValue = new string('\u00E9', 1365); + + var propagationContext = new PropagationContext( + default, + new Baggage(new Dictionary + { + { "a", nonAsciiValue }, + { "b", "c" }, + })); + + var carrier = new Dictionary(); + this.baggage.Inject(propagationContext, carrier, Setter); + + Assert.Single(carrier); + + var header = carrier[BaggagePropagator.BaggageHeaderName]; + + Assert.True( + header.Length <= 8192, + $"Injected header length {header.Length} exceeds maximum of 8192 bytes"); + + Assert.False( + header.Contains("b=c", StringComparison.Ordinal), + "Second entry should have been excluded to stay within the byte limit"); + } + + [Fact] + public void ValidateRawPercentInValueIsEncodedOnInject() + { + var propagationContext = new PropagationContext( + default, + new Baggage(new Dictionary { { "key", "x%20y" } })); + + var carrier = new Dictionary(); + this.baggage.Inject(propagationContext, carrier, Setter); + + // The literal '%' must be encoded as %25 so the value is unambiguous on the wire + Assert.Contains("%2520", carrier[BaggagePropagator.BaggageHeaderName], StringComparison.Ordinal); + } + + [Fact] + public void RoundTripRawPercentInValuePreservesLiteralPercent() + { + var propagationContext = new PropagationContext( + default, + new Baggage(new Dictionary { { "key", "x%20y" } })); + + var carrier = new Dictionary(); + this.baggage.Inject(propagationContext, carrier, Setter); + + var extracted = this.baggage.Extract(default, carrier, Getter).Baggage.GetBaggage(); + var entry = Assert.Single(extracted); + Assert.Equal("key", entry.Key); + Assert.Equal("x%20y", entry.Value); + } + + // ------------------------------------------------------------------------- + // Keys incorrect path + // The current implementation URL-decodes keys on extract (#5479). + // These tests document what the correct behaviour SHOULD be: keys arriving + // on the wire that happen to contain %-sequences or '+' must be treated as + // literal token strings they are NOT decoded. + // + // '%' is a valid token char, so "key%20name" is a valid token whose name is + // literally "key%20name", not "key name". '+' is also a valid token char. + // ------------------------------------------------------------------------- + [Fact] + public void KeyPercentSequenceInKeyPreservedLiterallyOnExtract() + { + var carrier = new Dictionary + { + { BaggagePropagator.BaggageHeaderName, "key%20name=value,valid-key=valid-value" }, + }; + + var context = this.baggage.Extract(default, carrier, Getter); + var baggage = context.Baggage.GetBaggage(); + + Assert.Equal(2, baggage.Count); + Assert.True(baggage.ContainsKey("key%20name")); + Assert.False(baggage.ContainsKey("key name")); + } + + [Fact] + public void KeyPlusInKeyPreservedLiterallyOnExtract() + { + var carrier = new Dictionary + { + { BaggagePropagator.BaggageHeaderName, "key+name=value" }, + }; + + var context = this.baggage.Extract(default, carrier, Getter); + var entry = Assert.Single(context.Baggage.GetBaggage()); + + Assert.Equal("key+name", entry.Key); + } + + [Theory(Skip = "https://github.com/open-telemetry/opentelemetry-dotnet/pull/7051")] + [InlineData(" ", "%20")] + [InlineData("(", "%28")] + [InlineData(":", "%3A")] + [InlineData(";", "%3B")] + [InlineData("@", "%40")] + public void KeyWithDelimiterCharIsPercentEncodedOnInject(string delimiter, string expectedEncoding) + { + var propagationContext = new PropagationContext( + default, + new Baggage(new Dictionary + { + { $"invalid{delimiter}key", "should-be-kept" }, + { "valid-key", "valid-value" }, + })); + + var carrier = new Dictionary(); + this.baggage.Inject(propagationContext, carrier, Setter); + + Assert.Single(carrier); + + var header = carrier[BaggagePropagator.BaggageHeaderName]; + + Assert.Contains("valid-key=valid-value", header, StringComparison.Ordinal); + var expectedEncodedKey = $"invalid{expectedEncoding}key=should-be-kept"; + Assert.Contains(expectedEncodedKey, header, StringComparison.Ordinal); + Assert.Equal(2, header.Split(',').Length); + } + + [Theory] + [InlineData("!")] + [InlineData("#")] + [InlineData("&")] + [InlineData("*")] + [InlineData("+")] + [InlineData("-")] + [InlineData("/")] + [InlineData(":")] + [InlineData("=")] + [InlineData("?")] + [InlineData("@")] + [InlineData("~")] + public void ValueValidBaggageOctetCharPassesThroughUnchanged(string octet) + { + // These characters are valid unencoded baggage-octets and must not be + // transformed on either inject or extract. + var value = $"val{octet}use"; + var propagationContext = new PropagationContext( + default, + new Baggage(new Dictionary { { "key", value } })); + + var carrier = new Dictionary(); + this.baggage.Inject(propagationContext, carrier, Setter); + + // Confirm it was injected without encoding + Assert.Contains(value, carrier[BaggagePropagator.BaggageHeaderName], StringComparison.Ordinal); + + // Confirm it round-trips back unchanged + var extracted = this.baggage.Extract(default, carrier, Getter).Baggage.GetBaggage(); + Assert.True(extracted.TryGetValue("key", out var extractedValue)); + Assert.Equal(value, extractedValue); + } + + [Theory] + [InlineData("key%201=value%201", "key%201", "value 1")] + [InlineData("key=val%2Buse", "key", "val+use")] + [InlineData("key=val%20use", "key", "val use")] + [InlineData("key=%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~", "key", " !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~")] + public void ValidateSpecialCharsBaggageExtraction(string propagatedBaggage, string key, string expectedDecodedValue) + { + var carrier = new List> + { + new(BaggagePropagator.BaggageHeaderName, propagatedBaggage), + }; + + var propagationContext = this.baggage.Extract(default, carrier, GetterList); + + Assert.False(propagationContext == default); + Assert.True(propagationContext.ActivityContext == default); + + Assert.Equal(1, propagationContext.Baggage.Count); + + var actualBaggage = propagationContext.Baggage.GetBaggage(); + + Assert.Single(actualBaggage); + + Assert.Contains(key, actualBaggage); + Assert.Equal(expectedDecodedValue, actualBaggage[key]); + } + + [Fact] + public void ValidatePlusIsNotEncoded() + { + var carrier = new Dictionary + { + { BaggagePropagator.BaggageHeaderName, "key=a+b%20c" }, + }; + + var propagationContext = this.baggage.Extract(default, carrier, Getter); + Assert.Single(propagationContext.Baggage.GetBaggage()); + + var entry = propagationContext.Baggage.GetBaggage().First(); + Assert.Equal("key", entry.Key); + Assert.Equal("a+b c", entry.Value); // '+' stays as '+', not ' ' + } + + [Theory] + [InlineData("!")] + [InlineData("#")] + [InlineData("$")] + [InlineData("%")] + [InlineData("&")] + [InlineData("'")] + [InlineData("*")] + [InlineData("+")] + [InlineData("-")] + [InlineData(".")] + [InlineData("^")] + [InlineData("_")] + [InlineData("`")] + [InlineData("|")] + [InlineData("~")] + public void ValidateValidTokenCharInKeyIsAcceptedOnExtract(string specialChar) + { + var key = $"key{specialChar}name"; + var carrier = new Dictionary + { + { BaggagePropagator.BaggageHeaderName, $"{key}=value" }, + }; + + var propagationContext = this.baggage.Extract(default, carrier, Getter); + Assert.Single(propagationContext.Baggage.GetBaggage()); + Assert.Equal(key, propagationContext.Baggage.GetBaggage().First().Key); + } + + [Theory] + [InlineData(" ")] + [InlineData("\"")] + [InlineData("(")] + [InlineData(")")] + [InlineData("/")] + [InlineData(":")] + [InlineData(";")] + [InlineData("<")] + [InlineData(">")] + [InlineData("?")] + [InlineData("@")] + [InlineData("[")] + [InlineData("\\")] + [InlineData("]")] + [InlineData("{")] + [InlineData("}")] + public void ValidateKeyWithInvalidTokenCharDroppedOnExtract(string invalidChar) + { + var invalidKey = $"key{invalidChar}name"; + var carrier = new Dictionary + { + { + BaggagePropagator.BaggageHeaderName, + $"{invalidKey}=should-be-dropped,valid-key=valid-value" + }, + }; + + var propagationContext = this.baggage.Extract(default, carrier, Getter); + Assert.Single(propagationContext.Baggage.GetBaggage()); + Assert.Equal("valid-key", propagationContext.Baggage.GetBaggage().First().Key); + } + + [Fact] + public void ValidateKeyWithValidDelimitersNotDroppedOnExtract() + { + var invalidKey = $"key,name"; + var carrier = new Dictionary + { + { + BaggagePropagator.BaggageHeaderName, + $"{invalidKey}=should-be-dropped,valid-key=valid-value" + }, + }; + + var propagationContext = this.baggage.Extract(default, carrier, Getter); + Assert.Equal("name", propagationContext.Baggage.GetBaggage().First().Key); + + invalidKey = $"key=name"; + carrier = new Dictionary + { + { + BaggagePropagator.BaggageHeaderName, + $"{invalidKey}=should-be-dropped,valid-key=valid-value" + }, + }; + + propagationContext = this.baggage.Extract(default, carrier, Getter); + Assert.Equal("key", propagationContext.Baggage.GetBaggage().First().Key); + } + + [Theory] + [InlineData("%21", "!")] + [InlineData("%22", "\"")] + [InlineData("%23", "#")] + [InlineData("%24", "$")] + [InlineData("%25", "%")] + [InlineData("%26", "&")] + [InlineData("%27", "'")] + [InlineData("%28", "(")] + [InlineData("%29", ")")] + [InlineData("%2A", "*")] + [InlineData("%2B", "+")] + [InlineData("%2C", ",")] + [InlineData("%2D", "-")] + [InlineData("%2E", ".")] + [InlineData("%2F", "/")] + [InlineData("%3A", ":")] + [InlineData("%3B", ";")] + [InlineData("%3C", "<")] + [InlineData("%3D", "=")] + [InlineData("%3E", ">")] + [InlineData("%3F", "?")] + [InlineData("%40", "@")] + [InlineData("%5B", "[")] + [InlineData("%5C", "\\")] + [InlineData("%5D", "]")] + [InlineData("%5E", "^")] + [InlineData("%5F", "_")] + [InlineData("%60", "`")] + [InlineData("%7B", "{")] + [InlineData("%7C", "|")] + [InlineData("%7D", "}")] + [InlineData("%7E", "~")] + [InlineData("%20", " ")] + public void ValidatePercentEncodedValueCharacterDecodesCorrectly(string encoded, string expected) + { + var carrier = new Dictionary + { + { BaggagePropagator.BaggageHeaderName, $"key={encoded}" }, + }; + + var propagationContext = this.baggage.Extract(default, carrier, Getter); + var baggage = Assert.Single(propagationContext.Baggage.GetBaggage()); + Assert.Equal(expected, baggage.Value); + } + + [Theory] + [InlineData("key=%", "key", "\uFFFD")] + [InlineData("key=%2", "key", "\uFFFD")] + [InlineData("key=%GG", "key", "\uFFFD")] + public void ValidateMalformedPercentSequenceInValueIsReplacedWithReplacementCharacter( + string headerValue, string expectedKey, string expectedValue) + { + var carrier = new Dictionary + { + { BaggagePropagator.BaggageHeaderName, headerValue }, + }; + + var propagationContext = this.baggage.Extract(default, carrier, Getter); + + var entry = Assert.Single(propagationContext.Baggage.GetBaggage()); + Assert.Equal(expectedKey, entry.Key); + Assert.Equal(expectedValue, entry.Value); + } + + [Theory] + [InlineData(" ", "%20")] + [InlineData("\"", "%22")] + [InlineData(",", "%2C")] + [InlineData(";", "%3B")] + [InlineData("\\", "%5C")] + public void ValidateCharacterOutsideBaggageOctetIsPercentEncodedOnInject( + string rawChar, string expectedEncoded) + { + var propagationContext = new PropagationContext( + default, + new Baggage(new Dictionary { { "key", $"prefix{rawChar}suffix" } })); + + var carrier = new Dictionary(); + this.baggage.Inject(propagationContext, carrier, Setter); + + Assert.Single(carrier); + Assert.Contains(expectedEncoded, carrier[BaggagePropagator.BaggageHeaderName], StringComparison.Ordinal); + } + + // ========================================================================= + // ROUND-TRIP + // These tests inject baggage and then extract from the same carrier, + // verifying that the full pipeline preserves the original data. + // ========================================================================= + + [Fact] + public void RoundTripValueWithSpacePreservedAsSpace() + { + var carrier = new Dictionary(); + this.baggage.Inject( + new PropagationContext(default, new Baggage(new Dictionary { { "key", "value with space" }, })), carrier, Setter); + + // The intermediate header must use %20, not '+' + Assert.Contains("%20", carrier[BaggagePropagator.BaggageHeaderName], StringComparison.Ordinal); + Assert.DoesNotContain("+", carrier[BaggagePropagator.BaggageHeaderName], StringComparison.Ordinal); + + var extracted = this.baggage.Extract(default, carrier, Getter).Baggage.GetBaggage(); + Assert.Equal("value with space", extracted["key"]); + } + + [Fact] + public void RoundTripValueWithAllMandatoryEncodeCharsPreservedExactly() + { + const string original = "val use\"wi,thback\\slash"; + + var carrier = new Dictionary(); + this.baggage.Inject(new PropagationContext(default, new Baggage(new Dictionary { { "key", original }, })), carrier, Setter); + + var extracted = this.baggage.Extract(default, carrier, Getter).Baggage.GetBaggage(); + Assert.Equal(original, extracted["key"]); + } + + [Fact] + public void RoundTripMixedValidAndInvalidKeysOnlyValidKeysSurvive() + { + var carrier = new Dictionary(); + this.baggage.Inject(new PropagationContext(default, new Baggage(new Dictionary { { "valid-key", "valid-value" }, })), carrier, Setter); + + var entry = Assert.Single(this.baggage.Extract(default, carrier, Getter).Baggage.GetBaggage()); + Assert.Equal("valid-key", entry.Key); + Assert.Equal("valid-value", entry.Value); + } + + // ------------------------------------------------------------------------- + // Non-ASCII encoding + // ------------------------------------------------------------------------- + + [Fact] + public void ValidateNonAsciiValueIsUtf8PercentEncodedOnInject() + { + // '\u00E9' is U+00E9, UTF-8: 0xC3 0xA9 -> %C3%A9 + var propagationContext = new PropagationContext( + default, + new Baggage(new Dictionary { { "key", "caf\u00E9" } })); + + var carrier = new Dictionary(); + this.baggage.Inject(propagationContext, carrier, Setter); + + Assert.Contains("%C3%A9", carrier[BaggagePropagator.BaggageHeaderName], StringComparison.Ordinal); + Assert.DoesNotContain("\u00E9", carrier[BaggagePropagator.BaggageHeaderName], StringComparison.Ordinal); + } + + [Theory] + [InlineData("caf\u00E9")] + [InlineData("emoji-\U0001F600")] + public void RoundTripNonAsciiValuePreservesOriginalString(string value) + { + var propagationContext = new PropagationContext( + default, + new Baggage(new Dictionary { { "key", value } })); + + var carrier = new Dictionary(); + this.baggage.Inject(propagationContext, carrier, Setter); + + var extracted = this.baggage.Extract(default, carrier, Getter).Baggage.GetBaggage(); + var entry = Assert.Single(extracted); + Assert.Equal("key", entry.Key); + Assert.Equal(value, entry.Value); + } + + [Theory] + [InlineData("\u00E9", "%C3%A9")] + [InlineData("\u4E2D", "%E4%B8%AD")] + [InlineData("\u00A3", "%C2%A3")] + [InlineData("\U0001F600", "%F0%9F%98%80")] + public void ValidateNonAsciiUtf8EncodingIsCorrect(string character, string expectedEncoding) + { + var propagationContext = new PropagationContext( + default, + new Baggage(new Dictionary { { "key", character } })); + + var carrier = new Dictionary(); + this.baggage.Inject(propagationContext, carrier, Setter); + + Assert.Contains(expectedEncoding, carrier[BaggagePropagator.BaggageHeaderName], StringComparison.Ordinal); + } + + [Fact] + public void ValidateNonAsciiInExtractedValueIsPreservedAfterPercentEncoding() + { + // Simulate a header that already has correctly UTF-8 percent-encoded non-ASCII + var carrier = new Dictionary + { + { BaggagePropagator.BaggageHeaderName, "key=%C3%A9" }, + }; + + var extracted = this.baggage.Extract(default, carrier, Getter).Baggage.GetBaggage(); + var entry = Assert.Single(extracted); + Assert.Equal("key", entry.Key); + Assert.Equal("\u00E9", entry.Value); + } } diff --git a/test/OpenTelemetry.Api.Tests/Context/Propagation/EnvironmentVariableCarrierTests.cs b/test/OpenTelemetry.Api.Tests/Context/Propagation/EnvironmentVariableCarrierTests.cs index cdacdb3d90a..cb6b81728d0 100644 --- a/test/OpenTelemetry.Api.Tests/Context/Propagation/EnvironmentVariableCarrierTests.cs +++ b/test/OpenTelemetry.Api.Tests/Context/Propagation/EnvironmentVariableCarrierTests.cs @@ -248,7 +248,7 @@ public void BaggagePropagator_RoundTripsThroughEnvironmentVariableCarrier() { var baggage = Baggage.Create(new Dictionary { - ["key 1"] = "value 1", + ["key1"] = "value 1", // space is not a valid token char in keys refer to #7051 ["key2"] = "value2", }); @@ -260,7 +260,7 @@ public void BaggagePropagator_RoundTripsThroughEnvironmentVariableCarrier() var extracted = propagator.Extract(default, EnvironmentVariableCarrier.Capture(carrier), EnvironmentVariableCarrier.Get); - Assert.Equal("key+1=value+1,key2=value2", carrier["BAGGAGE"]); + Assert.Equal("key1=value%201,key2=value2", carrier["BAGGAGE"]); AssertBaggageEqual(baggage.GetBaggage(), extracted.Baggage.GetBaggage()); }