Skip to content

avformat/http: stale range_end is never reset between requests on a reused HTTPContext, causing spurious EAGAIN re-requests and keep-alive protocol desync #41

Description

@ronag

Summary

HTTPContext.range_end is per-connection state that is set from each 206 Partial Content response's Content-Range "to" value (parse_content_range), but it is never cleared when the context is reused for a new request: neither ff_http_do_new_request2's state-reset block nor http_connect's per-request init block (nor http_read_header) zeroes it. Since commit bf1722a9c6 ("avformat/http: request more data after partial response"), http_buf_read compares the current offset against range_end to decide when to issue a follow-up range request. A stale range_end left over from a previous response can make this comparison fire in the middle of a later response's body, causing a brand-new HTTP request to be written onto the keep-alive socket while unread body bytes are still pending — the next http_read_header then parses leftover body bytes as HTTP headers (protocol desync), typically ending in an "Unexpected offset" AVERROR(EIO) and loss of the in-flight data. The stale value also corrupts the soft-seek drain arithmetic in http_seek_internal.

Location

  • Declaration (per-connection state):

    FFmpeg/libavformat/http.c

    Lines 141 to 152 in 9a83bff

    /************************
    * Per-connection state *
    ************************/
    URLContext *hd;
    char *uri;
    char *new_location;
    int http_code;
    int64_t expires;
    /* Used if "Transfer-Encoding: chunked" otherwise -1. */
    uint64_t chunksize;
    int chunkend;
    uint64_t range_end;
    (HTTPContext)
  • Only write site:
    s->range_end = strtoull(end + 1, NULL, 10) + 1;
    (parse_content_range)
  • Missing reset, request reuse:

    FFmpeg/libavformat/http.c

    Lines 597 to 600 in 9a83bff

    s->end_chunked_post = 0;
    s->chunkend = 0;
    s->off = 0;
    s->icy_data_read = 0;
    (ff_http_do_new_request2)
  • Missing reset, per-request init:

    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
    (http_connect)
  • Mis-compare consuming the stale value:

    FFmpeg/libavformat/http.c

    Lines 1824 to 1829 in 9a83bff

    uint64_t file_end = s->end_off ? s->end_off : s->filesize;
    uint64_t target_end = s->range_end ? s->range_end : file_end;
    if ((!s->willclose || s->chunksize == UINT64_MAX) && s->off >= file_end)
    return AVERROR_EOF;
    if (s->off == target_end && target_end < file_end)
    return AVERROR(EAGAIN); /* reached end of content range */
    (http_buf_read)
  • Second consumer (soft-seek drain):

    FFmpeg/libavformat/http.c

    Lines 2205 to 2206 in 9a83bff

    uint64_t remaining = s->range_end - old_off - old_buf_size;
    if (s->hd && !s->willclose && s->range_end && remaining <= ffurl_get_short_seek(h)) {
    (http_seek_internal)

Details

range_end is written exactly once, in parse_content_range (http.c:961):

/* "bytes $from-$to/$document_size" */
static void parse_content_range(URLContext *h, const char *p)
{
    ...
    if (!strncmp(p, "bytes ", 6)) {
        p     += 6;
        s->off = strtoull(p, NULL, 10);
        if ((end = strchr(p, '-')) && strlen(end) > 0)
            s->range_end = strtoull(end + 1, NULL, 10) + 1;
        ...

No code path zeroes it after the response it belongs to is finished. Compare the reset blocks that handle the companion per-request fields:

/* ff_http_do_new_request2, http.c:597-600 — range_end missing */
    s->end_chunked_post = 0;
    s->chunkend      = 0;
    s->off           = 0;
    s->icy_data_read = 0;

/* http_connect, http.c:1719-1731 — range_end missing */
    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;

(chunksize is similarly reset in http_read_header at http.c:1483; range_end was simply missed.)

The stale value is consumed in http_buf_read (http.c:1824-1829):

        uint64_t file_end   = s->end_off   ? s->end_off   : s->filesize;
        uint64_t target_end = s->range_end ? s->range_end : file_end;
        if ((!s->willclose || s->chunksize == UINT64_MAX) && s->off >= file_end)
            return AVERROR_EOF;
        if (s->off == target_end && target_end < file_end)
            return AVERROR(EAGAIN); /* reached end of content range */

Trace (keep-alive segment reuse, e.g. HLS byterange segment followed by a plain segment)

  1. Request A is sent with Range: bytes=100-1099 (off=100, end_off=1100). The server answers 206 with Content-Range: bytes 100-1099/50000; parse_content_range sets range_end = 1100. Segment A ends cleanly via the s->off >= file_end check (file_end = end_off = 1100, line 1826).
  2. ff_http_do_new_request2() (http.c:556-619) is called for the next URL on the same keep-alive connection. It resets off, chunkend, icy_data_read (lines 597-600) but not range_end; the new request has no offset/end_offset set.
  3. Response B is 200 OK with Content-Length: 5000 and no Content-Range header, so range_end stays 1100 from response A.
  4. While reading response B's body, http_buf_read reaches off == 1100: file_end = filesize = 5000, target_end = range_end = 1100 (lines 1824-1825). The condition s->off == target_end && target_end < file_end is true, so it returns AVERROR(EAGAIN) (lines 1828-1829) — even though 3900 body bytes of response B are still unread on the socket.
  5. The EAGAIN handler in http_read_stream (http.c:1916-1926) treats this as "end of content range, request more": willclose == 0, so s->hd is kept open and http_open_cnx is called. In http_open_cnx_internal the if (!s->hd) check (http.c:338) skips reconnecting, so http_connect writes a new Range: bytes=1100- request onto the same socket.
  6. http_read_header (called at http.c:1743) then consumes segment B's leftover body bytes as HTTP header lines. Status-line parsing of binary data yields a garbage http_code (via strtol of noise); with no Content-Range, s->off stays 0, and http_connect fails the offset check at http.c:1755-1761 ("Unexpected offset: expected 1100, got 0") with AVERROR(EIO) mid-stream. Either way the 3900 in-flight media bytes are destroyed and the connection state is desynced.

The stale value also feeds the soft-seek drain arithmetic in http_seek_internal (http.c:2205-2206):

    uint64_t remaining = s->range_end - old_off - old_buf_size;
    if (s->hd && !s->willclose && s->range_end && remaining <= ffurl_get_short_seek(h)) {

With a stale range_end, remaining is computed against the wrong response, so a seek may "soft-seek" by draining the wrong number of bytes from the wire (or underflow remaining since the operands are unsigned), again desyncing the keep-alive connection.

Impact

Severity: medium.

  • Mid-stream read failures (AVERROR(EIO), "Unexpected offset") and loss of in-flight body data whenever an HTTPContext that previously received a 206 response is reused (via ff_http_do_new_request2, used by the HLS/DASH demuxers for keep-alive segment reuse, or via seek/reconnect paths) for a response that does not itself carry a matching Content-Range.
  • Particularly likely with HLS playlists that mix #EXT-X-BYTERANGE segments with plain segments, or any flow alternating ranged and unranged requests on one connection.
  • Incorrect soft-seek drain amounts in http_seek_internal, which can stall or desync the connection on small forward seeks.
  • No memory-safety issue identified; the corrupt "headers" are parsed by the normal bounded line parser. The failure mode is data loss/stream abort rather than exploitable corruption.

Suggested fix

Zero range_end wherever per-request state is reinitialized.

--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -596,6 +596,7 @@ int ff_http_do_new_request2(URLContext *h, const char *uri, AVDictionary **opts
     s->end_chunked_post = 0;
     s->chunkend      = 0;
     s->off           = 0;
+    s->range_end     = 0;
     s->icy_data_read = 0;
 
     av_free(s->location);
@@ -1722,6 +1723,7 @@ static int http_connect(URLContext *h, const char *path, const char *local_path
     s->off              = 0;
     s->icy_data_read    = 0;
     s->filesize         = UINT64_MAX;
+    s->range_end        = 0;
     s->willclose        = 0;
     s->end_chunked_post = 0;
     s->end_header       = 0;

The reset in http_connect covers the seek/reconnect path; the one in ff_http_do_new_request2 makes the intent explicit at the reuse boundary (and matters because http_seek_internal reads range_end at http.c:2205 before http_connect runs, so the value must not survive from a previous request).

Note: http_seek_internal's soft-seek path (http.c:2205-2206) intentionally consumes the previous request's range_end before opening the new request, so the reset must happen after that read — both proposed locations satisfy this.

Upstream status

This bug also exists in upstream FFmpeg (upstream/master, https://code.ffmpeg.org/FFmpeg/FFmpeg.git): range_end is set in parse_content_range, read in http_buf_read and http_seek_internal, and is likewise absent from both reset blocks (ff_http_do_new_request2 and http_connect's init block). Commit bf1722a9c6 ("avformat/http: request more data after partial response"), which introduced the EAGAIN-based follow-up-request machinery, is upstream as well. Worth reporting/fixing upstream too.


Consolidates duplicate findings from the review: avformat/http: stale range_end across requests causes spurious EAGAIN and a pipelined request mid-body (HTTP framing desync).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinghttplibavformat/http.cupstreamAlso present in upstream FFmpeg

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions