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
net/http: HEAD responses should include the correct Content-Type #2886
Labels
Milestone
Comments
Correction: This is not afa99ad294e7+ tip. Copy-paste... I'm submitting this as a separate issue/CL because, although the change seems simple, this would require handling body completely differently in HEAD responses (bodyAllowed, ErrBodyNotAllowed, etc.), and so changing this may not be desirable for Go1. |
Current golang-nuts discussion: https://groups.google.com/d/topic/golang-nuts/XS42mnv2Et0/discussion For many cases, ServeContent already counts as a way of transparently handling both HEAD and GET methods if an io.ReadSeeker can be provided. Between DetectContentType and knowing the type ahead of time (i.e. always application/json) covers many of the other cases. If sniffing support for HEAD in ResponseWriter were added, it may be wise to have it panic with a sentinel on the first Write after sniffLen bytes had already been written (so a Write of 513 bytes won't panic, but the next Write of 1 byte will). This will prevent naive apps from wasting cpu time writing megabytes of data as if it were a GET request. The sentinel could be detected in the existing panic-recovery mechanism and log an informative "fix this app" message, but otherwise consider the response successful. |
I like the idea about a panic sentinel after writing 512 bytes in a HEAD response. (or 0 bytes in a HEAD response if the Content-Type was already written). I can't think of any precedent to that, though, so I'm wondering what the rest of the Go team thinks. Owner changed to @dsymonds. |
The Content-Type sniffing was always intended as a "do the right thing" default mechanism so the most common situations would not require explicit effort from the programmer to get the correct HTTP behaviour. In that sense, using the first 512 bytes of the response for sniffing HEAD follows that. On the other hand, HEAD is explicitly meant to be a lightweight GET for finding the metadata about an object without transferring the object. Note that the RFC only says the HEAD response headers SHOULD (not "MUST") be the same as for GET, so there's no requirement for Content-Type to be present for a HEAD request. Further, the whole point of sniffing to auto-set a Content-Type for a GET is to help ensure that HTTP clients (mostly browsers) behave correctly and don't do their own sniffing; a HEAD response has no such concerns since, by definition, there is no body for an HTTP client to sniff and misinterpret. In this sense sniffing for HEAD does not seem necessary or important. Note that there is *nothing* that precludes an HTTP handler from explicitly setting Content-Type, nor explictly handling HEAD differently from GET. In other words, for those who particularly care there is already a way to do it. The question of this bug is whether to try to do it automatically for simplistic handlers, namely those that just start writing to their http.ResponseWriter. My inclination would be that HEAD is meant to be more lightweight than GET, and there's no need to sniff. The one thing that would really sway me is an answer to this question: What HTTP clients particularly care about the Content-Type in a HEAD response? |
After thinking about this some more, I think the current situation is fine. Handlers that care can set a content type, and as far as I can tell the default behaviour (no Content-Type header sent in response to HEAD requests) does not cause problems for any known HTTP client. If someone wants this reopened I would suggest noting a practical problem with the current behaviour (e.g. a particular HTTP client that misbehaves because of this) along with a proposed solution. Status changed to WorkingAsIntended. |
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
The text was updated successfully, but these errors were encountered: