Saw multiple response.WriteHeader calls

[arlolra]

Saw multiple response.WriteHeader calls

[arlolra]

Seems to be caused by the HEAD method.

Reproduce with,
curl -I https://check2.torproject.org

[Ryman]

Not sure what else to do with Head requests, since the content does exist, would it make sense to 404 a request that wasn't from a Tor IP, while giving 200 to someone who is, just for HEAD requests? I'm not so sure.

I'd argue to just not support it, or if you do chose to support it then you might need to look into some more useful status codes for the situation.

[arlolra]

@Ryman I decided to fix this another way. I think a response to a HEAD request is supposed to look the same as a
response to a GET request, just without a body, so I'd rather not disallow it.

[Ryman]

In these cases though, the response will always be 200 for HEAD requests then, so it doesn't really mean much (as the handler has been found, and we're not doing any content searching in the handlers one you pass the asset handling lines Phttp.Serve...). HEAD is after all more generally used for checking cache times or to see if the server is working. HEAD doesn't actually make any sense for this service as both those handlers are 'content found' so they are 200 always regardless of isTor or not?

I'd suggest just quick returning 200s for HEAD instead of special error handling (which will always error for HEAD requests, so may aswell just catch it before doing work)

Regarding the more general problem of what happens if we write to the responsewriter (which sets status 200) and then there's an error in the template. This probably shouldn't happen unless there's an error in the template which will be caught by our initial compiles, or if there's network issues and the connection is lost which won't matter as the client won't get a response then anyway. Perhaps your buffering idea into a bytebuffer or something would be good for that. I think the HEAD issue is quite unique in that regard with it's BodyNotAllowed error.

[arlolra]

@Ryman These are good points. What do you think of 8652915?

[Ryman]

Seems to address the issues, perhaps add a Last-Modified header which is based on Exits.UpdateTime (incase anyone is actually using the HEAD requests to check when they need to recheck their data)?

[arlolra]

Hmm, I like that for the output of the bulk exit list but the other content is dynamic based on IP so although the data hasn't modified, the response will. In fact, we should maybe add Cache-Control: no-cache.

[Ryman]

Yes, good call!

[Ryman]

Sorry, I jumped the gun on that, I agree on Cache-Control but not with just having a Last-Modified on bulk exit (if one is given). Remember that the HEAD won't get the answer of if it is Tor or not, so all Last-Modified will tell it is when the situation last changed, which will still be correct even if it's based on IP, the last time the exitlist was updated it the last time the answers changed.

[arlolra]

From http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html,

The HEAD method is identical to GET except that the server MUST NOT return a message-body in the response. The metainformation contained in the HTTP headers in response to a HEAD request SHOULD be identical to the information sent in response to a GET request. This method can be used for obtaining metainformation about the entity implied by the request without transferring the entity-body itself. This method is often used for testing hypertext links for validity, accessibility, and recent modification.

The response to a HEAD request MAY be cacheable in the sense that the information contained in the response MAY be used to update a previously cached entity from that resource. If the new field values indicate that the cached entity differs from the current entity (as would be indicated by a change in Content-Length, Content-MD5, ETag or Last-Modified), then the cache MUST treat the cache entry as stale.

I highlighted my concerns. I guess since the information should only be used to bust a cache which hopefully won't exist in the first place, it'll be alright.

Ok, let's go for it.