You are viewing a plain text version of this content. The canonical link for it is here.
Posted to mod_python-dev@quetz.apache.org by Craig Warren <cr...@encorp.com> on 2005/01/13 20:44:36 UTC

Re: [mod_python] Cookie patch

Nicolas Lehuen wrote:

>On Thu, 13 Jan 2005 09:23:03 -0700, Craig Warren
><cr...@encorp.com> wrote:
>  
>
>> I found an error while with Cookie module.  When the cookie module parses a
>>cookie, if that cooke has $Version or $Path in it you get an error. My
>>cookie is coming from a java libaray, that puts $Version and $Path in it.
>> example ="Cookie: $Version=0; pysid=34a9b38c34;$Path=/"
>>    
>> 
>> RFC 2109 mentions $Version and $Path in Section 4.4
>> http://www.faqs.org/rfcs/rfc2109.html 4.4 How an Origin Server Interprets
>>the Cookie Header A user agent returns much of the information in the
>>Set-Cookie header to the origin server when the Path attribute matches that
>>of a new request. When it receives a Cookie header, the origin server should
>>treat cookies with NAMEs whose prefix is $ specially, as an attribute for
>>the adjacent cookie. The value for such a NAME is to be interpreted as
>>applying to the lexically (left-to-right) most recent cookie whose name does
>>not have the $ prefix. If there is no previous cookie, the value applies to
>>the cookie mechanism as a whole. For example, consider the cookie Cookie:
>>$Version="1"; Customer="WILE_E_COYOTE"; $Path="/acme" $Version applies to
>>the cookie mechanism as a whole (and gives the version number for the cookie
>>mechanism). $Path is an attribute whose value (/acme) defines the Path
>>attribute that was used when the Customer cookie was defined in a Set-Cookie
>>response header. In Cookie.py it looks like the code was in place to deal
>>with $Version and $Path, but not finished
>> 
>> from  _parse_cookie()
>> line ~321
>>  l_key = key.lower()
>>         
>>         if (l_key in valid or key[0] == '$'):
>>             
>>             # "internal" attribute, add to cookie
>> 
>>             if l_key == "max-age":
>>                 l_key = "max_age"
>>             setattr(c, l_key, val)
>> 
>>  The above code checks for the $, but doesn't do anything with it and in
>>fact when it tries to do a setattr with $Version or $Path, you get an error.
>> 
>> I modified the function to be
>> 
>> l_key = key.lower()
>>         
>>         if (l_key in valid or key[0] == '$'):
>>             
>>             # "internal" attribute, add to cookie
>> 
>>             if l_key == "max-age":
>>                 l_key = "max_age"
>>             if key[0] == '$':
>>                 l_key = l_key[1:]
>>             setattr(c, l_key, val)
>> 
>> 
>> Don't know if this is exactly the correct fix, but it works for me and I
>>thought that I would email the list.  I tried to subscribe to
>>python-dev@httpd.apache.org, but haven't gotten a response back yet, I CC
>>this message to python-dev@httpd.apache.org also.
>> 
>> Craig Warren
>>    
>>
>
>Hi,
>
>Yeah, your fix seems correct.
>
>The Cookie class has a metaclass which defines slots, so you cannot
>have arbitrary attribute names. The only valid attributes names are
>defined on line 58 ( _valid_attr =...) of Cookie.py, and the version
>and path attributes do not begin with a $. So the attribute names
>which are parsed in _parse_cookie() must have their initial $ removed.
>
>The problem of the current code is that the _valid_attr check is
>useless, but it's not totally spec compliant since the valid attribute
>coming first (as the $Version attribute) should be applied to all
>cookies. So I rewrote the whole function like this :
>
>def _parse_cookie(str, Class):
>    # XXX problem is we should allow duplicate
>    # strings
>    result = {}
>
>    all_cookies_attribute = {}
>
>    valid = Cookie._valid_attr
>
>    c = None
>    matchIter = _cookiePattern.finditer(str)
>
>    for match in matchIter:
>
>        key, val = match.group("key"), match.group("val")
>
>        # we will check whether the cookie name is a valid attribute name
>        # for the previous cookie.
>        l_key = key.lower()
>        # fix from Craig Warren
>        if l_key[0]=='$':
>            l_key=l_key[1:]
>        if l_key == "max-age":
>            l_key = "max_age"
>        
>        if l_key in valid:
>            if not c:
>                # 'global' attribute, will be added to all cookies
>                all_cookies_attribute[l_key]=val
>            else:
>                # "internal" attribute, add to cookie
>                setattr(c, l_key, val)
>        else:
>            # start a new cookie
>            # we don't use l_key so that we keep the initial name
>            # this way we are consistent with the creation of the first cookie
>            # as done in the previous version of the function
>            c = Class(key, val)
>
>            # XXX this is a bit heavyweight since usually we'll have only 0 or 1
>            # global attribute...
>            for key, val in all_cookies_attribute.items():
>                setattr(c,key,val)
>
>            result[key] = c
>
>    return result
>
>Since you've already set up a testing environment, can you test this
>new version of the parsing function ? If it works, I'll check it in.
>
>Regards,
>Nicolas
>  
>
Not quite sure this is correct above, I have a hard time with the spec 
though..

But the problem I see is depending on the order of the value name pairs 
you can get different results.

The first time though the loop will always put the l_key in the 
all_cookies_attribute, then depending on the order, it will put the 
attributes of the all_cookies_attribute on the next cookies

