Code

Opened 6 years ago

Closed 3 years ago

#6507 closed (fixed)

[proposal] Create extension to Python Cookie module

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

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 5 years ago.
django_http_init_11315.patch (588 bytes) - added by qingfeng 5 years ago.
tests.py (551 bytes) - added by qingfeng 5 years ago.
parse_cookie unit test

Download all attachments as: .zip

Change History (12)

comment:1 Changed 6 years ago by dcramer

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 Changed 6 years ago by pytechd <pytechd@…>

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

comment:3 Changed 6 years ago by mtredinnick

  • Triage Stage changed from Unreviewed to Someday/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 Changed 5 years ago by qingfeng

  • Cc qingfeng@… added
  • Has patch set

My "parse_cookie" patch

Changed 5 years ago by qingfeng

Changed 5 years ago by qingfeng

Changed 5 years ago by qingfeng

parse_cookie unit test

comment:5 Changed 5 years ago by qingfeng

  • 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 Changed 5 years ago by qingfeng

  • milestone set to 1.2

comment:7 Changed 5 years ago by qingfeng

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

comment:8 Changed 4 years ago by russellm

  • milestone 1.2 deleted

comment:9 Changed 3 years ago by ramiro

  • Resolution set to fixed
  • Status changed from new to closed

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.