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.

Sign up for GitHub

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

Jump to bottom

Option to allow leading whitespace in first HTTP/1 response header #3285

Open
bagder opened this issue Aug 7, 2023 · 4 comments · May be fixed by #3643
Open

Option to allow leading whitespace in first HTTP/1 response header #3285

bagder opened this issue Aug 7, 2023 · 4 comments · May be fixed by #3643
Labels
A-client Area: client. A-http1 Area: HTTP/1 specific. B-upstream Blocked: needs a change in a dependency or the compiler. C-feature Category: feature. This is adding a new feature. E-easy Effort: easy. A task that would be a great starting point for a new contributor.

Comments

@bagder
Copy link
Contributor

bagder commented Aug 7, 2023 •
edited
Loading

Version
current git master

Platform
Linux

Description
hyper rejects HTTP responses where the first header has leading whitespace. This is spec compliant.

The popular browsers accept such headers and therefore we are now converting curl to (again) also allow them (in curl/curl#11607).

hyper's current behavior prevents it from getting content other widely used HTTP clients can fetch. Do you think that is perhaps reason to reconsider this stance and allow this violation? It would make life in curl easier as right now curl+hyper builds have different behavior than curl with its native HTTP implementation.
@bagder bagder added the C-bug Category: bug. Something is wrong. This is bad! label Aug 7, 2023
@seanmonstar
Copy link
Member

I guess we could allow it in hyper too, perhaps when also enabled to allow obsolete line-folding. For anyone wanting to do this, the fix would need to be in httparse.

@seanmonstar seanmonstar added A-client Area: client. E-easy Effort: easy. A task that would be a great starting point for a new contributor. B-upstream Blocked: needs a change in a dependency or the compiler. C-feature Category: feature. This is adding a new feature. A-http1 Area: HTTP/1 specific. and removed C-bug Category: bug. Something is wrong. This is bad! labels Aug 30, 2023
@seanmonstar seanmonstar moved this from Triage to Cause Known in hyper in curl Aug 30, 2023
@seanmonstar seanmonstar changed the title Leading whitespace in first HTTP/1 response header? Option to allow leading whitespace in first HTTP/1 response header Aug 30, 2023
@PGIII PGIII mentioned this issue Sep 7, 2023
Merged
@JordanllHarper
Copy link

JordanllHarper commented Nov 4, 2023 •
edited
Loading

I would be interested in picking this up edit: once this is unblocked from httpparse.

@emirakdere
Copy link

I am interested in picking this up, can make the change in httparse

@vkobinski
Copy link

This issue was resolved at seanmonstar/httparse#147

@dswij dswij linked a pull request Apr 22, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-client Area: client. A-http1 Area: HTTP/1 specific. B-upstream Blocked: needs a change in a dependency or the compiler. C-feature Category: feature. This is adding a new feature. E-easy Effort: easy. A task that would be a great starting point for a new contributor.
Projects
hyper in curl
Status: Cause Known
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(http1): add option allowng leading whitespaces before first header dswij/hyper
5 participants
@seanmonstar @bagder @emirakdere @JordanllHarper @vkobinski