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

2006/10/29, Graham Dumpleton <grahamd at dscpl.com.au>:
> Clodoaldo Pinto Neto wrote ..
> > 2006/10/29, Graham Dumpleton <grahamd at dscpl.com.au>:
> > IMHO, the least surprise behavior is if Cookie.get_cookies() returned
> > all cookies of the given class regardless of how many different cookie
> > classes there are in the header.
>
> There is nothing in a cookie string though to be able to identify it as being
> of a particular type. The only way you can know is to try and decode it and
> if it works then it probably is.

It is exactly what MarshalCookie.parse() does:

    def parse(Class, s, secret):

        dict = _parse_cookie(s, Class)

        for k in dict:
            c = dict[k]
            try:
                c.unmarshal(secret)
            except (CookieError, ValueError):
                # downgrade to Cookie
                dict[k] = Cookie.parse(Cookie.__str__(c))[k]

        return dict

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.

> This is probably not a good way of doing
> things. First off the application should only be decoding its own cookies
> and not others which may have been sent to the site in general. Thus, allowing
> one to say which cookies to decode is probably a better step.

I'm not opposing the new names argument. I just don't know enough
about mod_python to opine on it, although the names argument alone
will not fix the issue: an *expected* error that should be handled by
mod_python. Or better, the names argument will only fix the issue if
it is passed *and* if none of the passed cookie names is a Signed
cookie. A nice addition perhaps, but not a fix.

> > What is the point in downgrading the cookie? If it is tampered or
> > corrupted then why not just discard it?
>
> This issue has come up recently in relation to signed Sessions. For
> Sessions at least it wasn't a big problem but couldn't get any feedback
> on issue in general so nothing done. See:
>
>   http://issues.apache.org/jira/browse/MODPYTHON-191
>
> For how the implementation works, the documentation certainly does
> not say that the type of the cookie should be checked, when it should.
>
> To affect some change, some sort of consensus is needed about how
> to change it and for there to be a good understanding that this will
> not cause problems with existing code. Remember this code was
> written many years ago (not by I) and so one always has to be sure
> that changes will not break things.
>
> So, please do contribute further feedback and ideas, but when it comes
> down to it, unless its obvious a change should be made in a certain
> way, if only one person is pushing it, I am not always going to do it.
> I would prefer a few people at least to agree, especially when I don't
> use a lot of this stuff personally and so don't always know what should
> be done. Some times I do make an arbitrary decision to make a change
> when I get no feedback, but one of these days it is going to surely
> bite me.

You are absolutely right about it. That was just my opinion, and i
didn't invest the minimum amount of time on it.

Regards, Clodoaldo Pinto Neto