[FFmpeg-devel] [PATCH] Fix HTTP authentication problem for POST actions.

Michael Niedermayer michaelni at gmx.at
Wed Sep 11 16:34:38 CEST 2013


Hi

On Wed, Sep 11, 2013 at 03:05:48PM +0200, Jakob van Bethlehem wrote:
> From: "J. van Bethlehem" <jakob at jet-stream.nl>
> 
> Upon executing HTTP POST requests, ffmpeg started the POST action
> immediately after sending initial headers, therefore making it
> impossible to properly deal with a 401 Authentication challenge from
> the target server. This fix forces an initial empty POST request,
> postponing POSTing data until after the proper HTTP authentication
> header has been constructed
> 
> Signed-off-by: J. van Bethlehem <jakob at jet-stream.nl>
> ---
>  libavformat/http.c | 82 +++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 47 insertions(+), 35 deletions(-)
> 
> diff --git a/libavformat/http.c b/libavformat/http.c
> index 3edddbf..9887490 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -194,6 +194,7 @@ static int http_open_cnx(URLContext *h)
>      if (http_connect(h, path, local_path, hoststr, auth, proxyauth, &location_changed) < 0)
>          goto fail;
>      attempts++;
> +  
>      if (s->http_code == 401) {
>          if ((cur_auth_type == HTTP_AUTH_NONE || s->auth_state.stale) &&
>              s->auth_state.auth_type != HTTP_AUTH_NONE && attempts < 4) {
> @@ -553,7 +554,6 @@ static int http_read_header(URLContext *h, int *new_location)
>              return err;
>  
>          av_dlog(NULL, "header='%s'\n", line);
> -
>          err = process_line(h, line, s->line_count, new_location);
>          if (err < 0)
>              return err;
> @@ -570,31 +570,37 @@ static int http_connect(URLContext *h, const char *path, const char *local_path,
>                          const char *proxyauth, int *new_location)
>  {
>      HTTPContext *s = h->priv_data;
> -    int post, err;
> +    int post, err, auth_phase;
>      char headers[4096] = "";
>      char *authstr = NULL, *proxyauthstr = NULL;
>      int64_t off = s->off;
>      int len = 0;
>      const char *method;
> -
> -
> -    /* send http header */
> +  
> +    // Determine HTTP method
>      post = h->flags & AVIO_FLAG_WRITE;
> -
>      if (s->post_data) {
>          /* force POST method and disable chunked encoding when
>           * custom HTTP post data is set */
>          post = 1;
>          s->chunked_post = 0;
>      }
> -
>      method = post ? "POST" : "GET";
> +
> +    /* If we expect to need authorization (ie: auth is non-NULL), but
> +     * the current authorization state is still HTTP_AUTH_NONE, assume
> +     * this is the first call to a server in order to get a 401
> +     * with the authentication scheme to use. This modus has an
> +     * impact in particular for POST-actions, for which the actual
> +     * posting needs to be postponed untill after the first pass
> +     */
> +    auth_phase = auth && s->auth_state.auth_type == HTTP_AUTH_NONE && s->http_code != 401;
>      authstr = ff_http_auth_create_response(&s->auth_state, auth, local_path,
>                                             method);
>      proxyauthstr = ff_http_auth_create_response(&s->proxy_auth_state, proxyauth,
>                                                  local_path, method);


>  
> -    /* set default headers if needed */
> +    /* Set default headers if needed */
>      if (!has_header(s->headers, "\r\nUser-Agent: "))
>          len += av_strlcatf(headers + len, sizeof(headers) - len,
>                             "User-Agent: %s\r\n", s->user_agent);

please dont make unrelated changes, instead post seperate patches,
that is one for each self contained change
"fix comments grammer and spelling"
"factorize foobar"
"fix this bug"
"fix that bug"





> @@ -607,7 +613,9 @@ static int http_connect(URLContext *h, const char *path, const char *local_path,
>      if (!has_header(s->headers, "\r\nRange: ") && !post && (s->off > 0 || s->seekable == -1))
>          len += av_strlcatf(headers + len, sizeof(headers) - len,
>                             "Range: bytes=%"PRId64"-\r\n", s->off);
> -
> +    if (!has_header(s->headers, "\r\nHost: "))
> +      len += av_strlcatf(headers + len, sizeof(headers) - len,
> +                         "Host: %s\r\n", hoststr);

indention should be consistent within each file


>      if (!has_header(s->headers, "\r\nConnection: ")) {
>          if (s->multiple_requests) {
>              len += av_strlcpy(headers + len, "Connection: keep-alive\r\n",
> @@ -617,16 +625,6 @@ static int http_connect(URLContext *h, const char *path, const char *local_path,
>                                sizeof(headers) - len);
>          }
>      }
> -
> -    if (!has_header(s->headers, "\r\nHost: "))
> -        len += av_strlcatf(headers + len, sizeof(headers) - len,
> -                           "Host: %s\r\n", hoststr);

why is this moved around ?



> -    if (!has_header(s->headers, "\r\nContent-Length: ") && s->post_data)
> -        len += av_strlcatf(headers + len, sizeof(headers) - len,
> -                           "Content-Length: %d\r\n", s->post_datalen);
> -    if (!has_header(s->headers, "\r\nContent-Type: ") && s->content_type)
> -        len += av_strlcatf(headers + len, sizeof(headers) - len,
> -                           "Content-Type: %s\r\n", s->content_type);
>      if (!has_header(s->headers, "\r\nCookie: ") && s->cookies) {
>          char *cookies = NULL;
>          if (!get_cookies(s, &cookies, path, hoststr)) {
> @@ -640,33 +638,54 @@ static int http_connect(URLContext *h, const char *path, const char *local_path,
>                             "Icy-MetaData: %d\r\n", 1);
>      }
>  
> -    /* now add in custom headers */
> +    /* The next set of headers should only be added when this is not
> +     * the initial (POST) authentication pass */
> +    if (!auth_phase) {
> +      if (!has_header(s->headers, "\r\nContent-Length: ") && s->post_data)
> +          len += av_strlcatf(headers + len, sizeof(headers) - len,
> +                             "Content-Length: %d\r\n", s->post_datalen);
> +      if (!has_header(s->headers, "\r\nContent-Type: ") && s->content_type)
> +          len += av_strlcatf(headers + len, sizeof(headers) - len,
> +                             "Content-Type: %s\r\n", s->content_type);


> +      if (!has_header(s->headers, "\r\nTransfer-Encoding") && post && s->chunked_post)
> +          len += av_strlcatf(headers + len, sizeof(headers) - len,
> +                             "Transfer-Encoding: %s\r\n", "chunked");
> +    }
> +  
> +    /* Add any custom headers */
>      if (s->headers)
>          av_strlcpy(headers + len, s->headers, sizeof(headers) - len);
>  
> +    /* Format and send headers */
>      snprintf(s->buffer, sizeof(s->buffer),
>               "%s %s HTTP/1.1\r\n"
>               "%s"
>               "%s"
> -             "%s"
>               "%s%s"
>               "\r\n",
>               method,
>               path,
> -             post && s->chunked_post ? "Transfer-Encoding: chunked\r\n" : "",

the new code checks has_header() the old doesnt. why and it seems
unrelated to the auth fix

also please see tools/patcheck

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130911/030d4d9c/attachment.asc>


More information about the ffmpeg-devel mailing list