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

[Michael Niedermayer]

On Fri, May 26, 2017 at 09:29:04PM -0400, Micah Galizia wrote:
> On Sat, May 20, 2017 at 9:36 PM, Micah Galizia <micahgalizia at gmail.com> wrote:
> > On 2017-05-17 05:23 AM, wm4 wrote:
> >>
> >> On Sat, 6 May 2017 14:28:10 -0400
> >> Micah Galizia <micahgalizia at gmail.com> 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.
> >>
> >> Sorry, I missed that. I guess this code is an artifact of some severely
> >> unclean hook up of the AVOPtion API to AVIOContext, that breaks with
> >> custom I/O. I guess your patch is fine then.
> >>
> >> I just wonder how an API user is supposed to pass along cookies then.
> >> My code passes an AVDictionary via a "cookies" entry when opening the
> >> HLS demuxer with custom I/O, so I was wondering whether the patch
> >> changes behavior here.
> >
> >
> > I wouldn't have expected anyone to pass the cookies to the HLS demuxer
> > directly -- there is an http protocol AVOption that should pass it along to
> > the demuxer. But I guess thats the whole point of the custom IO, right? It'd
> > replace the http demuxer?
> >
> > Even so, I don't think this is a good reason to hold up the the patch - it
> > corrects the problem for apps that use the http protocol and maintains the
> > existing behavior -- cookies are not (and were not) copied when the custom
> > flag is set because u was set to null. Am I wrong in that interpretation of
> > the existing behavior?
> 
> Hi,
> 
> What else do I need to do to get this fix checked in?

patch applied

thx

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

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170529/23092ded/attachment.sig>