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

Take current scheme (HTTP/S) in to account when redirecting. #27

Merged
merged 2 commits into from
Sep 30, 2023

Conversation

paulholden
Copy link
Member

See MDL-79361

Failing unit tests relating to curl redirects, because download.moodle.org now redirects everything HTTP -> HTTPS with a 301 response, then the rest redirection script redirects everything back to HTTP:

$ curl -iL "http://download.moodle.org/unittest/test_redir.php?redir=2&verbose=1"
HTTP/1.1 301 Moved Permanently
Date: Tue, 26 Sep 2023 21:41:40 GMT
Transfer-Encoding: chunked
Connection: keep-alive
Cache-Control: max-age=3600
Expires: Tue, 26 Sep 2023 22:41:40 GMT
Location: https://download.moodle.org/unittest/test_redir.php?redir=2&verbose=1
Server: cloudflare
CF-RAY: 80ceac219b0924e4-LHR

HTTP/2 302 
date: Tue, 26 Sep 2023 21:41:40 GMT
content-type: text/html; charset=UTF-8
location: http://download.moodle.org:80/unittest/test_redir.php?redir=1
cf-cache-status: DYNAMIC
server: cloudflare
cf-ray: 80ceac222e90499a-LHR

HTTP/1.1 301 Moved Permanently
Date: Tue, 26 Sep 2023 21:41:40 GMT
Transfer-Encoding: chunked
Connection: keep-alive
Cache-Control: max-age=3600
Expires: Tue, 26 Sep 2023 22:41:40 GMT
Location: https://download.moodle.org/unittest/test_redir.php?redir=1
Server: cloudflare
CF-RAY: 80ceac22bcbd24e4-LHR

HTTP/2 302 
date: Tue, 26 Sep 2023 21:41:40 GMT
content-type: text/html; charset=UTF-8
location: http://download.moodle.org:80/unittest/test_redir.php?done=1
cf-cache-status: DYNAMIC
server: cloudflare
cf-ray: 80ceac22df74499a-LHR

HTTP/1.1 301 Moved Permanently
Date: Tue, 26 Sep 2023 21:41:40 GMT
Transfer-Encoding: chunked
Connection: keep-alive
Cache-Control: max-age=3600
Expires: Tue, 26 Sep 2023 22:41:40 GMT
Location: https://download.moodle.org/unittest/test_redir.php?done=1
Server: cloudflare
CF-RAY: 80ceac231d5a24e4-LHR

HTTP/2 200 
date: Tue, 26 Sep 2023 21:41:41 GMT
content-type: text/html; charset=UTF-8
vary: Accept-Encoding
cf-cache-status: DYNAMIC
server: cloudflare
cf-ray: 80ceac233812499a-LHR

done

So the constant flip/flopping of HTTP/HTTPS breaks assumptions in our tests, e.g. those related to CURLOPT_MAXREDIRS

Copy link
Contributor

@jrchamp jrchamp left a comment

Choose a reason for hiding this comment

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

This makes sense to me. Based on how it's currently hosted, I don't expect the http condition to ever be followed, but this is a good minimal change.

test_redir.php Outdated Show resolved Hide resolved
Co-authored-by: Jonathan Champ <[email protected]>
@stronk7 stronk7 merged commit 94bb107 into moodlehq:master Sep 30, 2023
2 checks passed
@paulholden paulholden deleted the redir-https branch September 30, 2023 17:04
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.

3 participants