[drakma-devel] duplicate cookies problem with proposed patch

[Edi Weitz]

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 at 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 at common-lisp.net
> http://common-lisp.net/cgi-bin/mailman/listinfo/drakma-devel
>
>