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

[Eli Kara]

> -----Original Message-----
> From: ffmpeg-devel-bounces at ffmpeg.org [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of Michael Niedermayer
> Sent: Wednesday, January 22, 2014 5:16 AM
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] [PATCH] Properly handle cookies that specify sub-domain where the URL.

> 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 .

You're right, it is simpler. Thanks for pointing it out. Here is the revised patch. I also compiled XBMC's ffmpeg version with it to make
sure it works (as expected).

Thanks again,
Eli

---
 libavformat/http.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/libavformat/http.c b/libavformat/http.c
index 3b655c6..362a50e 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -490,8 +490,11 @@ static int get_cookies(HTTPContext *s, char **cookies, const char *path,
                 av_free(cpath);
                 cpath = av_strdup(&param[5]);
             } else if (!av_strncasecmp("domain=", param, 7)) {
+                // the domain in the URL 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 leading_dot = (param[7] == '.') ? 1 : 0;
                 av_free(cdomain);
-                cdomain = av_strdup(&param[7]);
+                cdomain = av_strdup(&param[7+leading_dot]);
             } else if (!av_strncasecmp("secure",  param, 6) ||
                        !av_strncasecmp("comment", param, 7) ||
                        !av_strncasecmp("max-age", param, 7) ||
@@ -516,12 +519,13 @@ 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
+        // 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)
         domain_offset = strlen(domain) - strlen(cdomain);
         if (domain_offset < 0)
             goto done_cookie;
 
-        // match the cookie domain
+        // 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))
             goto done_cookie;
 
-- 
1.8.3.msysgit.0