Skip to content

Follow redirects when checking served_by_pages? #19

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

Merged
merged 3 commits into from
Aug 6, 2015

Conversation

benbalter
Copy link
Contributor

A follow up to #13, this pull request makes the served_by_pages? redirect a bit more robust by actually following the location header of 30x redirects when checking if a given domain is in fact served by pages.

Previously, when we got a 30x redirect, we just requested the URL again, which is fine for DNS records pointed directly to pages.

In the case of management.cio.gov, for example, they proxy an actually-pages-served site through nginx to add SSL, which means we improperly said it wasn't served by pages because we didn't follow the redirect from HTTP to HTTPS.

To accomplish this, the check now uses Typhoeus, a wrapper for libcurl to handle the redirects a bit more robustly. I also pulled the uri and scheme variables into their own private methods, and moved to a HEAD request (which Pages supports), to speed up the check a bit by not downloading the page body.

Last, some of our existing served_by_pages? tests weren't actually checking if the site was served_by_pages?, now fixed. Heck, one test, choosealicense.com wasn't even pointing at a valid domain, yet the test still passed. We now actually look at served_by_pages? directly, rather than valid?.

/cc @jdennes, @spraints, @konklone

Fixes #17.

@benbalter benbalter self-assigned this Aug 5, 2015
@benbalter
Copy link
Contributor Author

Also cc @jessephus, as I know you use /pages check sometime for the served_by_pages? flag.

TL;DR: before, in some instances, if a user was using GitHub Pages to host their site, but pointing the DNS at domain that had a redirect (e.g., from HTTP to HTTPS, or even to the Pages server directly), we would return false because we didn't follow the redirect as part of the check. Once merged, we will now properly follow all redirects to check that the page a user would see if they had loaded the domain in their browser is reporting itself as our Pages server.

benbalter added a commit that referenced this pull request Aug 6, 2015
Follow redirects when checking served_by_pages?
@benbalter benbalter merged commit 3cd2fdf into master Aug 6, 2015
@benbalter benbalter deleted the follow-redirects branch August 6, 2015 14:23
@jdennes
Copy link
Member

jdennes commented Aug 6, 2015

Very nice!

@spraints
Copy link
Member

spraints commented Aug 6, 2015

🎊

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.

served_by_pages? check should follow 30x redirects
3 participants