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

Fix KeyError with missing Host in HTTP::Request #22

Conversation

felixbuenemann
Copy link

This fixes a KeyError: Missing hash key: HTTP::Headers::Key(@name="Host") exception if you use HTTP::client#exec with a HTTP::Request instance that has no "Host" header set.

Fixes #5

@felixbuenemann felixbuenemann changed the title Fix KeyError with missing Host in HTTP::Request [WIP] Fix KeyError with missing Host in HTTP::Request Jul 20, 2018
@felixbuenemann
Copy link
Author

Marking WIP, because I think simply returning true is wrong and the code should instead fallback to parsing the content of host in the HTTP::Client.

@felixbuenemann
Copy link
Author

Btw. spec failures are unrelated and fixed in #23.

When calling `HTTP::Client#exec` with an `HTTP::Request` instance the
Host header is not automatically filled by the http client, so we need
to add it manually or the `host_matches?` check in the stub will fail
with a `KeyError` exception.
@felixbuenemann felixbuenemann force-pushed the fix-crash-with-missing-host-header branch from 4655868 to 00574dd Compare July 21, 2018 13:24
@felixbuenemann felixbuenemann changed the title [WIP] Fix KeyError with missing Host in HTTP::Request Fix KeyError with missing Host in HTTP::Request Jul 21, 2018
@felixbuenemann
Copy link
Author

felixbuenemann commented Jul 21, 2018

OK, I've revised the code to instead set the Host header on the request if missing and added a spec.

This is now ready for review!

@aisrael
Copy link

aisrael commented Aug 17, 2018

When will this be merged? In the mean time we'll have merge @felixbuenemann's branch into our own local fork that also has an outstanding PR :(

@bcardiff
Copy link
Contributor

Thanks @felixbuenemann . #25 was merged and include this fixes also.
Apologies for the delay.

@bcardiff bcardiff closed this Aug 17, 2018
@felixbuenemann felixbuenemann deleted the fix-crash-with-missing-host-header branch August 19, 2018 06:26
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.

Missing hash key: HTTP::Headers::Key(@name="Host") calling client.exec with HTTP::Request
3 participants