http3: avoid re-parsing of the Content-Length header for responses
There's a slight change in behavior here: For requests that don't have a body, we now still set the http.Response.ContentLength to -1 (once we have #4645). I think this ok:
// ContentLength records the length of the associated content. The
// value -1 indicates that the length is unknown. Unless Request.Method
// is "HEAD", values >= 0 indicate that the given number of bytes may
// be read from Body.
ContentLength int
@bt90 @WeidiDeng What do you think?
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 84.94%. Comparing base (
b92bf0c) to head (cb82af2). Report is 1 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #4648 +/- ##
=======================================
Coverage 84.94% 84.94%
=======================================
Files 150 150
Lines 14930 14927 -3
=======================================
- Hits 12681 12679 -2
+ Misses 1722 1721 -1
Partials 527 527
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Talking about the principle of least surprise: if there's no body, I'd be surprised if the length was marked as unknown. A missing body has a known length of 0 bytes. Or is there a reason why we need to distinguish between no body and an empty body?
It just means we can't remove the lines I'm removing here in this PR. I'll update it.
It's in fd04f71. It's kind of ugly.
I haven't looked into it, but RFC7230 has largely been superseded by RFC9110.
https://datatracker.ietf.org/doc/html/rfc9110#name-content-length
It's in fd04f71. It's kind of ugly.
Ah, i see. So there are three cases:
- no
Content-Lengthheader: either the size is unknown or it's not allowed for this kind of response -
Content-Length: 0for an empty body -
Content-Length: xif a body of known length is present
I'd agree that the -1 value is probably the cleanest solution.
It's in fd04f71. It's kind of ugly.
This commit was slightly wrong, I messed up inverting the boolean expression. It's still not pretty, but it's not worse than what we had before, so I'm inclined to just merge it as is.