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

Let Host be automatically inferred on redirects #410

Merged
merged 1 commit into from
May 19, 2017

Conversation

janko
Copy link
Member

@janko janko commented May 18, 2017

When redirecting on http://localhost:<port> URLs, the Host will be correctly set to http://localhost:<port> on the initial request, but on following the redirect the port is lost, because we're only assigning the host.

Therefore when we initialize the follow-up request object we de-assign "Host", and let HTTP::Request set it in the same way as it is set on the initial request.

Since we're stubbing redirects at the moment, I didn't know how to write a test for this.

@tarcieri
Copy link
Member

This seems to break quite a few tests

@janko
Copy link
Member Author

janko commented May 18, 2017

Oh, that's probably because I forgot that HTTP::Response#headers is a HTTP::Headers instance. I'll fix it tomorrow.

When redirecting on "http://localhost:<port>" URLs, the Host will be
correctly set to "http://localhost:<port>" on the initial request, but
on following the redirect the port is lost, because we're only assigning
the host.

Therefore when we initialize the follow-up request object we de-assign
"Host", and let HTTP::Request set it in the same way as it is set on the
initial request.
@janko janko force-pushed the keep-port-in-host-when-redirecting branch from c08d085 to 561fb70 Compare May 18, 2017 17:33
@janko
Copy link
Member Author

janko commented May 18, 2017

Fixed the implementation and added a test, with verifying that it actually fails without this change.

@ixti
Copy link
Member

ixti commented May 18, 2017

Thanks for the PR. Can you please add spec for the issue too?

@janko
Copy link
Member Author

janko commented May 19, 2017

I added it already 😃

@ixti
Copy link
Member

ixti commented May 19, 2017

Oh. Indeed. Missed that somehow. Sorry, and thanks again!

@ixti ixti merged commit 823c7c2 into httprb:master May 19, 2017
@janko janko deleted the keep-port-in-host-when-redirecting branch May 19, 2017 04:24
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Dec 9, 2017
pkgsrc changes:
- sort DEPENDS

Upstream changes (from CHANGES.md):

## 3.0.0 (2017-10-01)

* Drop support of Ruby `2.0` and Ruby `2.1`.
  ([@ixti])

* [#410](httprb/http#410)
  Infer `Host` header upon redirects.
  ([@janko-m])

* [#409](httprb/http#409)
  Enables request body streaming on any IO object.
  ([@janko-m])

* [#413](httprb/http#413),
  [#414](httprb/http#414)
  Fix encoding of body chunks.
  ([@janko-m])

* [#368](httprb/http#368),
  [#357](httprb/http#357)
  Fix timeout issue.
  ([@HoneyryderChuck])
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.

3 participants