Hi Ryan,
Thanks for the patch which is kind of OK but I think can be improved. I wouldn't worry too much about performance (we're talking about very short lists here), but there's no reason to write a function. Just use REMOVE-DUPLICATES, or even DELETE-DUPLICATES.
I also like the idea of giving the user a choice which could be controlled by a global special variable (and see the :FROM-END keyword argument to REMOVE-DUPLICATES). I'd probably call the variable *REMOVE-DUPLICATE-COOKIES-P* with the possible values NIL, T, :KEEP-LAST, :KEEP-FIRST where T would mean the same as :KEEP-LAST. In this case you'd have to export the name of the variable and add it to the HTML documentation.
I'm kind of unsure, though, what the best default behavior should be. Do it like Firefox does it? What do others think?
Thanks, Edi.
On Sat, Apr 3, 2010 at 12:20 AM, Ryan Davis ryan@acceleration.net wrote:
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:
Add a exported *redundant-cookie-strategy* variable, with values :keep-all, :keep-first, and :keep-last, defaulting to :keep-all (the current behavior). remove the unique-cookies function and rewrite get-cookies to implement the *redundant-cookie-strategy*, making one pass through parsed-cookies. 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-devel mailing list drakma-devel@common-lisp.net http://common-lisp.net/cgi-bin/mailman/listinfo/drakma-devel