avformat/http: stale chunkend from previous response causes spurious EOF/EIO on new request over persistent connection

[ronag]

Summary

HTTPContext.chunkend is per-connection state (declared in the "Per-connection state" section, libavformat/http.c:151) but is only ever cleared in ff_http_do_new_request2() (line 598). The per-request init block in http_connect() (lines 1719-1731) resets buf_ptr, off, filesize, willclose, end_chunked_post, end_header — but not chunkend. After a chunked response has been fully consumed via the persistent-connection path (lines 1797-1801 set chunkend = 1), any subsequent request issued through http_seek_internal(), the reconnect logic, or the EAGAIN request renewal in http_read_stream() inherits chunkend == 1. If the new response is also chunked, the very first http_buf_read() returns AVERROR_EOF without reading a single body byte, which http_read_stream() then converts into AVERROR(EIO) or a pointless reconnect loop. Because this fork changed the multiple_requests default to -1 (truthy at line 1797), the chunkend = 1 path is taken by default for every fully-read chunked response, whereas upstream's default of 0 closes the connection instead — so the fork hits this far more often.

Location

• Missing reset in http_connect() init block:

  FFmpeg/libavformat/http.c

  Lines 1719 to 1731 in 9a83bff

  	/* init input buffer */
  	s->buf_ptr = s->buffer;
  	s->buf_end = s->buffer;
  	s->line_count = 0;
  	s->off = 0;
  	s->icy_data_read = 0;
  	s->filesize = UINT64_MAX;
  	s->willclose = 0;
  	s->end_chunked_post = 0;
  	s->end_header = 0;
  	#if CONFIG_ZLIB
  	s->compressed = 0;
  	#endif

• Stale-flag check and set in http_buf_read():

  FFmpeg/libavformat/http.c

  Lines 1778 to 1801 in 9a83bff

  	if (s->chunksize != UINT64_MAX) {
  	if (s->chunkend) {
  	return AVERROR_EOF;
  	}
  	if (!s->chunksize) {
  	char line[32];
  	int err;
  	do {
  	if ((err = http_get_line(s, line, sizeof(line))) < 0)
  	return err;
  	} while (!*line); /* skip CR LF from last chunk */
  	s->chunksize = strtoull(line, NULL, 16);
  	av_log(h, AV_LOG_TRACE,
  	"Chunked encoding data size: %"PRIu64"\n",
  	s->chunksize);
  	if (!s->chunksize && s->multiple_requests) {
  	http_get_line(s, line, sizeof(line)); // read empty chunk
  	s->chunkend = 1;
  	return 0;
  	}

• Only existing reset, in ff_http_do_new_request2():

  FFmpeg/libavformat/http.c

  Line 598 in 9a83bff

  s->chunkend = 0;

Details

The init block in http_connect() resets every other piece of per-request state, but not chunkend:

    /* init input buffer */
    s->buf_ptr          = s->buffer;
    s->buf_end          = s->buffer;
    s->line_count       = 0;
    s->off              = 0;
    s->icy_data_read    = 0;
    s->filesize         = UINT64_MAX;
    s->willclose        = 0;
    s->end_chunked_post = 0;
    s->end_header       = 0;
#if CONFIG_ZLIB
    s->compressed       = 0;
#endif

And in http_buf_read() (lines 1778-1801):

    if (s->chunksize != UINT64_MAX) {
        if (s->chunkend) {
            return AVERROR_EOF;
        }
        if (!s->chunksize) {
            ...
            s->chunksize = strtoull(line, NULL, 16);
            ...
            if (!s->chunksize && s->multiple_requests) {
                http_get_line(s, line, sizeof(line)); // read empty chunk
                s->chunkend = 1;
                return 0;
            }
            ...

Step-by-step trace (seek/rewind scenario):

1. A chunked but seekable response is opened (server sends Accept-Ranges: bytes, so h->is_streamed = 0, lines 1317-1320; Transfer-Encoding: chunked sets s->chunksize = 0, lines 1321-1324) and is read to completion. On the terminating zero-size chunk, line 1797 if (!s->chunksize && s->multiple_requests) is true — with multiple_requests = 1, or with this fork's default of -1 (option declared at line 199 with { .i64 = -1 }) — so line 1799 sets s->chunkend = 1 and returns 0 (clean EOF, connection left open).
2. The caller rewinds: avio_seek(0) → http_seek_internal() → http_open_cnx() → http_connect(). The init block (lines 1719-1731) contains no s->chunkend = 0. http_read_header() resets chunksize to UINT64_MAX (line 1483), then the new response's Transfer-Encoding: chunked header sets chunksize = 0 again (lines 1321-1324). chunkend is still 1.
3. First read of the new body: in http_buf_read(), s->chunksize != UINT64_MAX is true (line 1778), so the stale check at lines 1779-1781 returns AVERROR_EOF — zero bytes delivered from a fresh, valid response.
4. In http_read_stream() (lines 1908-1937): read_ret = AVERROR_EOF, is_premature = (s->filesize > 0 && s->off < s->filesize) is true since filesize is UINT64_MAX for a chunked response, is_streamed == 0, and with reconnect off the function returns AVERROR(EIO) at line 1934. With reconnect on, it instead enters a futile reconnect loop (http_seek_internal(..., force_reconnect=1) goes through http_connect() again, which still does not clear the flag).

The same stale flag is inherited by the AVERROR(EAGAIN) request-renewal path in http_read_stream() (lines 1916-1926, fork-specific initial_requests feature), which also re-enters http_open_cnx()/http_connect() without clearing chunkend.

grep -n chunkend libavformat/http.c confirms the only assignments are at line 598 (ff_http_do_new_request2()) and line 1799; the HLS/DASH code path through ff_http_do_new_request2() is the only one that clears the flag.

Impact

After fully reading one chunked response on a kept-alive connection, the next request on the same URLContext (seek/rewind, reconnect, or the fork's EAGAIN request renewal) that yields another chunked response is unreadable: the first body read reports EOF having delivered zero bytes, surfacing to callers as AVERROR(EIO) (premature EOF) or as a reconnect loop that never makes progress. For example, rewinding a chunked HTTP resource makes the whole resource unreadable. Severity: medium — no memory unsafety, but silent data-path failure on the default configuration of this fork.

Upstream status: the missing reset exists verbatim in upstream FFmpeg (git show upstream/master:libavformat/http.c: field at line 80, sole resets at 569/1735, same init block without chunkend), so this is worth reporting upstream too. However, upstream defaults multiple_requests to 0 (line 176), so the chunkend = 1 path at line 1735 is only reachable when a user explicitly enables persistent connections; this fork's multiple_requests default of -1 (line 199) makes the broken path the default behavior, i.e. a fork-amplified regression relative to upstream defaults.

Suggested fix

Clear chunkend in http_connect()'s per-request init block alongside the other per-request state:

--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ static int http_connect(URLContext *h, const char *path, const char *local_path,
     /* init input buffer */
     s->buf_ptr          = s->buffer;
     s->buf_end          = s->buffer;
     s->line_count       = 0;
     s->off              = 0;
+    s->chunkend         = 0;
     s->icy_data_read    = 0;
     s->filesize         = UINT64_MAX;
     s->willclose        = 0;
     s->end_chunked_post = 0;
     s->end_header       = 0;

The existing reset at line 598 in ff_http_do_new_request2() can stay; it becomes redundant but harmless. The same one-line fix applies to upstream.