fix(h1): prevent panic on non-UTF-8 response headers in trace logging (#1023) - #1039
Open
Aditya-9-6 wants to merge 2 commits into
Open
Aditya-9-6 wants to merge 2 commits into
Aditya-9-6 wants to merge 2 commits into
Conversation
When an upstream backend returns response headers containing non-UTF-8 bytes (e.g. ISO-8859-1 encoding or binary residue), 'str::from_utf8().unwrap()' in 'read_response_task' causes a panic. Replace 'str::from_utf8(...).unwrap()' with 'String::from_utf8_lossy(...)' so raw response header bytes are logged safely without risking process panics. Closes cloudflare#1023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
In
pingora-core/src/protocols/http/v1/client.rs(read_response_task), raw response header bytes were parsed usingstr::from_utf8(self.get_headers_raw()).unwrap()inside atrace!statement.When an upstream server returns response headers with non-UTF-8 bytes (such as ISO-8859-1 encodings or binary payloads), this
.unwrap()triggers a panic and aborts the proxy process. Furthermore, because arguments to logging macros can be evaluated eagerly when log level filters admit trace or dynamic filtering is used, this posed a major reliability risk in production.Fix
str::from_utf8(self.get_headers_raw()).unwrap()withString::from_utf8_lossy(self.get_headers_raw()). This borrows zero-copy when valid UTF-8 and safely replaces invalid byte sequences with replacement characters without ever panicking.read_response_header_non_utf8inpingora-core/src/protocols/http/v1/client.rsverifying that non-UTF-8 response headers (\xff) are processed safely byread_response_task().Verification
cargo fmt --all -- --checkpassed cleanly.cargo clippy -p pingora-core --libpassed with 0 warnings.cargo test -p pingora-core --lib read_response_header_non_utf8passed successfully.Closes #1023