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

try encode web seed redirect location #7640

Merged
merged 2 commits into from May 11, 2024
Merged

Conversation

ajax16384
Copy link
Contributor

@ajax16384 ajax16384 commented Feb 28, 2024

web server may redirect with non encoded paths (like "Location: http://newserver.net/with space/file.bin" instead of "Location: http://newserver.net/with%20space/file.bin")
let's try encode such values as curl or wget

Copy link
Owner

@arvidn arvidn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to have a test exercising this case as well.
you can add a new or extend an existing test for redirects here:
simulation/test_web_seed.cpp

@@ -666,6 +666,7 @@ void web_peer_connection::handle_redirect(int const bytes_left)
file_index_t const file_index = m_file_requests.front().file_index;

location = aux::resolve_redirect_location(m_url, location);
location = maybe_url_encode(location);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there should be a comment here explaining that this is a work-around for buggy HTTP servers, that fail to encode the redirect URL.

@arvidn
Copy link
Owner

arvidn commented Mar 16, 2024

re-running CI

@arvidn arvidn closed this Mar 16, 2024
@arvidn arvidn reopened this Mar 16, 2024
@arvidn arvidn closed this Apr 21, 2024
@arvidn arvidn reopened this Apr 21, 2024
@arvidn arvidn merged commit fd2dd81 into arvidn:master May 11, 2024
108 of 123 checks passed
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.

None yet

2 participants