-
Notifications
You must be signed in to change notification settings - Fork 84
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
Logic issue with is_redirect method #66
Comments
From #67:
# ...
and (
httpwww.redirect_eventually_to_external or
http.redirect_eventually_to_external or
httpswww.redirect_eventually_to_external or
https.redirect_eventually_to_external
) I closed #67, and am moving the proposed resolution here for tracking. |
Just noting for myself and others that this is still an issue, and affects at least one second-level domain in exim.gov: EDIT: URL is now https://s3-us-gov-west-1.amazonaws.com/cg-4adefb86-dadb-4ecf-be3e-f1c7b4f6d084/live/cache/pshtt/exim.gov.json |
The way is_redirect is currently written, a domain will get flagged as a redirect if all endpoints are down or otherwise code 400-ing. For example, imagine domain.tld, which only has its https endpoint up and happens to be returning for a 404 for its index. That will return
true
for is_redirect, which is confusing (and happening a bunch where I am).I'm curious if that whole logic block can be simplified to check and see if any of the end points are returning a code 300. That entire block starting on line 527 might reasonably be reduced to something like:
I'll submit a pull request and we can hash it out there.
The text was updated successfully, but these errors were encountered: