[proposal] Create extension to Python Cookie module

[David Cramer]

Create an extension to the Python Cookie module to help solve issues with cookie key/value errors.

By subclassing the SimpleCookie module you can add support into it to try/except on the set method so invalid cookies get thrown away, but valid cookies are not lost.

File "/home/curseweb/cursedjango/django/django/utils/defensive.py", line 65, in inner_email_exceptions
   return func(*args, **kwargs)
 
 File "/home/curseweb/cursedjango/cursesite/middleware/cookies.py", line 13, in process_request
   for k, v in request.COOKIES.iteritems():
 
 File "/home/curseweb/cursedjango/django/django/core/handlers/modpython.py", line 83, in _get_cookies
   self._cookies = http.parse_cookie(self._req.headers_in.get('cookie', ''))
 
 File "/home/curseweb/cursedjango/django/django/http/__init__.py", line 160, in parse_cookie
   c.load(cookie)
 
 File "/usr/local/lib/python2.4/Cookie.py", line 621, in load
   self.__ParseString(rawdata)
 
 File "/usr/local/lib/python2.4/Cookie.py", line 652, in __ParseString
   self.__set(K, rval, cval)
 
 File "/usr/local/lib/python2.4/Cookie.py", line 574, in __set
   M.set(key, real_value, coded_value)
 
 File "/usr/local/lib/python2.4/Cookie.py", line 453, in set
   raise CookieError("Illegal key value: %s" % key)
 
CookieError: Illegal key value: ??est

[David Cramer]

Here is some code (credits to the guys at pocoo.org for the solution). This is just an example solution, thus no diff (as I'd have to install trunk to do the diff :P). If it's accepted I can create a patch.

# Code modifications credit to pocoo.org team
from Cookie import SimpleCookie, Morsel, CookieError

def parse_cookie(cookie):
    if cookie == '':
        return {}
    c = _ExtendedSimpleCookie()
    c.load(cookie)
    cookiedict = {}
    for key, value in c.iteritems():
        try:
            cookiedict[key] = value.value.decode('utf-8', 'ignore')
        except AttributeError:
            pass
    return cookiedict

class _ExtendedMorsel(Morsel):
    """
    Subclass of regular morsels for simpler usage and support of the
    nonstandard but useful http only header.
    """
    _reserved = {'httponly': 'HttpOnly'}
    _reserved.update(Morsel._reserved)

    def __init__(self, name=None, value=None):
        Morsel.__init__(self)
        if name is not None:
            self.set(name, value, value)

    def OutputString(self, attrs=None):
        httponly = self.pop('httponly', False)
        result = Morsel.OutputString(self, attrs).rstrip('\t ;')
        if httponly:
            result += '; HttpOnly'
        return result

    def set(self, *args, **kwargs):
        try:
            Morsel.set(self, *args, **kwargs)
        except CookieError:
            pass

class _ExtendedSimpleCookie(SimpleCookie):

    def _BaseCookie__set(self, key, real_value, coded_value):
        morsel = self.get(key, _ExtendedMorsel())
        morsel.set(key, real_value, coded_value)
        dict.__setitem__(self, key, morsel)

[pytechd <pytechd@…>]

There's a ticket already for HttpOnly, #3304. This patch looks nicer.

[Malcolm Tredinnick]

This is probably a someday/maybe feature. However, in the sample code given, trying to decode the cookie value as UTF-8 (or with any encoding) isn't correct. Cookie data should be treated as opaque, because there's no uniformly respected standard for encoding there. It's up to the client app to make sure that ASCII data is sent out and then decode it as appropriate.

[QingFeng]

My "parse_cookie" patch

[QingFeng]

parse_cookie unit test

[QingFeng]

Unit Test:

from django.test import TestCase
from django import http

class SimpleTest(TestCase):
    def test_basic_addition(self):
        cookie_str = 'name=qingfeng;???=456;'#Error Cookie!
        cookie = http.parse_cookie(cookie_str)
        self.assertTrue('name' in cookie)
        self.assertEqual(cookie['name'],'qingfeng')

Error Log:

F
======================================================================
FAIL: test_basic_addition (fixbug.tests.SimpleTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/private/tmp/cookierror/fixbug/tests.py", line 16, in test_basic_addition
    self.assertTrue('name' in cookie)
AssertionError

----------------------------------------------------------------------
Ran 1 test in 0.003s

[QingFeng]

This bug will affect mod_python and wsgi reading session, because "cookie.load (str)" error, all the cookie will be cleared

[Ramiro Morales]

The issues described in this ticket Description field and comments were fixed in r14707 and r15523.