[mod_python] Cookie patch

On Thu, 13 Jan 2005 09:23:03 -0700, Craig Warren
<craig.warren at 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 at httpd.apache.org, but haven't gotten a response back yet, I CC
> this message to python-dev at 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