Skip to content

Activity.Baggage is url encoded when using HttpClient but not url decoded when read by AspNetCore host #18600

New issue

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

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

Already on GitHub? # to your account

Closed
theggelund opened this issue Jan 27, 2020 · 3 comments
Labels
area-hosting Includes Hosting area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug. good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue

Comments

@theggelund
Copy link

theggelund commented Jan 27, 2020

Describe the bug

When calling an AspNetCore service with HttpClient and activity has any baggage, all the entries are url encoded and added to Correlation-Context http header. When the service receives the request and reads the Correlation-Context header the keys and values are not url decoded.

Also, according to this description only the value should be value should be url encoded.
Correlation Context

The code that does not url decode is here HostingApplicationDiagnostics.cs

To Reproduce

class Program
    {
        static async Task Main(string[] args)
        {
            Activity.DefaultIdFormat = ActivityIdFormat.W3C;
            DiagnosticListener.AllListeners.Subscribe(new Listener());

            string baggage = string.Empty;

            await new HostBuilder()
                .ConfigureWebHost(webHost =>
                {
                    webHost
                        .UseKestrel(opt => opt.Listen(IPAddress.Loopback, 55555))
                        .Configure(app =>
                        {
                            app.Run(async ctx =>
                            {
                                baggage = string.Join(";",
                                    Activity.Current.Baggage.Select(entry => $"{entry.Key}:{entry.Value}"));
                                
                                await ctx.Response.WriteAsync("Hello World");
                            });
                        });
                })
                .StartAsync();

            var activity = new Activity("Call server");

            string key = "env";
            string value = "+-%/";
            
            activity.AddBaggage(key, value);
            activity.Start();
            
            var client = new HttpClient(new HttpClientHandler());
            await client.GetStringAsync("http://localhost:55555/test");

            if (!baggage.Contains($"{key}={value}"))
            {
                Console.Error.WriteLine("This should have worked => " + baggage);
            }
        }
    }

    class Listener : IObserver<DiagnosticListener>, IObserver<KeyValuePair<string, object>>
    {
        public void OnCompleted() { }

        public void OnError(Exception error) { }

        public void OnNext(KeyValuePair<string, object> value) { }

        public void OnNext(DiagnosticListener value)
        {
            value.Subscribe(this);
        }
    }

csproj

<Project Sdk="Microsoft.NET.Sdk">

    <PropertyGroup>
        <OutputType>Exe</OutputType>
        <TargetFramework>netcoreapp3.1</TargetFramework>
    </PropertyGroup>

    <ItemGroup>
      <PackageReference Include="Microsoft.AspNetCore.Mvc.Testing" Version="3.1.1" />
    </ItemGroup>

</Project>

Further technical details

  • ASP.NET Core version
    3.1.1
  • Include the output of dotnet --info
    .NET Core SDK (reflecting any global.json):
    Version: 3.1.101
    Commit: b377529961

Runtime Environment:
OS Name: Windows
OS Version: 10.0.18363
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\3.1.101\

Host (useful for support):
Version: 3.1.1
Commit: a1388f194c

.NET Core SDKs installed:
3.1.101 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
Microsoft.AspNetCore.All 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.App 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET Core runtimes or SDKs:
https://aka.ms/dotnet-download

@analogrelay
Copy link
Contributor

Seems like a legit bug.

@analogrelay analogrelay added good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue labels Jan 27, 2020
@analogrelay analogrelay added this to the Next sprint planning milestone Jan 27, 2020
@vaughanr
Copy link
Contributor

Hi, I attempted to fix this in a pull request above. I only url decoded the baggage value based on the spec in the link that @theggelund supplied. It would be trivial to do the same with the key if the fact that the key is url encoded is expected behavior. Please let me know if I need to make any changes?

@JunTaoLuo
Copy link
Contributor

Thanks for the contribution!

@JunTaoLuo JunTaoLuo added the bug This issue describes a behavior which is not expected - a bug. label Feb 19, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Mar 24, 2020
@amcasey amcasey added the area-hosting Includes Hosting label Jun 1, 2023
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
area-hosting Includes Hosting area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug. good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue
Projects
None yet
Development

No branches or pull requests

6 participants