On Thu, Apr 19, 2018 at 1:04 PM, wm4 <nfx...@googlemail.com> wrote:
> On Thu, 19 Apr 2018 12:55:00 -0700
> rshaf...@tunein.com wrote:
>
> > From: Richard Shaffer <rshaf...@tunein.com>
> >
> > This refactors get_cookies to simplify some code paths, specifically for
> > skipping logic in the while loop or exiting it. It also simplifies the
> logic
> > for appending additional values to *cookies by replacing
> strlen/malloc/snprintf
> > with one call av_asnprintf.
> >
> > This refactor fixes a bug where the cookie_params AVDictionary would get
> leaked
> > if we failed to allocate a new buffer for writing to *cookies.
> > ---
> > Updated so that next = set_cookies = av_strdup(s->cookies) assignment is
> done
> > on a separate line instead of inside the if conditional.
> >
> > libavformat/http.c | 65 +++++++++++++++++++++++-------
> ------------------------
> > 1 file changed, 28 insertions(+), 37 deletions(-)
> >
> > diff --git a/libavformat/http.c b/libavformat/http.c
> > index b4a1919f24..d59ffbbbe8 100644
> > --- a/libavformat/http.c
> > +++ b/libavformat/http.c
> > @@ -1015,7 +1015,8 @@ static int process_line(URLContext *h, char *line,
> int line_count,
> > /**
> > * Create a string containing cookie values for use as a HTTP cookie
> header
> > * field value for a particular path and domain from the cookie values
> stored in
> > - * the HTTP protocol context. The cookie string is stored in *cookies.
> > + * the HTTP protocol context. The cookie string is stored in *cookies,
> and may
> > + * be NULL if there are no valid cookies.
> > *
> > * @return a negative value if an error condition occurred, 0 otherwise
> > */
> > @@ -1025,15 +1026,20 @@ static int get_cookies(HTTPContext *s, char
> **cookies, const char *path,
> > // cookie strings will look like Set-Cookie header field values.
> Multiple
> > // Set-Cookie fields will result in multiple values delimited by a
> newline
> > int ret = 0;
> > - char *cookie, *set_cookies = av_strdup(s->cookies), *next =
> set_cookies;
> > -
> > - if (!set_cookies) return AVERROR(EINVAL);
> > + char *cookie, *set_cookies, *next;
> >
> > // destroy any cookies in the dictionary.
> > av_dict_free(&s->cookie_dict);
> >
> > + if (!s->cookies)
> > + return 0;
> > +
> > + next = set_cookies = av_strdup(s->cookies);
> > + if (!next)
> > + return AVERROR(ENOMEM);
> > +
> > *cookies = NULL;
> > - while ((cookie = av_strtok(next, "\n", &next))) {
> > + while ((cookie = av_strtok(next, "\n", &next)) && !ret) {
> > AVDictionary *cookie_params = NULL;
> > AVDictionaryEntry *cookie_entry, *e;
> >
> > @@ -1043,23 +1049,19 @@ static int get_cookies(HTTPContext *s, char
> **cookies, const char *path,
> >
> > // continue on to the next cookie if this one cannot be parsed
> > if (parse_set_cookie(cookie, &cookie_params))
> > - continue;
> > + goto skip_cookie;
> >
> > // if the cookie has no value, skip it
> > cookie_entry = av_dict_get(cookie_params, "", NULL,
> AV_DICT_IGNORE_SUFFIX);
> > - if (!cookie_entry || !cookie_entry->value) {
> > - av_dict_free(&cookie_params);
> > - continue;
> > - }
> > + if (!cookie_entry || !cookie_entry->value)
> > + goto skip_cookie;
> >
> > // if the cookie has expired, don't add it
> > if ((e = av_dict_get(cookie_params, "expires", NULL, 0)) &&
> e->value) {
> > struct tm tm_buf = {0};
> > if (!parse_set_cookie_expiry_time(e->value, &tm_buf)) {
> > - if (av_timegm(&tm_buf) < av_gettime() / 1000000) {
> > - av_dict_free(&cookie_params);
> > - continue;
> > - }
> > + if (av_timegm(&tm_buf) < av_gettime() / 1000000)
> > + goto skip_cookie;
> > }
> > }
> >
> > @@ -1067,42 +1069,31 @@ static int get_cookies(HTTPContext *s, char
> **cookies, const char *path,
> > if ((e = av_dict_get(cookie_params, "domain", NULL, 0)) &&
> e->value) {
> > // find the offset comparison is on the min domain (b.com,
> not a.b.com)
> > int domain_offset = strlen(domain) - strlen(e->value);
> > - if (domain_offset < 0) {
> > - av_dict_free(&cookie_params);
> > - continue;
> > - }
> > + if (domain_offset < 0)
> > + goto skip_cookie;
> >
> > // match the cookie domain
> > - if (av_strcasecmp(&domain[domain_offset], e->value)) {
> > - av_dict_free(&cookie_params);
> > - continue;
> > - }
> > + if (av_strcasecmp(&domain[domain_offset], e->value))
> > + goto skip_cookie;
> > }
> >
> > // ensure this cookie matches the path
> > e = av_dict_get(cookie_params, "path", NULL, 0);
> > - if (!e || av_strncasecmp(path, e->value, strlen(e->value))) {
> > - av_dict_free(&cookie_params);
> > - continue;
> > - }
> > + if (!e || av_strncasecmp(path, e->value, strlen(e->value)))
> > + goto skip_cookie;
> >
> > // cookie parameters match, so copy the value
> > if (!*cookies) {
> > - if (!(*cookies = av_asprintf("%s=%s", cookie_entry->key,
> cookie_entry->value))) {
> > - ret = AVERROR(ENOMEM);
> > - break;
> > - }
> > + *cookies = av_asprintf("%s=%s", cookie_entry->key,
> cookie_entry->value);
> > } else {
> > char *tmp = *cookies;
> > - size_t str_size = strlen(cookie_entry->key) +
> strlen(cookie_entry->value) + strlen(*cookies) + 4;
> > - if (!(*cookies = av_malloc(str_size))) {
> > - ret = AVERROR(ENOMEM);
> > - av_free(tmp);
> > - break;
> > - }
> > - snprintf(*cookies, str_size, "%s; %s=%s", tmp,
> cookie_entry->key, cookie_entry->value);
> > + *cookies = av_asprintf("%s; %s=%s", tmp, cookie_entry->key,
> cookie_entry->value);
> > av_free(tmp);
> > }
> > + if (!*cookies)
> > + ret = AVERROR(ENOMEM);
> > +
> > + skip_cookie:
> > av_dict_free(&cookie_params);
> > }
> >
>
> Pushed. (My mail client accidentally replied to the patch author only,
> so he'll see this a second time.)
>
Thanks.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
