[drakma-devel] duplicate cookies problem with proposed patch

[Ryan Davis]

On 4/6/2010 5:59 AM, Edi Weitz wrote:
> 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?
>   
According to RFC2109 (HTTP State Management Mechanism
http://www.ietf.org/rfc/rfc2109.txt), section 4.2.2 "Set-Cookie Syntax"

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

So... no help there.  I'll send out a revised patch shortly.

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mailman.common-lisp.net/pipermail/drakma-devel/attachments/20100406/1880f050/attachment.html>