[1/4] http: Make custom headers settable via an AVOption

Message ID 1320659572-8118-1-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö Nov. 7, 2011, 9:52 a.m.
---
 libavformat/http.c |   28 ++++++++++++++++++++--------
 1 files changed, 20 insertions(+), 8 deletions(-)

Comments

Anton Khirnov Nov. 8, 2011, 5:50 p.m. | #1
On Mon,  7 Nov 2011 11:52:49 +0200, Martin Storsjö <martin@martin.st> wrote:
> ---
>  libavformat/http.c |   28 ++++++++++++++++++++--------
>  1 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/libavformat/http.c b/libavformat/http.c
> index 52e1886..3c9b2d0 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -47,13 +47,14 @@ typedef struct {
>      int64_t off, filesize;
>      char location[MAX_URL_SIZE];
>      HTTPAuthState auth_state;
> -    unsigned char headers[BUFFER_SIZE];
> +    char *headers;
>      int willclose;          /**< Set if the server correctly handles Connection: close and will close the connection after feeding us the content. */
>  } HTTPContext;
>  
>  #define OFFSET(x) offsetof(HTTPContext, x)
>  static const AVOption options[] = {
>  {"chunksize", "use chunked transfer-encoding for posts, -1 disables it, 0 enables it", OFFSET(chunksize), AV_OPT_TYPE_INT64, {.dbl = 0}, -1, 0 }, /* Default to 0, for chunked POSTs */
> +{"headers", "custom HTTP headers, can override built in default headers", OFFSET(headers), AV_OPT_TYPE_STRING },
>  {NULL}
>  };
>  static const AVClass httpcontext_class = {
> @@ -69,12 +70,9 @@ static int http_connect(URLContext *h, const char *path, const char *hoststr,
>  void ff_http_set_headers(URLContext *h, const char *headers)
>  {
>      HTTPContext *s = h->priv_data;
> -    int len = strlen(headers);
>  
> -    if (len && strcmp("\r\n", headers + len - 2))
> -        av_log(h, AV_LOG_ERROR, "No trailing CRLF found in HTTP header.\n");
> -
> -    av_strlcpy(s->headers, headers, sizeof(s->headers));
> +    av_freep(&s->headers);
> +    s->headers = av_strdup(headers);
>  }
>  
>  void ff_http_init_auth_state(URLContext *dest, const URLContext *src)
> @@ -162,13 +160,23 @@ static int http_open_cnx(URLContext *h)
>  static int http_open(URLContext *h, const char *uri, int flags)
>  {
>      HTTPContext *s = h->priv_data;
> +    int ret;
>  
>      h->is_streamed = 1;
>  
>      s->filesize = -1;
>      av_strlcpy(s->location, uri, sizeof(s->location));
>  
> -    return http_open_cnx(h);
> +    if (s->headers) {
> +        int len = strlen(s->headers);
> +        if (len < 2 || strcmp("\r\n", s->headers + len - 2))
> +            av_log(h, AV_LOG_ERROR, "No trailing CRLF found in HTTP header.\n");

I know it's just copied from above, but using AV_LOG_ERROR looks fishy.
Either it's really an error, then it should fail. Or it's not, then the
loglevel should be warning.

> +    }
> +
> +    ret = http_open_cnx(h);
> +    if (ret < 0)
> +        av_freep(&s->headers);
> +    return ret;
>  }
>  static int http_getc(HTTPContext *s)
>  {
> @@ -285,6 +293,8 @@ static int process_line(URLContext *h, char *line, int line_count,
>  static inline int has_header(const char *str, const char *header)
>  {
>      /* header + 2 to skip over CRLF prefix. (make sure you have one!) */
> +    if (!str)
> +        return 0;
>      return av_stristart(str, header + 2, NULL) || av_stristr(str, header);
>  }
>  
> @@ -323,7 +333,8 @@ static int http_connect(URLContext *h, const char *path, const char *hoststr,
>                             "Host: %s\r\n", hoststr);
>  
>      /* now add in custom headers */
> -    av_strlcpy(headers+len, s->headers, sizeof(headers)-len);
> +    if (s->headers)
> +        av_strlcpy(headers + len, s->headers, sizeof(headers) - len);
>  
>      snprintf(s->buffer, sizeof(s->buffer),
>               "%s %s HTTP/1.1\r\n"
> @@ -463,6 +474,7 @@ static int http_close(URLContext *h)
>  
>      if (s->hd)
>          ffurl_close(s->hd);
> +    av_freep(&s->headers);

av_opt_free() isn't called for protocols? Something to be fixed
I suppose.
Martin Storsjö Nov. 8, 2011, 6:12 p.m. | #2
On Tue, 8 Nov 2011, Anton Khirnov wrote:

>
> On Mon,  7 Nov 2011 11:52:49 +0200, Martin Storsjö <martin@martin.st> wrote:
>> ---
>>  libavformat/http.c |   28 ++++++++++++++++++++--------
>>  1 files changed, 20 insertions(+), 8 deletions(-)
>>
>> diff --git a/libavformat/http.c b/libavformat/http.c
>> index 52e1886..3c9b2d0 100644
>> --- a/libavformat/http.c
>> +++ b/libavformat/http.c
>> @@ -47,13 +47,14 @@ typedef struct {
>>      int64_t off, filesize;
>>      char location[MAX_URL_SIZE];
>>      HTTPAuthState auth_state;
>> -    unsigned char headers[BUFFER_SIZE];
>> +    char *headers;
>>      int willclose;          /**< Set if the server correctly handles Connection: close and will close the connection after feeding us the content. */
>>  } HTTPContext;
>>
>>  #define OFFSET(x) offsetof(HTTPContext, x)
>>  static const AVOption options[] = {
>>  {"chunksize", "use chunked transfer-encoding for posts, -1 disables it, 0 enables it", OFFSET(chunksize), AV_OPT_TYPE_INT64, {.dbl = 0}, -1, 0 }, /* Default to 0, for chunked POSTs */
>> +{"headers", "custom HTTP headers, can override built in default headers", OFFSET(headers), AV_OPT_TYPE_STRING },
>>  {NULL}
>>  };
>>  static const AVClass httpcontext_class = {
>> @@ -69,12 +70,9 @@ static int http_connect(URLContext *h, const char *path, const char *hoststr,
>>  void ff_http_set_headers(URLContext *h, const char *headers)
>>  {
>>      HTTPContext *s = h->priv_data;
>> -    int len = strlen(headers);
>>
>> -    if (len && strcmp("\r\n", headers + len - 2))
>> -        av_log(h, AV_LOG_ERROR, "No trailing CRLF found in HTTP header.\n");
>> -
>> -    av_strlcpy(s->headers, headers, sizeof(s->headers));
>> +    av_freep(&s->headers);
>> +    s->headers = av_strdup(headers);
>>  }
>>
>>  void ff_http_init_auth_state(URLContext *dest, const URLContext *src)
>> @@ -162,13 +160,23 @@ static int http_open_cnx(URLContext *h)
>>  static int http_open(URLContext *h, const char *uri, int flags)
>>  {
>>      HTTPContext *s = h->priv_data;
>> +    int ret;
>>
>>      h->is_streamed = 1;
>>
>>      s->filesize = -1;
>>      av_strlcpy(s->location, uri, sizeof(s->location));
>>
>> -    return http_open_cnx(h);
>> +    if (s->headers) {
>> +        int len = strlen(s->headers);
>> +        if (len < 2 || strcmp("\r\n", s->headers + len - 2))
>> +            av_log(h, AV_LOG_ERROR, "No trailing CRLF found in HTTP header.\n");
>
> I know it's just copied from above, but using AV_LOG_ERROR looks fishy.
> Either it's really an error, then it should fail. Or it's not, then the
> loglevel should be warning.

I can change that in a separate commit afterwards.

>> +    }
>> +
>> +    ret = http_open_cnx(h);
>> +    if (ret < 0)
>> +        av_freep(&s->headers);
>> +    return ret;
>>  }
>>  static int http_getc(HTTPContext *s)
>>  {
>> @@ -285,6 +293,8 @@ static int process_line(URLContext *h, char *line, int line_count,
>>  static inline int has_header(const char *str, const char *header)
>>  {
>>      /* header + 2 to skip over CRLF prefix. (make sure you have one!) */
>> +    if (!str)
>> +        return 0;
>>      return av_stristart(str, header + 2, NULL) || av_stristr(str, header);
>>  }
>>
>> @@ -323,7 +333,8 @@ static int http_connect(URLContext *h, const char *path, const char *hoststr,
>>                             "Host: %s\r\n", hoststr);
>>
>>      /* now add in custom headers */
>> -    av_strlcpy(headers+len, s->headers, sizeof(headers)-len);
>> +    if (s->headers)
>> +        av_strlcpy(headers + len, s->headers, sizeof(headers) - len);
>>
>>      snprintf(s->buffer, sizeof(s->buffer),
>>               "%s %s HTTP/1.1\r\n"
>> @@ -463,6 +474,7 @@ static int http_close(URLContext *h)
>>
>>      if (s->hd)
>>          ffurl_close(s->hd);
>> +    av_freep(&s->headers);
>
> av_opt_free() isn't called for protocols? Something to be fixed
> I suppose.

It might be - I just added it to be safe, without checking if it actually 
was necessary, I'll look into that.

// Martin
Martin Storsjö Nov. 8, 2011, 8:41 p.m. | #3
On Tue, 8 Nov 2011, Martin Storsjö wrote:

> On Tue, 8 Nov 2011, Anton Khirnov wrote:
>
>> 
>> On Mon,  7 Nov 2011 11:52:49 +0200, Martin Storsjö <martin@martin.st> 
>> wrote:
>>> @@ -463,6 +474,7 @@ static int http_close(URLContext *h)
>>>
>>>      if (s->hd)
>>>          ffurl_close(s->hd);
>>> +    av_freep(&s->headers);
>> 
>> av_opt_free() isn't called for protocols? Something to be fixed
>> I suppose.
>
> It might be - I just added it to be safe, without checking if it actually was 
> necessary, I'll look into that.

Without this, valgrind says the memory is leaked.

// Martin

Patch

diff --git a/libavformat/http.c b/libavformat/http.c
index 52e1886..3c9b2d0 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -47,13 +47,14 @@  typedef struct {
     int64_t off, filesize;
     char location[MAX_URL_SIZE];
     HTTPAuthState auth_state;
-    unsigned char headers[BUFFER_SIZE];
+    char *headers;
     int willclose;          /**< Set if the server correctly handles Connection: close and will close the connection after feeding us the content. */
 } HTTPContext;
 
 #define OFFSET(x) offsetof(HTTPContext, x)
 static const AVOption options[] = {
 {"chunksize", "use chunked transfer-encoding for posts, -1 disables it, 0 enables it", OFFSET(chunksize), AV_OPT_TYPE_INT64, {.dbl = 0}, -1, 0 }, /* Default to 0, for chunked POSTs */
+{"headers", "custom HTTP headers, can override built in default headers", OFFSET(headers), AV_OPT_TYPE_STRING },
 {NULL}
 };
 static const AVClass httpcontext_class = {
@@ -69,12 +70,9 @@  static int http_connect(URLContext *h, const char *path, const char *hoststr,
 void ff_http_set_headers(URLContext *h, const char *headers)
 {
     HTTPContext *s = h->priv_data;
-    int len = strlen(headers);
 
-    if (len && strcmp("\r\n", headers + len - 2))
-        av_log(h, AV_LOG_ERROR, "No trailing CRLF found in HTTP header.\n");
-
-    av_strlcpy(s->headers, headers, sizeof(s->headers));
+    av_freep(&s->headers);
+    s->headers = av_strdup(headers);
 }
 
 void ff_http_init_auth_state(URLContext *dest, const URLContext *src)
@@ -162,13 +160,23 @@  static int http_open_cnx(URLContext *h)
 static int http_open(URLContext *h, const char *uri, int flags)
 {
     HTTPContext *s = h->priv_data;
+    int ret;
 
     h->is_streamed = 1;
 
     s->filesize = -1;
     av_strlcpy(s->location, uri, sizeof(s->location));
 
-    return http_open_cnx(h);
+    if (s->headers) {
+        int len = strlen(s->headers);
+        if (len < 2 || strcmp("\r\n", s->headers + len - 2))
+            av_log(h, AV_LOG_ERROR, "No trailing CRLF found in HTTP header.\n");
+    }
+
+    ret = http_open_cnx(h);
+    if (ret < 0)
+        av_freep(&s->headers);
+    return ret;
 }
 static int http_getc(HTTPContext *s)
 {
@@ -285,6 +293,8 @@  static int process_line(URLContext *h, char *line, int line_count,
 static inline int has_header(const char *str, const char *header)
 {
     /* header + 2 to skip over CRLF prefix. (make sure you have one!) */
+    if (!str)
+        return 0;
     return av_stristart(str, header + 2, NULL) || av_stristr(str, header);
 }
 
@@ -323,7 +333,8 @@  static int http_connect(URLContext *h, const char *path, const char *hoststr,
                            "Host: %s\r\n", hoststr);
 
     /* now add in custom headers */
-    av_strlcpy(headers+len, s->headers, sizeof(headers)-len);
+    if (s->headers)
+        av_strlcpy(headers + len, s->headers, sizeof(headers) - len);
 
     snprintf(s->buffer, sizeof(s->buffer),
              "%s %s HTTP/1.1\r\n"
@@ -463,6 +474,7 @@  static int http_close(URLContext *h)
 
     if (s->hd)
         ffurl_close(s->hd);
+    av_freep(&s->headers);
     return ret;
 }