head method in health endpoint

[moinologics]

Fixes #687

[eternal-flame-AD]

Thanks for the PR. It is weird some company would do a HEAD preflight for a health check service, but good detail to include.

[eternal-flame-AD]

This is not compliant with HTTP specs. Please refer to the MDN docs and not return a body. Thanks!

https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/HEAD

[jmattheis]

@eternal-flame-AD I would merge this as is. The RFC states:

The server SHOULD send the same header fields in response to a HEAD request as it would have sent if the request method had been GET. However, a server MAY omit header fields for which a value is determined only while generating the content.

I'd see it as recommendation to include content-length, but you can/MAY optionally omit it in some circumstances. So the current implementation complies with the recommendation.

I tested this with socat, seems like gin internally strips the body if the verb was HEAD

I think it's actually the native go http server, and it makes sense, as you need the body to get the content-length and given HEAD requests must not have a body I'd see this as working as intended and not as error-catching feature.

[eternal-flame-AD]

It's not clearly stated that how acceptable possible inconsistencies are but I agree it seems to imply it's not super important, and in the remote possibility they are doing it in C we still have an always correct length in the actual GET.
Hyper/axum seem to do it too it seems. I just think the semantics looks like GET and HEAD would produce similar results because you just matched string-wise on the method and I'm not sure it's well-accepted for an http library to internally handle this for you.

[eternal-flame-AD]

@moinologics I tested this with socat, seems like gin internally strips the body if the verb was HEAD, so this code produces a compliant behavior (except dynamically generated JSON should not have a content-length on HEAD). However I still think this is not ideal, we shouldn't rely on undocumented error-catching features and write code with a looks-wrong semantics.

Feel free to @ jmattheis for a second opinion :) Everybody has different styles and preferences

[eternal-flame-AD]

Here's a solution I think is robust, (maybe even a little overkill?) , but at the very least I think connection, content type and content-encoding must be the same, and content length must not be present.

b43e7cd

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.46%. Comparing base (a58b0be) to head (2e0210d).
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #688   +/-   ##
=======================================
  Coverage   80.46%   80.46%           
=======================================
  Files          56       56           
  Lines        2191     2191           
=======================================
  Hits         1763     1763           
  Misses        337      337           
  Partials       91       91           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[eternal-flame-AD]

Thanks.

Cc @moinologics

[eternal-flame-AD]

It's not clearly stated that how acceptable possible inconsistencies are but I agree it seems to imply it's not super important, and in the remote possibility they are doing it in C we still have an always correct length in the actual GET.
Hyper/axum seem to do it too it seems. I just think the semantics looks like GET and HEAD would produce similar results because you just matched string-wise on the method and I'm not sure it's well-accepted for an http library to internally handle this for you.