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 http redirects, where the target host differs #4420

Merged
merged 1 commit into from Dec 26, 2018

Conversation

Projects
None yet
2 participants
@enilfodne
Copy link
Contributor

enilfodne commented Dec 25, 2018

I've encountered an issue where an OPDS catalog was redirecting the client to a different host, but since the Host header is set in frontend/ui/widget/opdsbrowser.lua, the client retries, until it fails.

I've noticed, that before creating the request, socket/http automatically creates the Authorization (if username and password are provided) and Host headers, which is why i split the http and https request generation.

There's probably a better way to handle this (preferably by using a library that can do http/https seamlessly).

Please feel free to disregard this PR if does not conform to the code quality of the project.

@Frenzie
Copy link
Member

Frenzie left a comment

Looks decent. :-)

Show resolved Hide resolved frontend/ui/widget/opdsbrowser.lua Outdated
Show resolved Hide resolved frontend/ui/widget/opdsbrowser.lua Outdated
Show resolved Hide resolved frontend/ui/widget/opdsbrowser.lua Outdated
Show resolved Hide resolved frontend/ui/widget/opdsbrowser.lua Outdated
Fix socket.http redirect, where the target is a different host
Split https/http code-path (as socket.http can adjust the Authorization/Host headers automatically)
Warn the user when the protocol differs from http/https

@enilfodne enilfodne force-pushed the enilfodne:master branch from 902d363 to 220edbc Dec 26, 2018

@enilfodne

This comment has been minimized.

Copy link
Contributor

enilfodne commented Dec 26, 2018

Thanks for taking the time, the changes you've requested have been applied

@Frenzie Frenzie merged commit 295cc55 into koreader:master Dec 26, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@Frenzie

This comment has been minimized.

Copy link
Member

Frenzie commented Dec 26, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment