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

double quote "Sensitive" header value #584

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

haruska
Copy link

@haruska haruska commented Jan 24, 2023

In the implementation of Debug for HeaderValue if a value is not marked sensitive, the value is written to the formatter with double quotes added. However, if marked sensitive the unquoted static string Sensitive is instead written to the formatter. This causes issues when attempting to output headers during tracing/logging.

This PR adds double quotes to the "Sensitive" string output in the case of a sensitive HeaderValue.

@seanmonstar
Copy link
Member

It would be interesting to hear what issues that causes. The existing implementation purposefully doesn't wrap the word in quotes, so as to not be confused with a value that literally has the string "Sensitive".

@haruska
Copy link
Author

haruska commented Jan 25, 2023

The existing implementation purposefully doesn't wrap the word in quotes

That's fair. My specific issue was using tower-http DefaultMakeSpan to output http headers. Specifically Line 91 adds a headers key with the value being the map of http headers. The header values get outputted with double quotes.

This works fine until a header value gets marked sensitive. Then, that header value is output as Sensitive (without double quotes) while other values are quoted. Unfortunately, I'm outputting as JSON which makes the Sensitive output invalid.

The answer could just be I need to write a serializer for the headers which check the same sensitive flag and set a known string in the JSON output. However, it might make more sense (or be less confusing for other devs) for the tower-http default MakeSpan to just continue to work with marked sensitive values.

Maybe some more obvious string like "<REDACTED>"? It would just be nice to have the output always be quoted strings.

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.

None yet

2 participants