-
-
Notifications
You must be signed in to change notification settings - Fork 319
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
Resolve #383 redirect with protocol change #396
base: master
Are you sure you want to change the base?
Conversation
More results for this spec :
|
Thanks for looking into this issue! |
5f9d47f
to
4dd059b
Compare
My fix is running well with the original tests table and with the spec added here 2 days ago. Should it manage a maximum number of redirections? add a user option like the okhttp followSslRedirects? catch invalid url? match a more reduced set of redirect response codes? |
When this PR joins to repo? |
Hey guys, sorry for late response. I think this implementation is very dangerous. As far as I understand, you do not check if the redirect URL requests a HTTPS upgrade (requesting HTTP --> redirected to HTTPS) or a downgrade (requesting HTTPS --> redirected to HTTP). Downgrades shouldn't be allowed, or at least it should be configurable to prevent that. |
e8a8d65
to
0ccf64e
Compare
1a1c9e0
to
ed08534
Compare
449aa28
to
6fc8a7e
Compare
Fix #383