[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