routing-release icon indicating copy to clipboard operation
routing-release copied to clipboard

Gorouter: go1.22 content-length header check metric too aggressive

Open peanball opened this issue 9 months ago • 0 comments

Current behavior

The missing_content_length_header metric is too aggressive. It is intended to catch the new, more strict behavior of Go 1.22, where empty Content-Length headers will be rejected.

The current code is also counting requests where the Content-Length header is not there. GET requests usually don't have a body or Content-Length, as do many other request types.

Go checks, whether a set Content-Length header is an empty string, but is OK with the absence of the header. See https://github.com/golang/go/blob/33496c2dd310aad1d56bae9febcbd2f02b4985cb/src/net/http/transfer.go#L1051

The metric was introduced with https://github.com/cloudfoundry/gorouter/pull/412.

Desired behavior

The metric should count the number of empty Content-Length headers, not of missing headers.

We will provide a fix PR that replicates the logic from Go, i.e. accepts the absence of the Content-Length header.

Affected Version

0.297.0

peanball avatar May 22 '24 14:05 peanball