PR #24232 opened by Romain Beauxis (toots) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24232 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24232.patch
This PR fixes issues with native HTTP parser accepting malformed response status lines. These issues were uncovered while working on adding `ICY` support to the new `libcurl` implementation. Before the changes, malformed HTTP status responses were mistakenly taken for valid `200` responses, prompting the parser to accept them and proceed further. One side-effect was that `ICY 200` requests were accepted (as well as `ICY 404` requests). The changes in this patchset remove this unexpected feature. Worth noting: `ICY` status response is very old and specific to shoutcast so I don't think that we should worry too much about support them. Opt-in support for them is planned as part of the upcoming changes to the new `libcurl` backend. See https://code.ffmpeg.org/toots/FFmpeg/commit/2cfcd0c11dc08b9d8a58c6962e71af69af68ff42 for tests details. >From 9b76690f4926fcd81a012b49bd161308f244f8a1 Mon Sep 17 00:00:00 2001 From: Romain Beauxis <[email protected]> Date: Fri, 21 Aug 2026 08:27:16 -0500 Subject: [PATCH 1/3] avformat/http: split the response status line parser out of process_line Makes the parser reachable from a test program. No functional change, beyond checking the allocation of the exported version string. --- libavformat/http.c | 59 +++++++++++++++++++++++++++++++++------------- libavformat/http.h | 21 +++++++++++++++++ 2 files changed, 64 insertions(+), 16 deletions(-) diff --git a/libavformat/http.c b/libavformat/http.c index 69d4c8ab72..50fd88970d 100644 --- a/libavformat/http.c +++ b/libavformat/http.c @@ -890,6 +890,34 @@ static int http_get_line(HTTPContext *s, char *line, int line_size) } } +int ff_http_parse_status_line(void *logctx, const char *line, HTTPStatusLine *st) +{ + const char *p = line; + char *end; + + memset(st, 0, sizeof(*st)); + + if (av_strncasecmp(p, "HTTP/1.0", 8) == 0) + st->willclose = 1; + + while (*p != '/' && *p != '\0') + p++; + while (*p == '/') + p++; + av_strlcpy(st->version, p, sizeof(st->version)); + + while (!av_isspace(*p) && *p != '\0') + p++; + while (av_isspace(*p)) + p++; + st->code = strtol(p, &end, 10); + st->reason = end; + + av_log(logctx, AV_LOG_TRACE, "http_code=%d\n", st->code); + + return 0; +} + static int check_http_code(URLContext *h, int http_code, const char *end) { HTTPContext *s = h->priv_data; @@ -1181,7 +1209,7 @@ static int process_line(URLContext *h, char *line, int line_count, int *parsed_h { HTTPContext *s = h->priv_data; const char *auto_method = h->flags & AVIO_FLAG_READ ? "POST" : "GET"; - char *tag, *p, *end, *method, *resource, *version; + char *tag, *p, *method, *resource, *version; int ret; /* end of header */ @@ -1245,25 +1273,24 @@ static int process_line(URLContext *h, char *line, int line_count, int *parsed_h } av_log(h, AV_LOG_TRACE, "HTTP version string: %s\n", version); } else { - if (av_strncasecmp(p, "HTTP/1.0", 8) == 0) - s->willclose = 1; - while (*p != '/' && *p != '\0') - p++; - while (*p == '/') - p++; - av_freep(&s->http_version); - s->http_version = av_strndup(p, 3); - while (!av_isspace(*p) && *p != '\0') - p++; - while (av_isspace(*p)) - p++; - s->http_code = strtol(p, &end, 10); + HTTPStatusLine st; - av_log(h, AV_LOG_TRACE, "http_code=%d\n", s->http_code); + if ((ret = ff_http_parse_status_line(h, p, &st)) < 0) + return ret; + + /* Only ever set: a keep-alive decision made earlier must survive. */ + if (st.willclose) + s->willclose = 1; + + av_freep(&s->http_version); + if (!(s->http_version = av_strdup(st.version))) + return AVERROR(ENOMEM); + + s->http_code = st.code; *parsed_http_code = 1; - if ((ret = check_http_code(h, s->http_code, end)) < 0) + if ((ret = check_http_code(h, s->http_code, st.reason)) < 0) return ret; } } else { diff --git a/libavformat/http.h b/libavformat/http.h index 08605c8f91..dd2c15789e 100644 --- a/libavformat/http.h +++ b/libavformat/http.h @@ -27,6 +27,27 @@ #define HTTP_HEADERS_SIZE 4096 +/** + * Parsed form of a response status line. + */ +typedef struct HTTPStatusLine { + char version[4]; /**< protocol version, e.g. "1.1" */ + int code; /**< response status code */ + const char *reason; /**< reason phrase, points into the parsed line */ + int willclose; /**< the version implies the connection will close */ +} HTTPStatusLine; + +/** + * Parse the status line of an HTTP response. + * + * @param logctx context used for logging, may be NULL + * @param line NUL terminated status line, without its trailing CRLF + * @param st filled in with the parsed status line + * @return 0 on success, a negative AVERROR code on failure + */ +int ff_http_parse_status_line(void *logctx, const char *line, + HTTPStatusLine *st); + /** * Initialize the authentication state based on another HTTP URLContext. * This can be used to pre-initialize the authentication parameters if -- 2.52.0 >From 2cfcd0c11dc08b9d8a58c6962e71af69af68ff42 Mon Sep 17 00:00:00 2001 From: Romain Beauxis <[email protected]> Date: Fri, 21 Aug 2026 08:27:25 -0500 Subject: [PATCH 2/3] tests/http: test response status line parsing Covers well formed status lines, a missing or non numeric status code, and lines that do not start with an HTTP version. --- libavformat/Makefile | 1 + libavformat/tests/.gitignore | 1 + libavformat/tests/http.c | 67 +++++++++++++++++++++++++++++++++ tests/fate/libavformat.mak | 4 ++ tests/ref/fate/http-status-line | 28 ++++++++++++++ 5 files changed, 101 insertions(+) create mode 100644 libavformat/tests/http.c create mode 100644 tests/ref/fate/http-status-line diff --git a/libavformat/Makefile b/libavformat/Makefile index 466f5d1894..038e0afd41 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -790,6 +790,7 @@ TESTPROGS = id3v2 \ FIFO-MUXER-TESTPROGS-$(CONFIG_NETWORK) += fifo_muxer TESTPROGS-$(CONFIG_FIFO_MUXER) += $(FIFO-MUXER-TESTPROGS-yes) TESTPROGS-$(CONFIG_FFRTMPCRYPT_PROTOCOL) += rtmpdh +TESTPROGS-$(CONFIG_HTTP_PROTOCOL) += http TESTPROGS-$(CONFIG_NETWORK) += noproxy TESTPROGS-$(CONFIG_SRTP) += srtp TESTPROGS-$(CONFIG_IMF_DEMUXER) += imf diff --git a/libavformat/tests/.gitignore b/libavformat/tests/.gitignore index 1807488603..2be3cc8fac 100644 --- a/libavformat/tests/.gitignore +++ b/libavformat/tests/.gitignore @@ -9,3 +9,4 @@ /srtp /url /seek_utils +/http diff --git a/libavformat/tests/http.c b/libavformat/tests/http.c new file mode 100644 index 0000000000..3767cf319b --- /dev/null +++ b/libavformat/tests/http.c @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2026 Romain Beauxis + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <stdio.h> + +#include "libavformat/http.h" + +static void test(const char *line) +{ + HTTPStatusLine st; + int ret; + + printf("\"%s\"\n", line); + + ret = ff_http_parse_status_line(NULL, line, &st); + if (ret < 0) { + printf(" rejected\n"); + return; + } + + printf(" version=\"%s\" code=%d willclose=%d reason=\"%s\"\n", + st.version, st.code, st.willclose, st.reason); +} + +int main(void) +{ + /* Well formed. */ + test("HTTP/1.1 200 OK"); + test("HTTP/1.0 200 OK"); + test("HTTP/1.1 404 Not Found"); + test("HTTP/1.1 500 Internal Server Error"); + test("HTTP/1.1 204 No Content"); + + /* Well formed, without a reason phrase. */ + test("HTTP/1.1 200"); + + /* Malformed: no version, no status code, or not HTTP at all. */ + test("HTTP/1.1"); + test("HTTP/1.1 OK"); + test("HTTP"); + test("200 OK"); + test("NOT A STATUS LINE"); + test(""); + + /* Shoutcast answers with its own status line, which is not HTTP. */ + test("ICY 200 OK"); + test("ICY 404 Not Found"); + + return 0; +} diff --git a/tests/fate/libavformat.mak b/tests/fate/libavformat.mak index 09649fed11..4125dde0b4 100644 --- a/tests/fate/libavformat.mak +++ b/tests/fate/libavformat.mak @@ -12,6 +12,10 @@ fate-rename: libavformat/tests/rename$(EXESUF) fate-rename: CMD = run libavformat/tests/rename$(EXESUF) fate-rename: CMP = null +FATE_LIBAVFORMAT-$(CONFIG_HTTP_PROTOCOL) += fate-http-status-line +fate-http-status-line: libavformat/tests/http$(EXESUF) +fate-http-status-line: CMD = run libavformat/tests/http$(EXESUF) + FATE_LIBAVFORMAT-$(CONFIG_NETWORK) += fate-noproxy fate-noproxy: libavformat/tests/noproxy$(EXESUF) fate-noproxy: CMD = run libavformat/tests/noproxy$(EXESUF) diff --git a/tests/ref/fate/http-status-line b/tests/ref/fate/http-status-line new file mode 100644 index 0000000000..20ce4b0889 --- /dev/null +++ b/tests/ref/fate/http-status-line @@ -0,0 +1,28 @@ +"HTTP/1.1 200 OK" + version="1.1" code=200 willclose=0 reason=" OK" +"HTTP/1.0 200 OK" + version="1.0" code=200 willclose=1 reason=" OK" +"HTTP/1.1 404 Not Found" + version="1.1" code=404 willclose=0 reason=" Not Found" +"HTTP/1.1 500 Internal Server Error" + version="1.1" code=500 willclose=0 reason=" Internal Server Error" +"HTTP/1.1 204 No Content" + version="1.1" code=204 willclose=0 reason=" No Content" +"HTTP/1.1 200" + version="1.1" code=200 willclose=0 reason="" +"HTTP/1.1" + version="1.1" code=0 willclose=0 reason="" +"HTTP/1.1 OK" + version="1.1" code=0 willclose=0 reason="OK" +"HTTP" + version="" code=0 willclose=0 reason="" +"200 OK" + version="" code=0 willclose=0 reason="" +"NOT A STATUS LINE" + version="" code=0 willclose=0 reason="" +"" + version="" code=0 willclose=0 reason="" +"ICY 200 OK" + version="" code=0 willclose=0 reason="" +"ICY 404 Not Found" + version="" code=0 willclose=0 reason="" -- 2.52.0 >From 023b69c97bc0ac2b709cfc804abed6d95b375b28 Mon Sep 17 00:00:00 2001 From: Romain Beauxis <[email protected]> Date: Fri, 21 Aug 2026 08:27:25 -0500 Subject: [PATCH 3/3] avformat/http: reject malformed response status lines A status line carrying no HTTP version parsed as status code 0, which check_http_code() then accepted as a success: the body of an error reply, or of a reply from something that is not an HTTP server at all, was handed to the demuxer. The request line parser in the same function already rejects a version string that does not start with "HTTP/"; do the same for responses, and reject a status line whose code is missing or not a number. --- libavformat/http.c | 18 ++++++++++++------ tests/ref/fate/http-status-line | 16 ++++++++-------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/libavformat/http.c b/libavformat/http.c index 50fd88970d..26130a20bc 100644 --- a/libavformat/http.c +++ b/libavformat/http.c @@ -897,20 +897,26 @@ int ff_http_parse_status_line(void *logctx, const char *line, HTTPStatusLine *st memset(st, 0, sizeof(*st)); - if (av_strncasecmp(p, "HTTP/1.0", 8) == 0) + if (av_strncasecmp(p, "HTTP/", 5)) { + av_log(logctx, AV_LOG_ERROR, "Malformed HTTP status line.\n"); + return AVERROR_INVALIDDATA; + } + p += 5; + + if (av_strncasecmp(line, "HTTP/1.0", 8) == 0) st->willclose = 1; - while (*p != '/' && *p != '\0') - p++; - while (*p == '/') - p++; av_strlcpy(st->version, p, sizeof(st->version)); while (!av_isspace(*p) && *p != '\0') p++; while (av_isspace(*p)) p++; - st->code = strtol(p, &end, 10); + st->code = strtol(p, &end, 10); + if (end == p) { + av_log(logctx, AV_LOG_ERROR, "Malformed HTTP status code.\n"); + return AVERROR_INVALIDDATA; + } st->reason = end; av_log(logctx, AV_LOG_TRACE, "http_code=%d\n", st->code); diff --git a/tests/ref/fate/http-status-line b/tests/ref/fate/http-status-line index 20ce4b0889..05ce524573 100644 --- a/tests/ref/fate/http-status-line +++ b/tests/ref/fate/http-status-line @@ -11,18 +11,18 @@ "HTTP/1.1 200" version="1.1" code=200 willclose=0 reason="" "HTTP/1.1" - version="1.1" code=0 willclose=0 reason="" + rejected "HTTP/1.1 OK" - version="1.1" code=0 willclose=0 reason="OK" + rejected "HTTP" - version="" code=0 willclose=0 reason="" + rejected "200 OK" - version="" code=0 willclose=0 reason="" + rejected "NOT A STATUS LINE" - version="" code=0 willclose=0 reason="" + rejected "" - version="" code=0 willclose=0 reason="" + rejected "ICY 200 OK" - version="" code=0 willclose=0 reason="" + rejected "ICY 404 Not Found" - version="" code=0 willclose=0 reason="" + rejected -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
