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

"infinite redirection loop detected" error on many websites, but Firefox and curl work fine #49

Closed
Shnatsel opened this issue Feb 16, 2021 · 8 comments

Comments

@Shnatsel
Copy link

On some websites, e.g. http://mockup.love, minreq fails with the following error:

infinite redirection loop detected

Firefox and curl work fine.

80,000+ websites out of the top million from Feb 3 Tranco list are affected.

Tested using this code. Test tool output from all affected websites: minreq-infinite-redirection.tar.gz

I admit, I am a bit surprised to see a check for this particular condition as opposed to simply setting a limit on the number of redirections.

@neonmoe
Copy link
Owner

neonmoe commented Feb 18, 2021

This seems to work as intended. Firefox does indeed show a page, but I can't figure out where it gets the page from. Maybe it reads the body of the 301 and simply displays that after N loops? Curl, on the other hand, seems to simply return the 301.

I tested the following manually with curl (from the .tar.gz):

  • mockup.love
  • lumibear.com.au
  • dogstudio.co
  • your-travel.uk

@Shnatsel
Copy link
Author

You need to pass the --location flag to curl, otherwise it won't follow any redirections. With that flag it does get to the HTML eventually. Try curl -v --location mockup.love

My full testing setup (including for curl) can be found here: https://github.com/Shnatsel/rust-http-clients-smoke-test

@joeried
Copy link

joeried commented May 31, 2021

I have debugged this problem a bit and think I have found the cause.

These servers seem to match the host and depending on that redirect the user to URLs.
minreq always sends the port as part of the host header, even if it is the default port (i.e. 80 for HTTP and 443 for HTTPS). For these servers, it makes a difference whether an HTTP request is sent with "Host: example.org" or "Host: example.org:443".

In this case the server recognizes the host "example.org:443" and wants to forward to "example.org". minreq then tries to follow this and sends another request with "example.org:443". This starts the fun all over again.

I quickly wrote a patch, joeried@258500b
This seems to solve the problems with the domains mentioned above.
But I am not sure if such behavior is desired and if the place where I work around the problem is the right one.
However, it seems that browser and curl do not send the default port either (even if you explicitly specify it). So it could be the right solution.

@neonmoe
Copy link
Owner

neonmoe commented Jun 1, 2021

Oh, that's definitely an issue. Thanks for investigating, I was stumped! Your patch would work for this specific issue, but I think this should be fixed in an architectural way: Request should have a port field, instead of the port being included in the host. If the host has a port, then the port should be set to that, otherwise 80 or 443 based on the scheme. I'll write a patch for this tomorrow and push it as 2.4.1 so we can get this out of the way :)

@joeried
Copy link

joeried commented Jun 1, 2021

The solution you propose is probably much nicer, because it eliminates unnecessary adding/removing of the port.

By the way, it looked yesterday like my proof of concept solution would also solve #48 (at least for the one URL given)
So it might be worth testing your patch more extensively against the test cases from this issue as well.

@neonmoe
Copy link
Owner

neonmoe commented Jun 1, 2021

Thanks for the heads up! #48 is such an odd issue that I could see this fixing it, maybe some https servers don't like weird hostnames.

@neonmoe
Copy link
Owner

neonmoe commented Jun 3, 2021

Ended up being a relatively small change, but I think this is now fixed with 318d2d2. Didn't test extensively yet (nor if the change fixed #48), but now the ports are handled properly in any case.

I'll close this when I get around to ensuring that this is actually fixed.

@neonmoe
Copy link
Owner

neonmoe commented Jun 5, 2021

Tested this, 318d2d2 seems to indeed fix the issue.

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