Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SecureCookie: serialization issues #287

Closed
ivan-kleshnin opened this issue Apr 25, 2013 · 12 comments
Closed

SecureCookie: serialization issues #287

ivan-kleshnin opened this issue Apr 25, 2013 · 12 comments

Comments

@ivan-kleshnin
Copy link

import json
from werkzeug.datastructures import MultiDict
from werkzeug.contrib.securecookie import SecureCookie
  1. MultiDicts produce inconsinstent behavior:
cookie1 = SecureCookie(
     MultiDict([('multikey', u'x'), ('multikey', u'xx'), ('monokey', 'y')]), secret_key='xxx'
)
cookie2 = SecureCookie(
    {'data': MultiDict([('multikey', u'x'), ('multikey', u'xx'), ('monokey', 'y')])}, secret_key='xxx'
)

print SecureCookie.unserialize(cookie1.serialize(), secret_key='xxx') # compare this
print SecureCookie.unserialize(cookie2.serialize(), secret_key='xxx') # to this!
  1. With json method serialization it gets even worse:
class SecureCookie(SecureCookie):
        serialization_method = json

cookie1 = ...
cookie2 =  ...

print SecureCookie.unserialize(cookie1.serialize(), secret_key='xxx') # this is ok
print SecureCookie.unserialize(cookie2.serialize(), secret_key='xxx') # we lost data here!!!

P.S.
MultiDict provokes plenty of problems around. Especially with 3-rd party libs. At least, it causes dependency breeding.
To be fair, I believe that adding all those XxxMultiDict was design mistake and I would consider total removal.

@untitaker
Copy link
Contributor

I just ran your first example and it seems to run fine. What is your output?

$ python -i werkzeug/datastructures.py
>>> from werkzeug.contrib.securecookie import SecureCookie
>>> c1 = SecureCookie(
...     MultiDict([('multikey', u'x'), ('multikey', u'xx'), ('monokey', 'y')]), secret_key='xxx'
... )
>>> c1
<SecureCookie {'multikey': [u'x', u'xx'], 'monokey': ['y']}>
>>> c2 = SecureCookie(
...     {'data': MultiDict([('multikey', u'x'), ('multikey', u'xx'), ('monokey', 'y')])}, secret_key='xxx'
... )
>>> c2
<SecureCookie {'data': MultiDict([('multikey', u'x'), ('multikey', u'xx'), ('monokey', 'y')])}>
>>> SecureCookie.unserialize(c1.serialize(), secret_key='xxx')
<SecureCookie {'multikey': [u'x', u'xx'], 'monokey': ['y']}>
>>> SecureCookie.unserialize(c2.serialize(), secret_key='xxx')
<SecureCookie {'data': MultiDict([('multikey', u'x'), ('multikey', u'xx'), ('monokey', 'y')])}>
>>>

@ivan-kleshnin
Copy link
Author

Example 1.

>>> c1
<SecureCookie {'multikey': [u'x', u'xx'], 'monokey': ['y']}>

I.e. list instead of MultiDict.

Example 2.

>>> c2
<SecureCookie {'data': {u'multikey': u'x', u'monokey': u'y'}}>

I.e. only first value of multikey remained.

In your Example 2. you've obtain diverse output (list vs MultiDict) as well.

I wonder why default behavior of somemultidict[somekey] is returning the first value only.
For backward compatibility, perhaps, but it seems like it brings more problem than prevents.
The basic behavior of 3-rd party lib is to call __getitem__ and .items() in your dict. Therefore we lose data, (some of the keys from HTTP POST) with MultiDict default behavior. items() wait for multi=True flag too which never comes.

As I've said, i see no preference of MultiDict over simple dicts with list values as it done in other languages like PHP.
Ruby also uses old convention x=y&x=z (in URL) instead of new x[]=y&x[]=z.
Does it provide data in MultiDict-like wrappers?

My version of Werkzeug is the last that installs with pip.

@untitaker
Copy link
Contributor

