[FFmpeg-devel] [PATCH] Ignore expired cookies

[Micah Galizia]

On 2017-03-25 07:11 AM, wm4 wrote:

<snip>

>> -        while ((param = av_strtok(cookie, "; ", &next_param))) {
>> +        while ((param = av_strtok(cookie, ";", &next_param))) {
>> +
>> +            // move past any leading whitespace
>> +            param += strspn(param, WHITESPACES);
>> +
> Not quite sure why this change is even needed, but seems ok.

Its just a little more flexible than expecting a single space and only a space between each delimited field of a cookie.

>>               if (cookie) {
>>                   // first key-value pair is the actual cookie value
>>                   cvalue = av_strdup(param);
>> @@ -899,6 +905,33 @@ static int get_cookies(HTTPContext *s, char **cookies, const char *path,
>>                   int leading_dot = (param[7] == '.');
>>                   av_free(cdomain);
>>                   cdomain = av_strdup(&param[7+leading_dot]);
>> +            } else if (!av_strncasecmp("expires=", param, 8)) {
>> +                int i = 0, j = 0;
>> +                struct tm tm_buf = {0};
>> +                char *expiry = av_strdup(&param[8]);
> Unchecked memory allocation that could fail.

Thanks, resolved using the on-stack buffer described below.

>> +
>> +                // strip off any punctuation or whitespace
>> +                for (; i < strlen(expiry); i++) {
> A bit ugly stylistically: the i=0 initialization should be in here, and
> the strlen should be cached in a variable, instead of recomputing it on
> every loop.

Fixed.

>> +                    if ((expiry[i] >= '0' && expiry[i] <= '9') ||
>> +                        (expiry[i] >= 'A' && expiry[i] <= 'Z') ||
>> +                        (expiry[i] >= 'a' && expiry[i] <= 'z')) {
>> +                        expiry[j] = expiry[i];
>> +                        j++;
>> +                    }
>> +                }
>> +                expiry[j] = '\0';
> (To be honest I don't understand why the string is even strduped - you
> could just make it with a fixed-sized on-stack buffer. Just discard the
> remainder of the string if the buffer is unexpectedly too small - time
> strings probably have an upper size bound anyway.)

Changed.

>> +
>> +                // move the string beyond the day of week
>> +                i = 0;
>> +                while ((expiry[i] < '0' || expiry[i] > '9') && (i < j))
>> +                    i++;
>> +
>> +                if (av_small_strptime(&expiry[i], "%d%b%Y%H%M%SGMT", &tm_buf)) {
>> +                    if (av_timegm(&tm_buf) < time(NULL))
>> +                        expired = 1;
> A bit unsure about this time() call. Also nervous about having this in
> library code.

I borrowed some code from parseutils to resolve this.

Thanks for the prompt review.