Hi,

The current Set-Cookie handling appends cookies to the string without
checking to see if a cookie with the same name should have its value
updated.

This is my second attempt to fix this issue -- last time it was a bit
of a string parsing nightmare. This time its a little less complex
(using a dictionary).

Before we start the "the existing implementation sucks, change it"
discussion I've looked into passing them around in their dictionary
form (an only using the string to initialize cookies from an external
source) but so far as I can tell, AVDictonary cannot be retrieved
using av_opt_get or av_opt_find right now.

Anyway, I hope this is sufficient for inclusion.

TIA
-- 
"The mark of an immature man is that he wants to die nobly for a
cause, while the mark of the mature man is that he wants to live
humbly for one."   --W. Stekel
From 18b00063d177965facd805d6b89eb96af371de94 Mon Sep 17 00:00:00 2001
From: Micah Galizia <micahgali...@gmail.com>
Date: Tue, 17 Mar 2015 20:22:59 +1100
Subject: [PATCH 3/3] replace cookies with updated values instead of appending
 forever

---
 libavformat/http.c | 65 +++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 52 insertions(+), 13 deletions(-)

diff --git a/libavformat/http.c b/libavformat/http.c
index 86380b2..da3c9be 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -77,6 +77,8 @@ typedef struct HTTPContext {
     int is_akamai;
     int is_mediagateway;
     char *cookies;          ///< holds newline (\n) delimited Set-Cookie header field values (without the "Set-Cookie: " field name)
+    /* A dictionary containing cookies keyed by cookie name */
+    AVDictionary *cookie_dict;
     int icy;
     /* how much data was read since the last ICY metadata packet */
     int icy_data_read;
@@ -466,6 +468,43 @@ static int parse_icy(HTTPContext *s, const char *tag, const char *p)
     return 0;
 }
 
+static int parse_cookie(HTTPContext *s, const char *p, AVDictionary **cookies)
+{
+    char *eql, *name;
+
+    // duplicate the cookie name (dict will dupe the value)
+    if (!(eql = strchr(p, '='))) return AVERROR(EINVAL);
+    if (!(name = av_strndup(p, eql - p))) return AVERROR(ENOMEM);
+
+    // add the cookie to the dictionary
+    av_dict_set(cookies, name, eql, AV_DICT_DONT_STRDUP_KEY);
+
+    return 0;
+}
+
+static int cookie_string(AVDictionary *dict, char **cookies)
+{
+    AVDictionaryEntry *e = NULL;
+    int len = 1;
+
+    // determine how much memory is needed for the cookies string
+    while (e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX))
+        len += strlen(e->key) + strlen(e->value) + 1;
+
+    // reallocate the cookies
+    e = NULL;
+    if (*cookies) av_free(*cookies);
+    *cookies = av_malloc(len);
+    if (!cookies) return AVERROR(ENOMEM);
+    *cookies[0] = '\0';
+
+    // write out the cookies
+    while (e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX))
+        av_strlcatf(*cookies, len, "%s%s\n", e->key, e->value);
+
+    return 0;
+}
+
 static int process_line(URLContext *h, char *line, int line_count,
                         int *new_location)
 {
@@ -537,19 +576,8 @@ static int process_line(URLContext *h, char *line, int line_count,
             av_free(s->mime_type);
             s->mime_type = av_strdup(p);
         } else if (!av_strcasecmp(tag, "Set-Cookie")) {
-            if (!s->cookies) {
-                if (!(s->cookies = av_strdup(p)))
-                    return AVERROR(ENOMEM);
-            } else {
-                char *tmp = s->cookies;
-                size_t str_size = strlen(tmp) + strlen(p) + 2;
-                if (!(s->cookies = av_malloc(str_size))) {
-                    s->cookies = tmp;
-                    return AVERROR(ENOMEM);
-                }
-                snprintf(s->cookies, str_size, "%s\n%s", tmp, p);
-                av_free(tmp);
-            }
+            if (parse_cookie(s, p, &s->cookie_dict))
+                av_log(h, AV_LOG_WARNING, "Unable to parse '%s'\n", p);
         } else if (!av_strcasecmp(tag, "Icy-MetaInt")) {
             s->icy_metaint = strtoll(p, NULL, 10);
         } else if (!av_strncasecmp(tag, "Icy-", 4)) {
@@ -580,12 +608,19 @@ static int get_cookies(HTTPContext *s, char **cookies, const char *path,
 
     if (!set_cookies) return AVERROR(EINVAL);
 
+    // destroy any cookies in the dictionary.
+    av_dict_free(&s->cookie_dict);
+
     *cookies = NULL;
     while ((cookie = av_strtok(set_cookies, "\n", &next))) {
         int domain_offset = 0;
         char *param, *next_param, *cdomain = NULL, *cpath = NULL, *cvalue = NULL;
         set_cookies = NULL;
 
+        // store the cookie in a dict in case it is updated in the response
+        if (parse_cookie(s, cookie, &s->cookie_dict))
+            av_log(s, AV_LOG_WARNING, "Unable to parse '%s'\n", cookie);
+
         while ((param = av_strtok(cookie, "; ", &next_param))) {
             if (cookie) {
                 // first key-value pair is the actual cookie value
@@ -693,6 +728,10 @@ static int http_read_header(URLContext *h, int *new_location)
     if (s->seekable == -1 && s->is_mediagateway && s->filesize == 2000000000)
         h->is_streamed = 1; /* we can in fact _not_ seek */
 
+    // add any new cookies into the existing cookie string
+    cookie_string(s->cookie_dict, &s->cookies);
+    av_dict_free(&s->cookie_dict);
+
     return err;
 }
 
-- 
2.1.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to