Open avodonosov opened 1 year ago
Currently I think the best approach would be to extend chunga with a possibility for content length limit of unchunked input, continuing the spirit of the "repeatable EOF after last chunk" feature added in https://github.com/edicl/chunga/commit/e7dc5365db9ebdd9368a87dcf66c0df7e35ba79a.
For compatibility reasons, both new features of the chunga stream will only be enabled if user calls http-request
with new keyword arg :want-body-stream
- the same meaning as :want-stream
except it is guaranteed to return (repeatable) EOF after the response body is fully read.
Correction: when deciding between chunking and content-length, the user code should first check for Transfer-Encoding, and only if it is absent look for Content-Length.
Early implementations of Transfer-Encoding would occasionally send both a chunked transfer coding for message framing and an estimated Content-Length header field for use by progress bars. This is why Transfer-Encoding is defined as overriding Content-Length, as opposed to them being mutually incompatible.
https://www.rfc-editor.org/rfc/rfc9112#section-6.1
If neither Transfer-Encoding nor Content-Length is present, the response message body is termitated by the server closing the connection (highly not recommended for servers to work that way). See the item 8 at https://www.rfc-editor.org/rfc/rfc9112#section-6.3.
Drakma docs do not instruct how to correctly read response data from a stream returned with
:want-stream t
parameter.And guidance is needed, because it's very non-trivial to do correctly. Users are confused.
Simply reading from the stream till EOF is inadequate, because:
:keep-alive t
so that the connection is kept open.So, we should instruct the users to only read as many bytes as HTTP protocol guides through Content-Length or chunking, and not rely on connection termination.
A possible instruction will be to look if Content-Length response header is present and read this number of bytes. Otherwise assume chunked encoding (or explicitly test for "chunked" present in Transfer-Encoding response header), and rely on drakma wrapping response stream into a chunga stream, which can be read till EOF, as this stream reports EOF when chunked body finishes, without relying on TCP connection closure. Users should stop at the first EOF. (Because chunga streams normally report EOF only once, after which they switch
chunked-stream-input-chunking-p
tonil
and continue passing read calls to the underlying stream. So attempts to read from the stream after the first EOF will result in attempt to read more from the network. This may result in OpenSSL layer of the underlying stream signalling the "Unexpected EOF" in case of "Connection: close" without close_notify or in endless waiting in case of "Connection: keep-alive".)That's already somewhat complex. But there is more trouble.
If user does
and the server sends a gzipped response, drakma will wrap the response stream into
chipz::decompressing-stream
which can ungzip on the fly.In case server uses Content-Length, the chipz::decompressing-stream knows nothing about it, so user reading till EOF will only succeed in case of "Connection: close" mode in plain HTTP or HTTPS of a server who sends close_notify before closing the connection. Otherwise, client will get the Unexpected EOF error from OpenSSL for servers closing the connection without close_notify, or will hang forever for connection "Connection: keep-alive".
In case the server uses Transfer-Encoding: ...chunked..., the chipz::decompressing-stream could notice end of the body because it wraps a chunga stream with chunking enabled. But the chipz library has a bug where it thinks EOF will be repeatedly reported by the underlying stream. Which is not true for chunga streams and may result in errors, as explained above.
All this complexity is represented in the code of the
read-body
internal drakma function: https://github.com/edicl/drakma/blob/f128fe4ca8ed56c8a6d2ecac934d2f7b22926973/request.lisp#L152For example, for a gzipped response and Content-Length, this number of bytes is first read into an array, and then decompressed. For a chunked gzipped response, @stassas has implemented a special mode for chunga, that repeatedly reports EOF to the calling code. The read-body uses this mode to work-around the chipz library bug (https://github.com/edicl/drakma/commit/88b05e0c54ee65e45bcb687f67aede42918c5a6b, https://github.com/edicl/chunga/commit/e7dc5365db9ebdd9368a87dcf66c0df7e35ba79a)
One solution idea is to implement a kind of length limited stream, which would know Content-Length if specified, or remember EOF signaled by chunga. If such a stream is returned to client both when compression is present or absent, the client could safely read till EOF and not bother looking into headers.
But drakma docs explicitly specify that the stream returned is a flexi-stream that wraps a chunga-stream. Changing the stream structure would be an API change. So it seems, such a solution is only possible if length limiting is added to flexi-stream or to chunga, but not into some separate stream wrapper class.
Or, a separate helper function / macro like
(drakma:length-limit (drakma:http-request ... :want-stream t))
could inject a special length limiting stream into the stream delegation chain, without breaking the http-request API.Wrapping structure for the first return value of
(drakma:http-request ... :want-stream t)
: