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

LibWeb: Avoid re-encoding response headers #2890

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

F3n67u
Copy link
Contributor

@F3n67u F3n67u commented Dec 12, 2024

isomorphic encoding a value that has already been encoded will result in garbage data. response_headers is already encoded in ISO-8859-1, we cannot use from_string_pair, as it calls isomorphic_decode.

Follow-up of #1893

@F3n67u F3n67u force-pushed the response-header-latin1 branch 3 times, most recently from 8e6b30a to 002ab62 Compare December 13, 2024 02:53
@F3n67u F3n67u marked this pull request as ready for review December 13, 2024 05:46
@Gingeh
Copy link
Contributor

Gingeh commented Dec 13, 2024

I believe this also fixes the main issue that prevented me from putting a VERIFY(code_point <= 0xFF) in the isomorphic_encode implementation.

@F3n67u F3n67u force-pushed the response-header-latin1 branch from 002ab62 to 5664768 Compare December 13, 2024 18:26
isomorphic encoding a value that has already been encoded will
result in garbage data. `response_headers` is already encoded in
ISO-8859-1/latin1, we cannot use `from_string_pair`, as it triggers
ISO-8859-1/latin1 encoding.

Follow-up of LadybirdBrowser#1893
@F3n67u F3n67u force-pushed the response-header-latin1 branch from 5664768 to e5a1a0b Compare December 13, 2024 18:27
@F3n67u
Copy link
Contributor Author

F3n67u commented Dec 13, 2024

I believe this also fixes the main issue that prevented me from putting a VERIFY(code_point <= 0xFF) in the isomorphic_encode implementation.

I uncomment VERIFY(code_point <= 0xFF) in this PR, looks good!

@avbdr
Copy link

avbdr commented Dec 14, 2024

i assume that should fix opening of the cnn.com. It currently fails due to wrongly encoded cookie header

@gmta gmta merged commit e0c0668 into LadybirdBrowser:master Dec 17, 2024
6 checks passed
@F3n67u F3n67u deleted the response-header-latin1 branch December 17, 2024 18:28
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.

4 participants