I'm running into a problem with duplicated cookies. This is due to
misbehaving servers sending back duplicate Set-Cookie headers in the
same response, like this:
Set-Cookie: JSESSIONID=foo; Path=/; Secure
Set-Cookie: JSESSIONID=bar; Path=/; Secure
Firefox deals with this by picked the last cookie value specified.
There is no logic in drakma currently to handle that, and so my
cookie-jar is getting two JSESSION cookies, and further requests are
sending both cookies. I do not control the servers returning me these
duplicate headers.
This patch adds a unique-cookies function that iterates through cookie
objects, keeping only the last one. This is then called in get-cookies
to ensure no duplicate cookie objects are propagated to the rest of the
system. I've got my "make it work on a friday afternoon" solution in
the attached patch, but I'm not really happy with it long term, and
wanted some advice. Problems I see:
* I think the dolist/find combination is a recipe for performance
problems with many cookies
* It seems odd to build up a list of cookies, then rebuild it
* The most common case of no duplicate cookies has an extra bunch of
consing to rebuilding up the list
* I think there should be a user choice of "ignore all but the last
cookie" vs "ignore all but the first cookie" vs "leave them all in
there"
Some thoughts on how to proceed:
1. Add a exported *redundant-cookie-strategy* variable, with values
:keep-all, :keep-first, and :keep-last, defaulting to :keep-all
(the current behavior).
2. remove the unique-cookies function and rewrite get-cookies to
implement the *redundant-cookie-strategy*, making one pass through
parsed-cookies.
3. leave get-cookies mostly as-is, but add some detection for
duplicates, and run unique-cookies only if it we need to
Thanks,
Ryan
PS: I'm still kinda new to contributing on open source projects via
emailed patches, sorry if this is in a bad format. I read
http://weitz.de/patches.html and created this patch from my working copy
using >diff -u ../drakma-1.1.0/cookies.lisp cookies.lisp
--- ../drakma-1.1.0/cookies.lisp 2009-12-01 17:06:56.000000000 -0500
+++ cookies.lisp 2010-04-02 17:57:34.000000000 -0400
@@ -263,25 +263,48 @@
`Set-Cookie' header as found in HEADERS \(an alist as returned by
HTTP-REQUEST). Collects only cookies which match the domain of
the \(PURI) URI URI."
- (loop with set-cookie-header = (header-value :set-cookie headers)
- with parsed-cookies = (and set-cookie-header (parse-set-cookie
set-cookie-header))
- for (name value parameters) in parsed-cookies
- for expires = (parameter-value "expires" parameters)
- for domain = (or (parameter-value "domain" parameters) (uri-host uri))
- when (and (valid-cookie-domain-p domain)
- (cookie-domain-matches domain uri))
- collect (make-instance 'cookie
- :name name
- :value value
- :path (or (parameter-value "path" parameters)
- (uri-path uri)
- "/")
- :expires (and expires
- (plusp (length expires))
- (parse-cookie-date expires))
- :domain domain
- :securep (not (not (parameter-present-p
"secure" parameters)))
- :http-only-p (not (not (parameter-present-p
"HttpOnly" parameters))))))
+ (unique-cookies
+ (loop with set-cookie-header = (header-value :set-cookie headers)
+ with parsed-cookies = (and set-cookie-header (parse-set-cookie
set-cookie-header))
+ for (name value parameters) in parsed-cookies
+ for expires = (parameter-value "expires" parameters)
+ for domain = (or (parameter-value "domain" parameters) (uri-host uri))
+ when (and (valid-cookie-domain-p domain)
+ (cookie-domain-matches domain uri))
+ collect (make-instance 'cookie
+ :name name
+ :value value
+ :path (or (parameter-value "path" parameters)
+ (uri-path uri)
+ "/")
+ :expires (and expires
+ (plusp (length expires))
+ (parse-cookie-date expires))
+ :domain domain
+ :securep (not (not (parameter-present-p
"secure" parameters)))
+ :http-only-p (not (not (parameter-present-p
"HttpOnly" parameters)))))))
+
+(defun unique-cookies (cookies)
+ "Iterates through the given list of COOKIE objects, and returns a
+new list of COOKIE objects, with any duplicate cookies removed, using COOKIE=
+to determine duplicates. COOKIEs earlier in the list will be replaced by
+those later in the list.
+
+This is meant to deal with duplicate `Set-Cookie` headers from misbehaving
servers.
+For example, if the HTTP response contains:
+
+ Set-Cookie: JSESSIONID=foo; Path=/; Secure
+ Set-Cookie: JSESSIONID=bar; Path=/; Secure
+
+This function will only keep the last value for JSESSIONID Path=/; Secure
+
+The choice of which duplicate to use (first or last) is arbitrary, but last was
+chosen here to match the behavior of Firefox 3.6.
+"
+ (let (res (cookies (reverse cookies)))
+ (dolist (cookie cookies res)
+ (unless (find cookie res :test #'cookie=)
+ (push cookie res)))))
(defun update-cookies (new-cookies cookie-jar)
"Updates the cookies in COOKIE-JAR by replacing those which are
_______________________________________________
drakma-devel mailing list
drakma-devel@common-lisp.net
http://common-lisp.net/cgi-bin/mailman/listinfo/drakma-devel
