[FFmpeg-devel] [PATCH] lavf/http: Export headers as AVDictionary

Stephan Holljes klaxa1337 at googlemail.com
Wed Aug 26 18:41:51 CEST 2015


On Tue, Aug 25, 2015 at 4:51 PM, Nicolas George <george at nsup.org> wrote:
> Le quintidi 5 fructidor, an CCXXIII, Stephan Holljes a écrit :
>> Signed-off-by: Stephan Holljes <klaxa1337 at googlemail.com>
>> ---
>>  libavformat/http.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/libavformat/http.c b/libavformat/http.c
>> index fba87ac..064239b 100644
>> --- a/libavformat/http.c
>> +++ b/libavformat/http.c
>> @@ -48,6 +48,7 @@
>>  #define MAX_REDIRECTS 8
>>  #define HTTP_SINGLE   1
>>  #define HTTP_MUTLI    2
>> +#define MAX_HEADER_LINES 100
>>  typedef enum {
>>      LOWER_PROTO,
>>      READ_HEADERS,
>> @@ -69,6 +70,8 @@ typedef struct HTTPContext {
>>      HTTPAuthState auth_state;
>>      HTTPAuthState proxy_auth_state;
>>      char *headers;
>> +    AVDictionary *headers_dict;
>> +    int nb_headers;
>>      char *mime_type;
>>      char *user_agent;
>>      char *content_type;
>> @@ -128,6 +131,7 @@ static const AVOption options[] = {
>>      { "seekable", "control seekability of connection", OFFSET(seekable), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 1, D },
>>      { "chunked_post", "use(s) chunked transfer-encoding for posts", OFFSET(chunked_post), AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, D | E },
>>      { "headers", "set custom HTTP headers, can override built in default headers", OFFSET(headers), AV_OPT_TYPE_STRING, { 0 }, 0, 0, D | E },
>> +    { "headers_dict", "Contains the parsed headers as a dictionary.", OFFSET(headers_dict), AV_OPT_TYPE_DICT, { 0 }, 0, 0, AV_OPT_FLAG_EXPORT | D | E },
>>      { "content_type", "set a specific content type for the POST messages", OFFSET(content_type), AV_OPT_TYPE_STRING, { 0 }, 0, 0, D | E },
>>      { "body", "set the body of a simple HTTP reply", OFFSET(body), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, E },
>>      { "user_agent", "override User-Agent header", OFFSET(user_agent), AV_OPT_TYPE_STRING, { .str = DEFAULT_USER_AGENT }, 0, 0, D },
>> @@ -910,6 +914,14 @@ static int process_line(URLContext *h, char *line, int line_count,
>>                  return AVERROR(ENOMEM);
>>          }
>>      }
>
>> +    if (s->nb_headers < MAX_HEADER_LINES) {
>> +        if (av_dict_get(s->headers_dict, tag, NULL, 0)) {
>> +            av_dict_set(&s->headers_dict, tag, ",", AV_DICT_APPEND);
>> +            av_dict_set(&s->headers_dict, tag, p, AV_DICT_APPEND);
>> +        } else
>> +            av_dict_set(&s->headers_dict, tag, p, 0);
>> +        s->nb_headers++;
>> +    }
>
> Nit: the usual coding style for FFmpeg is to put braces on both the if
> clause and the else clause if one of them needs them.

Ok.

>
> This version has a more severe problem: it stores everything sent by the
> client without a limit if it is given in duplicated headers. A malicious
> client could exhaust the server's memory by sending endless similar headers.

s->nb_headers is still being increased for duplicated headers, too.

>
> I strongly suggest to limit both the total number of different headers as
> you already do and the (approximate) total size of the headers data.

Apache enforces a default limit of 8190 bytes per HTTP header field.
Is there anything that speaks against using that too? (I took the 100
HTTP header limit from the Apache default too.)

>
>>      return 1;
>>  }
>>
>> @@ -1032,6 +1044,7 @@ static int http_read_header(URLContext *h, int *new_location)
>>      int err = 0;
>>
>>      s->chunksize = -1;
>> +    s->nb_headers = 0;
>>
>>      for (;;) {
>>          if ((err = http_get_line(s, line, sizeof(line))) < 0)
>
> Note: I write this mail assuming you intend to continue working on the
> project in your free time. Of course, whether you decide to or not does not
> affect the outcome of the GsoC evaluation.

I at least intend to finish what I started, preferably more if
interesting things show up :)

>
> Regards,
>
> --
>   Nicolas George

Regards,
Stephan


More information about the ffmpeg-devel mailing list