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?
  

... If an attribute appears more than once in a cookie, the behavior is undefined.

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


    

_______________________________________________
drakma-devel mailing list
drakma-devel@common-lisp.net
http://common-lisp.net/cgi-bin/mailman/listinfo/drakma-devel