V1F: Read end-of-chunk as part of the chunk
Taken from #3809 as requested by @bsdphk
Until now, we read the (CR)?LF at the end of a chunk as part of the next chunk header (see: /* Skip leading whitespace */).
For a follow up commit, we are going to want to know if the next chunk header is available for read, so we now consume the chunk end as part of the chunk itself.
This also fixes a corner case: We previously accepted chunks with a missing end-of-chunk (see fix of r01729.vtc).
Ref: https://datatracker.ietf.org/doc/html/rfc7230#section-4.1
I'm pretty sure we implemented it the current way to cater to some bogus implementation of chunked, but that would be more than 14 years ago, so one would hope that it no longer applies.
phk OK via bugwash
martin said -1
@mbgrydeland raised the concern that waiting for the chunk end [CR]LF could increase latency, in particular if that final byte was sent in another packet.
So maybe a middle ground could be to have a flag set by VFPs downstream if they need an accurate VFP_END? This way, the request body caching code could still use it, leaving the existing code unaffected.
@mbgrydeland thank you for your constructive suggestion, something like this seems sensible to require the [CR]LF at the end of the chunk for the case where VFP_END is not required with the last bytes.
As laid out in https://github.com/varnishcache/varnish-cache/pull/3811#issuecomment-1215050989, I think we do need a way to get VFP_END signaled with the last bytes for partial request body caching, see #3809
@mbgrydeland what do you think about this?
- Fix the existing
v1f_chunkedalong the lines of what you suggested - Add a
v1f_chunked_strictvariant which implements the strictVFP_ENDsignaling