[FFmpeg-devel] [PATCH] libavformat/hls: Observe Set-Cookie headers

[Micah Galizia]

On 2017-05-06 02:28 PM, Micah Galizia wrote:
> On 2017-05-05 09:28 PM, wm4 wrote:
>> On Fri,  5 May 2017 20:55:05 -0400
>> Micah Galizia <micahgalizia at gmail.com> wrote:
>>
>>> Signed-off-by: Micah Galizia <micahgalizia at gmail.com>
>>> ---
>>>   libavformat/hls.c | 12 ++++++++++--
>>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavformat/hls.c b/libavformat/hls.c
>>> index bac53a4350..bda9abecfa 100644
>>> --- a/libavformat/hls.c
>>> +++ b/libavformat/hls.c
>>> @@ -630,8 +630,16 @@ static int open_url(AVFormatContext *s, 
>>> AVIOContext **pb, const char *url,
>>>       ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp);
>>>       if (ret >= 0) {
>>>           // update cookies on http response with setcookies.
>>> -        void *u = (s->flags & AVFMT_FLAG_CUSTOM_IO) ? NULL : s->pb;
>>> -        update_options(&c->cookies, "cookies", u);
>>> +        char *new_cookies = NULL;
>>> +
>>> +        if (s->flags ^ AVFMT_FLAG_CUSTOM_IO)
>>> +            av_opt_get(*pb, "cookies", AV_OPT_SEARCH_CHILDREN, 
>>> (uint8_t**)&new_cookies);
>> Did you mean & instead of ^?
>
> No, the original code was structured to set *u to null (and thus did 
> not copy cookies) iff AVFMT_FLAG_CUSTOM_IO was set in the flags.  So 
> using ^ is logically equivalent -- cookies are copied only if 
> AVFMT_FLAG_CUSTOM_IO is not set.
>
>> Did you find out yet what difference AVFMT_FLAG_CUSTOM_IO is supposed
>> to make in the existing code?
> Yes, I wrote back about it a day or two ago... here is the reference 
> to its original inclusion: 
> https://lists.ffmpeg.org/pipermail/ffmpeg-trac/2013-February/013020.html. 
> When the code was originally implemented we were using s->pb->opaque, 
> which in FFMPEG we could assume was a URLContext with options (one of 
> them possibly being "cookies").
>
> Based on the email above, that wasn't true for other apps like 
> mplayer, and could cause crashes trying to treat opaque as a 
> URLContext. So that is the purpose of checking the 
> AVFMT_FLAG_CUSTOM_IO flag (way back in 2013).
>
> Now though, we don't seem to touch opaque at all. The options are 
> stored in AVIOContext's av_class, which does have options.  Based on 
> this I think both patches are safe: this version has less change, but 
> the original gets rid of the check against AVFMT_FLAG_CUSTOM_IO that I 
> don't think we need anymore.
>
> I hope that clears things up.

Hi, is there anything further needed to get this fix committed?

Thanks,