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

Allow HEAD to redirect, but default to it not #160

Closed
wants to merge 1 commit into from

Conversation

oxinabox
Copy link
Member

Right now, if you are doing a HEAD request redirects are never followed.
but they are for GET and all other methods.

My reading of the spec
is that HEAD and GET are both allowed to follow redirects.
though I understand why maybe one would not want to with HEAD since you could be looking to check if there was a redirect going on or some other header information at that point.

So the way I made it possible to have it both ways (and not change behaviour (much))
is to make using the HEAD method set the default value of maxredirects to 0,
rather than blocking redirects directly.
I thinking any user setting maxredirects to any value (other than 0 themselves) when using HEAD, clearly does want redirects to happen.

This needs tests and docs, but I wanted to put it forward for discussion before I took it any further

@samoconnor
Copy link
Contributor

See also https://github.com/JuliaWeb/HTTP.jl/pull/135/files#r161218707

I've come around to the view that it is better to just delete the method == "HEAD" condition so that redirect is enabled for "HEAD" by default. The user can always do redirect=false if they really want to see the pre-redirect headers. I think the most likely use of "HEAD" is that you want to get some meta-data about the eventual target of the request (a timestamp, a checksum, content length). I think it is unlikely that looking at the meta-data of the redirect response is often useful.

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.

2 participants