[mod_python] Can i have both Marshal and Signed CookieswithPublisher?

2006/10/30, Graham Dumpleton <grahamd at dscpl.com.au>:
> Clodoaldo Pinto Neto wrote ..
> > 2006/10/29, Graham Dumpleton <grahamd at dscpl.com.au>:
> > > Clodoaldo Pinto Neto wrote ..
> > The problem that is happening is that one exception was not expected
> > by the original code piece author. The exception is binascii.Error. It
> > will happen whenever MarshalCookie.parse() tries to parse a Signed
> > cookie. My proposed fix is just to import binascii and include the
> > exception:
> >
> >             try:
> >                 c.unmarshal(secret)
> >             except (CookieError, ValueError, binascii.Error):
> >                 # downgrade to Cookie
> >                 dict[k] = Cookie.parse(Cookie.__str__(c))[k]
> >
> > This way MarshalCookie.get_cookies() will behave as per the documentation.
>
> Yes and no. Yes one could do it, but after looking at it and thinking about
> it overnight I would probably do it in a different place. Instead, one would
> be better of first changing unmarshal() to:
>
>     def unmarshal(self, secret):
>
>         self.unsign(secret)
>
>         try:
>             data = base64.decodestring(self.value)
>         except binascii.Error:
>             raise CookieError, "Incorrectly Encoded Cookie: %s=%s" % (self.name, self.value)
>
>         self.value = marshal.loads(data)
>
> and leave parse() as is.
>
> The difference here is that you have split out the base64 decoding of
> the value string from the actual unmarshalling of the data.
>
> Next one has to contemplate whether one should for the marshal.loads()
> call separately catch EOFError, ValueError or TypeError exceptions as
> being indicative of badly formatted marshal data.
>
> The reason for doing this would be that I could use a SignedCookie at
> the same time where I decide to base64 encode the value myself. If I do
> this and the SignedCookie uses the same secret, the data would get
> past the signed check and the base64 decoding implemented by the
> MarshalCookie but then fail on unmarshalling if the data wasn't actually
> marshal data but some other format.
>
> So, to be even more precise:
>
>     def unmarshal(self, secret):
>
>         self.unsign(secret)
>
>         try:
>             data = base64.decodestring(self.value)
>         except:
>             raise CookieError, "Value Not BASE64 Encoded: %s=%s" % (self.name, self.value)
>
>         try:
>             self.value = marshal.loads(data)
>         except (EOFError, ValueError, TypeError):
>             raise CookieError, "Cannot Unmarshal Cookie: %s=%s" % (self.name, self.value)

I'm not sure i understand all the implications, but i think it is the
correct approach to the problem.

Did you forget to specify the binascii.Error exception or am i missing
something?:

>         try:
>             data = base64.decodestring(self.value)
>         except:
>             raise CookieError, "Value Not BASE64 Encoded: %s=%s" % (self.name, self.value)

> The other thing I believe also should be added is a new keyword
> parameter called 'strict'. This would default to 0 to preserve existing
> behaviour. So I could say:
>
>   cookies = Cookie.get_cookies(req, Cookie.MarshalCookie, names=['xxx'], strict=1)
>
> If 'strict' is set to a truth value, then if a cookie fails to be decoded as
> the intended type, it would not fallback to returning Cookie, but raise
> again the original exception that cause the cookie not to be decoded.
>
> The Session code would for example always use 'strict' to ensure that
> SignedCookie doesn't fallback to 'Cookie', thus ensuring the extra
> security of the 64 character vs 32 character string is used.

Sounds good. It would be still better if i could also tell
Cookie.get_cookies() to just discard the failed cookies in case i want
all and only the cookies of a certain class.

If i understand what you mean with the strict parameter then
MarshalCookie.parse() could be something like this:

    def parse(Class, s, secret, downgrade=False, strict=False):

        dict = _parse_cookie(s, Class)

        del_list = list()
        for k in dict:
            c = dict[k]
            try:
                c.unmarshal(secret)
            except (CookieError, ValueError):
               if downgrade:
                   del_list.append(k)
               else:
                   if strict:
                       raise
                   else:
                       # downgrade to Cookie
                       dict[k] = Cookie.parse(Cookie.__str__(c))[k]

        for k in del_list:
            del dict(k)
        return dict

Regards, Clodoaldo Pinto Neto