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

Redirects to https domains break in 1.40 #832

Closed
arsduo opened this issue Feb 15, 2017 · 8 comments
Closed

Redirects to https domains break in 1.40 #832

arsduo opened this issue Feb 15, 2017 · 8 comments

Comments

@arsduo
Copy link

arsduo commented Feb 15, 2017

Thanks for the great gem! I discovered an error this week: #765 breaks HTTPS redirect URLs.

In app/controllers/devise_token_auth/omniauth_callbacks_controller.rb (line 17), the new URL is constructed using URI::HTTP.build. According to the documentation for URI::HTTP, though, build does not accept the scheme parameter that the code passes in.

Reproduction:

# "https" is the value returned by Rack::Request for scheme when appropriate (see http://www.rubydoc.info/gems/rack/Rack/Request/Helpers#scheme-instance_method) 
URI::HTTP.build(scheme: request.scheme, host: request.host, port: request.port, path: path).to_s

URI::HTTP.build(scheme: "https", host: "google.com", port: 443, path: "/").to_s
=> "http://google.com:443/"

I'll make a PR to fix this later this week if I have a chance.

@arcreative
Copy link

Ahmagawd, THANK YOU. I was tearing my hair out all day thinking it was my fault. Reverting to 0.1.39 fixed it for now... Going to go drink heavily now 🍻

@zachfeldman
Copy link
Contributor

@arsduo we'd love your PR! Feel free to submit.

@zachfeldman
Copy link
Contributor

...otherwise we'll close this out in 7 days

@arcreative
Copy link

...but he didn’t break it ;-) This is a pretty big issue, and a hard one to track down, at that...

@zachfeldman
Copy link
Contributor

@arcreative this is a volunteer-run project, so if something needs to get done, it's on people that use this gem to implement it. That's why I asked if he could write a PR. You can also feel free to submit one if you'd like! We're going to try to much more actively monitor open pull requests and prioritize issues for the community to take on. I'm also going to try to write some code in addition to maintaining. But nobody is paid or is paying anyone to maintain this gem.

@zachfeldman
Copy link
Contributor

....actually going to leave this open because there was at least a comment. @arcreative we await your PR! :)

@arcreative
Copy link

This seems to be fixed via 5300ea7. Feel free to close, thanks!

@zachfeldman
Copy link
Contributor

Great!

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

No branches or pull requests

3 participants