Traffic Router inconsistent behavior on HTTP HEAD requests

[guzzijason]

I'm submitting a ...

• bug report
• new feature / enhancement request
• improvement request (usability, performance, tech debt, etc.)
• other

Traffic Control components affected ...

• CDN in a Box
• Documentation
• Grove
• Traffic Control Client
• Traffic Monitor
• Traffic Ops
• Traffic Ops ORT
• Traffic Portal
• Traffic Router
• Traffic Stats
• Traffic Vault
• unknown

Current behavior:

The TR response to a HEAD request should resemble the same as a GET response. However, the behavior I'm seeing seems problematic. Following examples of each (from curl output) with timing info included:

Proper GET response:

> GET / HTTP/1.1
> Host: [REDACTED]
> User-Agent: curl/7.54.0
> Accept: */*
>
< HTTP/1.1 302 Found
< Location: [REDACTED]
< Content-Length: 0
< Date: Tue, 08 Oct 2019 14:58:45 GMT
<
* Connection #0 to host [REDACTED] left intact

real	0m0.705s
user	0m0.008s
sys	0m0.015s

Improper HEAD response:

> HEAD / HTTP/1.1
> Host: [REDACTED]
> User-Agent: curl/7.54.0
> Accept: */*
>
< HTTP/1.1 302 Found
< Location: [REDACTED]
< Transfer-Encoding: chunked
< Date: Tue, 08 Oct 2019 15:23:12 GMT
<
* transfer closed with outstanding read data remaining
* stopped the pause stream!
* Closing connection 0

real	0m10.855s
user	0m0.007s
sys	0m0.017s

Note that the HEAD response is including Transfer-Encoding: chunked, which is causing the client to wait for body data which never arrives. After ~10 seconds, Traffic Router times out the connection and resets the TCP connection (verified TR sends FIN at this point with a packet capture).

Expected / new behavior:

TR should be responding to a HEAD request with Content-Length: 0, and NOT Transfer-Encoding: chunked.

Minimal reproduction of the problem with instructions:

Traffic Router version: 3.0.0-9930.f225e63b.el7

Problem can easily be replicated by sending a HEAD request with curl:

time curl -vs -o /dev/null -X HEAD http://ccr.invalid.cdn.example.com/

Anything else:

[ocket8888]

Alternatively, chunked responses with zero chunks would be fine, I think. Not sure why we'd want to do that, but it just seems like something on the TR end is expecting to write data but doesn't get it.

[zrhoffman]

The wait itself is wontfix cURL-specific behavior. From a curl-users mailing list thread:

The difference in the HTTP protocol between a HEAD and a GET response is that a HEAD response is exactly the same as the GET response except that there's no response body. Since curl knows it sent a GET request, it will wait for a response body if the response headers indicate there is one. If curl knows it sent a HEAD it will just not wait for a response, even if the headers say there is one, since it knows a server will never send a response body to such a request.

That said, setting a content length does get rid of the timeout. As a proof-of-concept, I have added the Content-Length header for steering responses on a branch: a5a4bb9...zrhoffman:set_content_length

Would we also want to ensure a content length for the TR API?

[guzzijason]

Per RFC 7231:

The server SHOULD send the same
 header fields in response to a HEAD request as it would have sent if
 the request had been a GET, except that the payload header fields
(Section 3.3) MAY be omitted.

TR sends Content-Length: 0 as part of a HTTP GET response, so based on the RFC recommended behavior, then I believe it should do the same in response to an HTTP HEAD.

[guzzijason]

Sorry... actually, Content-Length is one of the payload headers that MAY be omitted in a HEAD response. At any rate, keeping that header as part of the HEAD response is still RFC-compliant behavior.

I think the bigger problem is the Transfer-Encoding: chunked header, which is NOT part of the standard HTTP GET request from TR. Its unclear why we would want TR to send that at in any case of a 302 response.

[zrhoffman]

Most of the API's mappings return a ResponseEntity , which guarantees setting Content-Length for any status code. The exceptions are in LocationController, StatsController, ZonesController (which return Content-Length for HEAD but not GET), but it looks like we can just use ResponseEntity for those, too. The object order in the response seems different, but the structure is the same and LocationsTest, and StatsTest, ZonesTest still pass.

The documentation will need to be updated to reflect the change in response headers.

[smalenfant]

I haven't verified hits PR, but what would be the Content-Length on a Steering delivery services with ?trred=false ?

[smalenfant]

This seems to be a big deal.. More than half our requests are HEAD requests... That should change the TCP stats for our Traffic Routers considerably.

[rob05c]

I don't object to the change itself, but I have concerns about what's driving it.

The HTTP Spec says servers MUST NOT send Content-Length if they use Chunked encoding, and clients MUST accept Chunked encoding.

Therefore, any Clients which require Content-Length are in violation of HTTP.

actually, Content-Length is one of the payload headers that MAY be omitted in a HEAD response.

Right. So, the server isn't in violation of the spec here, it's permitted to include Tranfer-Encoding. But I agree, it probably shouldn't.

But moreover, Transfer-Encoding is a payload field, and HEAD payloads have no defined semantics. So, if it's confusing any clients, those clients are violating the HTTP spec.

So again, it doesn't seem wrong to change the server (maybe even following the Robustness Principle). But we should fix the broken clients driving this.

[guzzijason]

But why do we bother to send Transfer-Encoding: chunked on a HEAD response with no body? Even if the spec says we can, should we be? Seems to serve no purpose. I would have expected the 302 response headers for both GET and HEAD to be basically identical. Is there a good reason not to have them be?

[ocket8888]

Is GET not chunked?

[guzzijason]

Nope. See my examples of the current GET/HEAD responses in the issue description.
GET responds with

Content-Length: 0

while HEAD responds with:

Transfer-Encoding: chunked

IMHO, they should both just be sending Content-Length.