[mod_python] Cookie patch

On Thu, 13 Jan 2005 18:48:08 +0100, Nicolas Lehuen
<nicolas.lehuen at gmail.com> wrote:
> 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
> 

Woops sorry, I found a bug *before* the test... The iteration on
all_cookies_attribute at the end of the method will overwrite the key
value. So the new version is really :

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)
            result[key] = c

            # 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)

    return result