[FFmpeg-devel] [PATCH] Fix HTTP authentication problem for POST actions.
jakob
jakob at jet-stream.nl
Sat Oct 5 10:49:34 CEST 2013
Hello,
After being busy with some other stuff, finally I had some time to go
back to this fix. One important mystery was solved in the meantime: I
was not able to retrieve the original bug reports on any of the
ffmpeg-mail lists, simply because they were posted on the libav-dev list
:P. Personally I had never heard about libav, but my colleague was under
the impression that libav now is a library used by ffmpeg. After reading
up a bit about libav/ffmpeg I'm wiser than that, yet the information is
relevant and is here:
http://lists.libav.org/pipermail/libav-tools/2013-August/000296.html.
After improving based on the remarks I intend to re-commit. I have a
question though:
* Is it desired to create a bug-report first for ffmpeg for having some
(future) reference?
On 11.09.2013 16:34, Michael Niedermayer wrote:
> 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
>> [ ... ]
> 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"
I'll work on that.
>> @@ -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
Thought I had this covered: I'll take care of this.
>
>
>> 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 ?
I believe there was a reason: I'll make sure to create separate commits
to explain when re-committing (or put it back if there was no reason)
>> [ ... ]
>
> the new code checks has_header() the old doesnt. why and it seems
> unrelated to the auth fix
Sounds like an oversight. I'll make sure also here to explain or remove
when re-committing
> also please see tools/patcheck
Did not report relevant issues.
Sincerely,
Jakob van Bethlehem
More information about the ffmpeg-devel
mailing list