Do not serve response body for HEAD requests

[jbagga]

Addresses #7208

[Tratcher]

Do any of these tests check that content-length is set? I'd expect the same header for GET and HEAD

[jbagga]

No these don't. There are unit tests that check that for GET. I can add Content-Length to these tests for HEAD

[jbagga]

@Tratcher #6875 We only set Content-Length when serving the body. Should HEAD be special cased and set Content-Length?

[jbagga]

Actually we can set it for all requests that get to that line since we short-circuit return for 304 and 412 before we even get to setting Content-Length. I will remove the check if(serveBody) before we set Content-Length here https://github.com/aspnet/Mvc/blame/dev/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/FileResultExecutorBase.cs#L89

[jbagga]

@Tratcher Updated

[dustinmoris]

In order to avoid any mistakes for HEAD requests I think the safest would be to remove all HEAD checks from the code and place only a single if statement at the very end of the execution just above the line where you'd start writing to the body of the response:

if !(isHeadRequest)
{
    StreamCopyOperation.CopyToAsync(..)
    //..
}

In this case you make sure that the HEAD request will be exactly equivalent to a GET request except it won't ever include a response body.

In other words I personally think the variable serveBody can be made entirely redundant, because in an error response you'd have short circuited anyway and in all other cases you want to reach the point of StreamCopyOperation.CopyToAsync(..) where you'd have a single if statement for HEAD.

This would hugely simplify the code which would make it more easy to understand and avoid future errors.

EDIT: Sorry forget what I said, because the method also checks for all the caching preconditions. It would have been easier to have the precondition logic extracted and done in a separate step before processing the rest, but that would be too big of a change now I guess.