>> c1
<SecureCookie {'multikey': [u'x', u'xx'], 'monokey': ['y']}>

That does make sense to me, since SecureCookie implements actually a dict interface and therefore has to convert the MultiDict's keys into a normal dict first.

>> c2
<SecureCookie {'data': {u'multikey': u'x', u'monokey': u'y'}}>

That is different from mine, i guess that has already been fixed between the latest release and the current HEAD.

I wonder why default behavior of somemultidict[somekey] is returning the first value only.

It seems sensible to me that getting a value via MultiDict.getitem actually returns an item (as the method name implies) and not a list of those.

Which third-party library are you talking about? If it expects a normal dictionary, you shouldn't be surprised that passing MultiDict isn't going to end that well.

@untitaker
Copy link
Contributor

Also:

1.) Referencing to PHP is not such a good idea
2.) Which Python version?

@ivan-kleshnin
Copy link
Author

  1. Referencing to PHP is not such a good idea

I refer PHP for only one single reason: it uses notably better (newer) convention for serialization which is strongly connected to the bug we discuss. http://api.jquery.com/jQuery.param/#jQuery-param-obj-traditional

Which Python version?
2) 2.7.3

@untitaker
Copy link
Contributor

You did read my comment about your output? Does the original issue still exist with an installation from master?

@ivan-kleshnin
Copy link
Author

Yes. The same output with development branch.

@untitaker
Copy link
Contributor

Can you write a test case that fails for you? I am not sure atm if we are talking about c1 or c2/the encoded-and-decoded cookie or the normal cookie that doesn't behave as it should.

@untitaker
Copy link
Contributor

Also i think we are talking past each other, so here goes:

>>> cookie1 = SecureCookie(
...     MultiDict([('multikey', u'x'), ('multikey', u'xx'), ('monokey', 'y')]), secret_key='xxx'
...)
>> cookie1
<SecureCookie {'multikey': [u'x', u'xx'], 'monokey': ['y']}>  # reasonable, as SecureCookie has to transform the values of MultiDict into a normal dict (SecureCookie is a dict itself)
>>> cookie2 = SecureCookie(
...    {'data': MultiDict([('multikey', u'x'), ('multikey', u'xx'), ('monokey', 'y')])}, secret_key='xxx'
... )
>>> cookie2
<SecureCookie {'data': {u'multikey': u'x', u'monokey': u'y'}}>  # you said this behavior exists in the dev branch?
>>> SecureCookie.unserialize(cookie1.serialize(), secret_key='xxx') # post the output for this
# ???
>>> SecureCookie.unserialize(cookie2.serialize(), secret_key='xxx') # and this
# ???

@ivan-kleshnin
Copy link
Author

  1. If you find type fluctuations reasonable we are going to talk about "data loss" issue only.
  2. The object (python) data is always correct.
  3. "Data loss" issue arises only after unserialization.
  4. "Data loss" issue arises only with json serialization method.

Updated example:

import json
import simplejson
import werkzeug
from werkzeug.datastructures import MultiDict
from werkzeug.contrib.securecookie import SecureCookie

class SecureCookie(SecureCookie):
    serialization_method = json
    # serialization_method = simplejson — same result :(

>>> werkzeug.__version__
'0.9-dev'

>>> cookie = SecureCookie(
...     {'data': MultiDict([('multikey', u'x'), ('multikey', u'xx'), ('monokey', 'y')])}, secret_key='xxx'
... )

>>> cookie
<SecureCookie {'data': MultiDict([('multikey', u'x'), ('multikey', u'xx'), ('monokey', 'y')])}>

>>> SecureCookie.unserialize(cookie.serialize(), secret_key='xxx')
<SecureCookie {'data': {u'multikey': u'x', u'monokey': u'y'}}>

@mitsuhiko
Copy link
Contributor

As bad as it sounds: works as intended. MultiDict's do not work with JSON.

@ivan-kleshnin
Copy link
Author

See #389

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants