Do not write manifests on HEAD requests

[jaimem88]

The GetManifest handler is used for both HEAD and GET requests. For HEAD requests, the handler also calls w.Write() on the response writer.

This is not needed as HEAD requests should not include a response body. Most clients will actually just ignore the body, for example curl does this and simply skips it.

This PR adds a check to the GetManifest handler to return early when the request method is a HEAD. This would prevent wasting resources writing to the response writer.

The ServeBlob method of the blobServer uses the standard library http.ServeContent which internally has a check to only copy the result if the request method is not HEAD. So this change will follow the same approach as the blobs handler.

The change has also been recently merged to GitLab's Container Registry via https://gitlab.com/gitlab-org/container-registry/-/merge_requests/1585.

[jaimem88]

@joaodrp can you please have a look here as well?

[wy65701436]

according the distribution spec outlined in the heading and manifest, it makes sense to me.

A HEAD request to an existing blob or manifest URL MUST return 200 OK. A successful response SHOULD contain the digest of the uploaded blob in the header Docker-Content-Digest. A successful response SHOULD contain the size in bytes of the uploaded blob in the header Content-Length.

If the blob or manifest is not found in the registry, the response code MUST be 404 Not Found.

[wy65701436]

@jaimem88 please help to fix the CI failure.

[joaodrp]

LGTM, thanks!

[Jamstah]

Thanks, LGTM.

[thaJeztah]

I recalled there was some discussion about HEAD requests, but that was not about body. And looking back at my comments there, the http RFC even marks it as "MUST NOT send a body"; #3691 (comment)

[thaJeztah]

changes LGTM, but could you squash the commits? Looks like the second one is fixing an issue that was introduced in the first commit, so we don't need to preserve that fix up in history

[wy65701436]

lgtm