Skip to content

Conversation

@floge07
Copy link
Contributor

@floge07 floge07 commented Aug 27, 2025

Resolves #368

I basically looked at the HttpRequestMessage.ToString and replicated it, except for the Headers appending part.

At first, I wanted to only redact/censor the individual values for sensitive Headers but that got too annoying. The DumpHeaders method is internal and replicating it seemed wrong. Then I tried cloning the RequestMessage object but with redacted headers and simply doing ToString on that object, but cloning the Content is really weird... Then I thought about a Regex Replace on the result string... 😑 yeah, nope.

So I just pushed this solution, that doesn't include any Headers at all. Maybe no one even cares about the Header values in general and I don't need to put more time into this.

Message before:

Unexpected response: StatusCode: 400 BadRequest, Content: '{"error":"bad_request","reason":"Cast from Array(JSON::Any) to String failed"}' from request: Method: PUT, RequestUri: 'http://host.docker.internal:15672/api/users/test', Version: 1.1, Content: System.Net.Http.Json.JsonContent, Headers:
{
  Authorization: Basic Z3Vlc3Q6Z3Vlc3Q=
  Transfer-Encoding: chunked
  Content-Type: application/json
}

Now:

Unexpected response: StatusCode: 400 BadRequest, Content: '{"error":"bad_request","reason":"Cast from Array(JSON::Any) to String failed"}' from request: Method: PUT, RequestUri: 'http://host.docker.internal:15672/api/users/test', Version: 1.1, Content: System.Net.Http.Json.JsonContent

…HttpRequestMessage.ToString anymore, it includes the header values e.g. "Authorization". Now append all the request properties ourself, except for Headers.
@micdenny
Copy link
Member

I think it's super reasonable to don't include the headers, usually it's a best practice to redact headers from any message than can be logged, and an exception is usually something that is logged, so for me it's a 👍

@floge07
Copy link
Contributor Author

floge07 commented Sep 1, 2025

So what now? Does everyone need to approve? How long do you think that will take?

@floge07
Copy link
Contributor Author

floge07 commented Sep 16, 2025

Hello? Anyone?

@floge07
Copy link
Contributor Author

floge07 commented Oct 1, 2025

Waiting for more than a month now for any further reaction. Sorry, now everyone's getting a ping.
@micdenny @zidad @Pliner @luigiberrettini

@zidad zidad merged commit d9b1fd9 into EasyNetQ:master Oct 1, 2025
8 checks passed
@floge07
Copy link
Contributor Author

floge07 commented Oct 1, 2025

@floge07
Copy link
Contributor Author

floge07 commented Oct 6, 2025

Oh right. Still need a new release for the change to be available on nuget.

@floge07
Copy link
Contributor Author

floge07 commented Oct 9, 2025

Oh right. Still need a new release for the change to be available on nuget.

@zidad Could you make one soon? Or do you usually want to wait to batch more changes? I hope not 😅

@zidad
Copy link
Member

zidad commented Oct 17, 2025

@floge07 sorry to make you wait, I'm publishing it now!

@zidad
Copy link
Member

zidad commented Oct 17, 2025

@zidad
Copy link
Member

zidad commented Oct 17, 2025

@floge07 if you're a heavy user of easynetq.management, are you interested in becoming a maintainer?

@floge07
Copy link
Contributor Author

floge07 commented Oct 22, 2025

@floge07 if you're a heavy user of easynetq.management, are you interested in becoming a maintainer?

Interesting offer, but I don't think it would make sense for me.

I mean I only ran into this "regression" because I finally updated from a 3 years old version and then I got such a quick initial response to the issue I opened, and then I was kind of stuck with a sunk cost fallacy to get this pr through. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The UnexpectedHttpStatusCodeException message leaks API credentials

3 participants