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

use URI::HTTPS to generate HTTPS redirects #864

Merged
merged 1 commit into from
Apr 9, 2017

Conversation

cgc
Copy link
Contributor

@cgc cgc commented Apr 8, 2017

When deploying my app, I noticed the callback URL (after this redirect) was stalling, trying to load http://my-app.herokuapp.com:443/api/auth/twitter/callback. Playing with URI::HTTP.build and URI::HTTPS.build, I noticed the scheme kwarg is ignored

> URI::HTTP.build(scheme: 'http', host: 'hi.com', port: 80).to_s
=> "http://hi.com"
> URI::HTTP.build(scheme: 'https', host: 'hi.com', port: 443).to_s
=> "http://hi.com:443"
> URI::HTTPS.build(scheme: 'http', host: 'hi.com', port: 80).to_s
=> "https://hi.com:80"
> URI::HTTPS.build(scheme: 'https', host: 'hi.com', port: 443).to_s
=> "https://hi.com"

It seems like the best solution is to switch which build() we are using based on the request scheme. I modeled my change after this PR fog/fog#2159 which seems to be the same issue. I was unable to write a test because I don't think request_via_redirect or get_via_redirect supports having a scheme like https.

@booleanbetrayal
Copy link
Collaborator

LGTM! Will look at coverage after it lands. Thanks @cgc !

@booleanbetrayal booleanbetrayal merged commit ac48a53 into lynndylanhurley:master Apr 9, 2017
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.

2 participants