avformat/http: stale icy_metaint persists across requests and corrupts non-ICY responses

[ronag]

Summary

HTTPContext.icy_metaint is set from the Icy-MetaInt response header but is never reset when a new request is issued on the same context. Since Icy-MetaData: 1 is sent on every request by default (the icy option defaults to 1), once any response on a HTTPContext carries Icy-MetaInt, every subsequent response on that context — keep-alive reuse via ff_http_do_new_request2() (e.g. HLS segment fetches), redirects, or mid-stream reconnects that land on a server/edge which does not emit Icy-MetaInt — is still pushed through the ICY de-framing path. http_read() then silently strips 1 + len*16 bytes of real payload every icy_metaint bytes and exports the removed bytes as icy_metadata_packet: corrupted data is returned to the caller with no error.

Location

• http_connect() per-request init block (does not reset icy_metaint):

  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

• ff_http_do_new_request2() (resets only icy_data_read):

  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;

• process_line() setting icy_metaint from the header:

  FFmpeg/libavformat/http.c

  Lines 1346 to 1347 in 9a83bff

  	} else if (!av_strcasecmp(tag, "Icy-MetaInt")) {
  	s->icy_metaint = strtoull(p, NULL, 10);

• store_icy() / http_read() applying ICY de-framing whenever icy_metaint > 0:

  FFmpeg/libavformat/http.c

  Lines 2006 to 2058 in 9a83bff

  	static int store_icy(URLContext *h, int size)
  	{
  	HTTPContext *s = h->priv_data;
  	/* until next metadata packet */
  	uint64_t remaining;
  	if (s->icy_metaint < s->icy_data_read)
  	return AVERROR_INVALIDDATA;
  	remaining = s->icy_metaint - s->icy_data_read;
  	if (!remaining) {
  	/* The metadata packet is variable sized. It has a 1 byte header
  	* which sets the length of the packet (divided by 16). If it's 0,
  	* the metadata doesn't change. After the packet, icy_metaint bytes
  	* of normal data follows. */
  	uint8_t ch;
  	int len = http_read_stream_all(h, &ch, 1);
  	if (len < 0)
  	return len;
  	if (ch > 0) {
  	char data[255 * 16 + 1];
  	int ret;
  	len = ch * 16;
  	ret = http_read_stream_all(h, data, len);
  	if (ret < 0)
  	return ret;
  	data[len] = 0;
  	if ((ret = av_opt_set(s, "icy_metadata_packet", data, 0)) < 0)
  	return ret;
  	update_metadata(h, data);
  	}
  	s->icy_data_read = 0;
  	remaining = s->icy_metaint;
  	}
  	return FFMIN(size, remaining);
  	}
  	static int http_read(URLContext *h, uint8_t *buf, int size)
  	{
  	HTTPContext *s = h->priv_data;
  	if (s->icy_metaint > 0) {
  	size = store_icy(h, size);
  	if (size < 0)
  	return size;
  	}
  	size = http_read_stream(h, buf, size);
  	if (size > 0)
  	s->icy_data_read += size;
  	return size;
  	}

Details

icy_metaint lives in the struct's "Per-connection state" section (libavformat/http.c:141-163), but it is really per-response state: it is only valid for the response whose headers set it.

It is populated in process_line():

} else if (!av_strcasecmp(tag, "Icy-MetaInt")) {
    s->icy_metaint = strtoull(p, NULL, 10);

http_connect() resets the other response-scoped fields before reading the new response's headers (lines 1719-1731), but icy_metaint is missing from the list:

/* 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

Likewise ff_http_do_new_request2() (lines 597-600) resets icy_data_read but not icy_metaint:

s->end_chunked_post = 0;
s->chunkend      = 0;
s->off           = 0;
s->icy_data_read = 0;

http_read() then keys the ICY de-framing solely off the stale value (lines 2044-2058):

static int http_read(URLContext *h, uint8_t *buf, int size)
{
    HTTPContext *s = h->priv_data;

    if (s->icy_metaint > 0) {
        size = store_icy(h, size);
        if (size < 0)
            return size;
    }

    size = http_read_stream(h, buf, size);
    if (size > 0)
        s->icy_data_read += size;
    return size;
}

Step-by-step trace:

1. A request is sent with Icy-MetaData: 1 (default, icy option at line 206, header added at lines 1691-1692). The response contains Icy-MetaInt: 8192, so process_line() sets s->icy_metaint = 8192 (line 1347).
2. A new resource is requested on the same HTTPContext — e.g. ff_http_do_new_request2() from hls.c:650 for the next segment on a keep-alive connection, or a reconnect inside http_read_stream() (line 1951 → http_seek_internal → http_open_cnx), or a redirect. Both reset paths clear icy_data_read (lines 600, 1724) but leave icy_metaint = 8192.
3. The new response carries no Icy-MetaInt header (plain HTTP resource, or a CDN edge that strips ICY headers), so nothing overwrites the stale value.
4. http_read() still sees s->icy_metaint > 0 and routes every read through store_icy() (lines 2006-2042): after each 8192 bytes of body, one body byte is consumed as the metadata length ch, then ch * 16 further body bytes are consumed via http_read_stream_all() and stuffed into the icy_metadata_packet export option.
5. Those bytes are deleted from the payload handed to the caller, and arbitrary payload bytes are exported as ICY "metadata". No error is ever raised.

Note that every other response-scoped field is correctly re-initialized per request (filesize, willclose, compressed, end_header, icy_data_read, and chunksize in http_read_header() at line 1483) — icy_metaint is the one exception.

Impact

Silent data corruption: the caller receives a payload with bytes removed at fixed intervals, with no error or warning. For media this manifests as decode errors or audible/visible glitches; for anything else fetched over the same reused connection (HLS playlists/segments, sidecar files) it is plain wrong data. The corruption only triggers when an ICY response and a non-ICY response share one HTTPContext (keep-alive reuse, redirect, or reconnect), which makes it rare but very hard to diagnose when it happens. Severity: medium.

This bug also exists in upstream FFmpeg (upstream/master libavformat/http.c: icy_metaint set at line 1306, init block at line ~1660 and ff_http_do_new_request2 at line 571 reset only icy_data_read), so it is worth reporting upstream as well. It is not a regression introduced by this fork.

Suggested fix

Reset icy_metaint alongside icy_data_read in http_connect()'s per-request init block. http_read_header() runs after this block, so a response that does carry Icy-MetaInt re-populates it correctly:

--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -1721,6 +1721,7 @@ static int http_connect(URLContext *h, const char *path, const char *local_path
     s->line_count       = 0;
     s->off              = 0;
     s->icy_data_read    = 0;
+    s->icy_metaint      = 0;
     s->filesize         = UINT64_MAX;
     s->willclose        = 0;
     s->end_chunked_post = 0;

This covers all paths (ff_http_do_new_request2, redirects, and reconnects) since they all go through http_open_cnx() → http_connect().