[FFmpeg-devel] [PATCH] Properly handle cookies that specify sub-domain where the URL.

[Michael Niedermayer]

On Tue, Jan 21, 2014 at 02:12:20PM +0000, Eli Kara wrote:
> Hi,
> 
> I've encountered a small bug in avformat with handling cookies that specify a subdomain when trying to play an HLS playlist (m3u8)
> that requires cookies. I've reported it in the bugtracker as #3336 (http://trac.ffmpeg.org/ticket/3336).
> 
> I'm only mentioning this here because I opened the bug before I knew it's possible to submit patches here and I'm not sure what's the proper procedure
> to get the issue fixed (I have a working fix).
> 
> The bug essentially happens when playing an m3u8 HLS playlist that requires cookies, but the server sets cookies that specify they are valid for an entire
> subdomain *AND* the URL to play the m3u8 playlist is pointing at the top-level domain. In this case, the domain matching fails (get_cookies function).
> 
> I'm using XBMC Gotham (ffmpeg 1.2 currently, but code hasn't changed in http.c) and tested the fix and it works as expected.
> It works for the 3 following scenarios:
> 
> 1. Cookie sets a specific domain name. i.e: foo.bar.com. - WORKS.
> 2. Cookie sets a subdomain. i.e: .bar.com . If the URL to the video is foo.bar.com/<file>.m3u8 - WORKS.
> 3. Cookie sets a subdomain. i.e: .bar.com . If the URL to the vide is bar.com/<file>.m3u8 - WORKS (whereas before it didn't).
> 
> Attached is the patch. What are the criteria for applying it ? I'm really hoping it will be accepted as team XBMC informed me that only if it's accepted in
> ffmpeg will they consider including it, even if it gets accepted into a newer version of the library.
> 
> You help is very much appreciated!
> 
> Thanks,
> Eli
> 
> ---
>  libavformat/http.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/http.c b/libavformat/http.c
> index 3b655c6..5b691de 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -516,13 +516,15 @@ static int get_cookies(HTTPContext *s, char **cookies, const char *path,
>          if (av_strncasecmp(path, cpath, strlen(cpath)))
>              goto done_cookie;
>  
> -        // the domain should be at least the size of our cookie domain
> -        domain_offset = strlen(domain) - strlen(cdomain);
> +        // the domain should be at least the size of our cookie domain. correctly support sub-domains AND the master domain
> +        // by not comparing the dot at all (ex: domain=bar.com and cdomain=.bar.com)
> +        int subdomain = (*cdomain == '.') ? 1 : 0;
> +        domain_offset = strlen(domain) - (strlen(cdomain)-subdomain);
>          if (domain_offset < 0)
>              goto done_cookie;
>  
> -        // match the cookie domain
> -        if (av_strcasecmp(&domain[domain_offset], cdomain))
> +        // match the cookie domain, skipping leading dot, if present. this works for sub-domains AND the master domain
> +        if (av_strcasecmp(&domain[domain_offset], cdomain+subdomain))
>              goto done_cookie;

i think it would be simpler to add 1 when needed before the
av_strdup() so cdomain never contains the .

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

Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
-------------- 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/20140122/3023fd53/attachment.asc>