[cwarren@lapbob mod_python]$ python cookie_test.py
TEST
 --------------------------------------------------
New Funtion
Cookie:test=1;bob=1;$Version=0;$Path=/
Cookies
test=1
bob=1; version=0; path=/
 
My function Old with my fix
Cookie:test=1;bob=1;$Version=0;$Path=/
Cookies
test=1
bob=1; version=0; path=/
 
 --------------------------------------------------
 
TEST
 --------------------------------------------------
New Funtion
Cookie:$Version=0;$Path=/;test=1
Cookies
test=1; version=0; path=/
 
My function Old with my fix
Cookie:$Version=0;$Path=/;test=1
Cookies
$Version=0; version=0; path=/
test=1
 
 --------------------------------------------------
 
TEST
 --------------------------------------------------
New Funtion
Cookie:test=1;$Version=0;bob=1;$Path=/
Cookies
test=1; version=0
bob=1; path=/
 
My function Old with my fix
Cookie:test=1;$Version=0;bob=1;$Path=/
Cookies
test=1; version=0
bob=1; path=/
 
 --------------------------------------------------
 
[cwarren@lapbob mod_python]$


Maybe only set the "global" attributes to all cookies if it has a "$" in 
front of it, so you would have to collect those the first time through 
the loop then add them afterwards to each Cookie.  But like I said I 
don't quite understand the spec very well.



Re: [mod_python] Cookie patch

Posted by Nicolas Lehuen <ni...@gmail.com>.
On Thu, 13 Jan 2005 12:44:36 -0700, Craig Warren
<cr...@encorp.com> wrote:
> Not quite sure this is correct above, I have a hard time
> with the spec though..
>  
>  But the problem I see is depending on the order of the value name pairs you
> can get different results.
>  
>  The first time though the loop will always put the l_key in the
> all_cookies_attribute, then depending on the order, it will put the
> attributes of the all_cookies_attribute on the next cookies
>  
>  [cwarren@lapbob mod_python]$ python cookie_test.py
>  TEST
>   --------------------------------------------------
>  New Funtion
>  Cookie:test=1;bob=1;$Version=0;$Path=/
>  Cookies
>  test=1
>  bob=1; version=0; path=/
>   
>  My function Old with my fix
>  Cookie:test=1;bob=1;$Version=0;$Path=/
>  Cookies
>  test=1
>  bob=1; version=0; path=/
>   
>   --------------------------------------------------
>   
>  TEST
>   --------------------------------------------------
>  New Funtion
>  Cookie:$Version=0;$Path=/;test=1
>  Cookies
>  test=1; version=0; path=/
>   
>  My function Old with my fix
>  Cookie:$Version=0;$Path=/;test=1
>  Cookies
>  $Version=0; version=0; path=/
>  test=1
>   
>   --------------------------------------------------
>   
>  TEST
>   --------------------------------------------------
>  New Funtion
>  Cookie:test=1;$Version=0;bob=1;$Path=/
>  Cookies
>  test=1; version=0
>  bob=1; path=/
>   
>  My function Old with my fix
>  Cookie:test=1;$Version=0;bob=1;$Path=/
>  Cookies
>  test=1; version=0
>  bob=1; path=/
>   
>   --------------------------------------------------
>   
>  [cwarren@lapbob mod_python]$
>  
>  
>  Maybe only set the "global" attributes to all cookies if it has a "$" in
> front of it, so you would have to collect those the first time through the
> loop then add them afterwards to each Cookie.  But like I said I don't quite
> understand the spec very well.
[...]

Well, the spec is pretty thin on the subject indeed, but what I
understand is that the result MUST depend on the order in which the
attributes are written. For example :

Cookie: $Version=1;test=2;$Path=/
==> a single Cookie with key=='test', value=='2' and attributes
version=='1' and path=='/'

Cookie: $Version=1;test1=2;$Path=/;test2=3;$Path=/foo
==> two Cookies :
 - one with key=='test1', value=='2', version=='1' and path=='/'
 - one with key=='test2', value=='3', version=='1' and path=='/foo'

Cookie: test1=2;$Path=/;$Version=1;test2=3;$Path=/foo;$Version=2
==> two Cookies :
 - one with key=='test1', value=='2', version=='1' and path=='/'
 - one with key=='test2', value=='3', version=='2' and path=='/foo'

Cookie: $Version=2;$Path='/fubar';test1=2;test2=3;$Path=/foo;$Version=1;test3=4;$Path=/bar;$Version=4
==> three Cookies :
 - one with key=='test1', value=='2', version=='2' and path=='/fubar'
 - one with key=='test2', value=='3', version=='1' and path=='/foo'
 - one with key=='test3', value=='4', version=='4' and path=='/bar'

To sum up, from what I understand, attributes are written like cookies
whose name start by a dollar sign, and they apply to the rightmost
preceding cookie.

Attributes that are at the beginning of the Cookie header are 'global'
attributes, used to describe the cookie 'baking' mechanism, like the
Version attributes does. Here I chose to put those attributes in each
Cookie, since I don't see any better way to return the information to
the API user (maybe in a Cookies collection ?).

I wrote a few tests and I checked that the new function does follow
this interpretation of the RFC. The problem is that I'm not 100% sure
that it's meant to be interpreted this way... I'll check in other
Cookies implementations (in Python or Java) to make sure.

Regards,
Nicolas