Opened 16 years ago

Closed 13 years ago

#6507 closed (fixed)

[proposal] Create extension to Python Cookie module

Reported by: David Cramer Owned by: nobody
Component: HTTP handling Version: dev
Severity: Keywords:
Cc: qingfeng@… Triage Stage: Someday/Maybe
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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

Attachments (3)

__init__.patch (545 bytes ) - added by QingFeng 15 years ago.
django_http_init_11315.patch (588 bytes ) - added by QingFeng 15 years ago.
tests.py (551 bytes ) - added by QingFeng 15 years ago.
parse_cookie unit test

Download all attachments as: .zip

Change History (12)

comment:1 by David Cramer, 16 years ago

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)

comment:2 by pytechd <pytechd@…>, 16 years ago

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

comment:3 by Malcolm Tredinnick, 16 years ago

Triage Stage: UnreviewedSomeday/Maybe

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.

comment:4 by QingFeng, 15 years ago

Cc: qingfeng@… added
Has patch: set

My "parse_cookie" patch

by QingFeng, 15 years ago

Attachment: __init__.patch added

by QingFeng, 15 years ago

by QingFeng, 15 years ago

Attachment: tests.py added

parse_cookie unit test

comment:5 by QingFeng, 15 years ago

Needs tests: set

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

comment:6 by QingFeng, 15 years ago

milestone: 1.2

comment:7 by QingFeng, 15 years ago

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

comment:8 by Russell Keith-Magee, 14 years ago

milestone: 1.2

comment:9 by Ramiro Morales, 13 years ago

Resolution: fixed
Status: newclosed

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

Note: See TracTickets for help on using tickets.
Back to Top