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

default user-agent header is malformed #286

Closed
volfco opened this issue Jan 1, 2021 · 5 comments · Fixed by #287
Closed

default user-agent header is malformed #286

volfco opened this issue Jan 1, 2021 · 5 comments · Fixed by #287
Labels
bug Something is borken

Comments

@volfco
Copy link

volfco commented Jan 1, 2021 •
edited
Loading

This drove me insane to figure out. This library is sending out a malformed user-agent header.

This library sends user-agent:curl/7.74.0 isahc/1.0.1 compared to stock cURL: User-Agent: curl/7.74.0.

There is a space omitted after the colon. While this doesn't seem to be invalid HTTP 1/1, It will trip up crappy http parsers who assume : for header fields.

Thanks for building one of the only Rust HTTP libraries that supports Unix sockets!
@sagebind
Copy link
Owner

sagebind commented Jan 1, 2021

Thanks for reporting this; sorry this caused so much trouble for you!

This looks to be how Isahc sends all custom header fields with HTTP/1.x (without whitespace), not just the user agent. This is compliant with RFC 7230, section 3.2. However, this is a bit unusual and it would probably be better for us to include a single space after the : to avoid confusing bad HTTP parsers. I will submit a PR to change this and release a patch version shortly.

@sagebind sagebind added the bug Something is borken label Jan 1, 2021
sagebind added a commit that referenced this issue Jan 1, 2021
@sagebind
Always use a single space after colon for HTTP 1 headers
bbb79a1 
Previously headers were sent with no whitespace before or after the colon (`:`). This is legal according to RFC 7230, section 3.2, but is somewhat unusual and can cause poorly-written servers to choke on parsing the request. Update the encoder to always add a single space after the colon to be in line with what most parsers should expect.

This has no effect on HTTP/2 or newer, since they use a binary format for headers.

Fixes #286.
@sagebind sagebind mentioned this issue Jan 1, 2021
Merged
sagebind added a commit that referenced this issue Jan 1, 2021
@sagebind
Always use a single space after colon for HTTP 1 headers (#287)
61dec2d 
Previously headers were sent with no whitespace before or after the colon (`:`). This is legal according to RFC 7230, section 3.2, but is somewhat unusual and can cause poorly-written servers to choke on parsing the request. Update the encoder to always add a single space after the colon to be in line with what most parsers should expect.

This has no effect on HTTP/2 or newer, since they use a binary format for headers.

Fixes #286.
@sagebind
Copy link
Owner

sagebind commented Jan 1, 2021

This is now fixed in the 1.0.2 release!

@volfco
Copy link
Author

volfco commented Jan 4, 2021

Thanks for the quick fix! Awesome work here

@numerodix
Copy link

I just hit this problem today - 18 months later - when using surf 2.3.2 (latest release) 🤣

Glad to see my bug report isn't needed. 😄

@sagebind
Copy link
Owner

Unfortunately Surf uses a very old version of Isahc, so there's not much I can do about that other than advocating that they upgrade to a newer version, which included this, and many other fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something is borken
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Always use a single space after colon for HTTP 1 headers sagebind/isahc
3 participants
@numerodix @volfco @sagebind