pallets / werkzeug

The comprehensive WSGI web application library.
https://werkzeug.palletsprojects.com
BSD 3-Clause "New" or "Revised" License
6.66k stars 1.73k forks source link

Response.set_cookie no longer accepts integer values #436

Closed florentx closed 10 years ago

florentx commented 11 years ago

We have a regression when moving from Werkzeug 0.8 to 0.9, because the method Response.set_cookie no longer accepts integer values.

AFAICT, this change of behavior is not documented.

openerp.addons.web.common.http.HttpRequest.dispatch: An error occurred while handling a json request
Traceback (most recent call last):
  File "./addons/web/common/http.py", line 260, in dispatch
    r = method(controller, self, **self.params)
  File "./addons/web/controllers/main.py", line 1876, in index
    cookies={'fileToken': int(token)})
  File "./addons/web/common/http.py", line 307, in make_response
    response.set_cookie(k, v)
  File "./Werkzeug-0.9.4-py2.6.egg/werkzeug/wrappers.py", line 992, in set_cookie
    self.charset))
  File "./Werkzeug-0.9.4-py2.6.egg/werkzeug/http.py", line 905, in dump_cookie
    value = to_bytes(value, charset)
  File "./Werkzeug-0.9.4-py2.6.egg/werkzeug/_compat.py", line 106, in to_bytes
    raise TypeError('Expected bytes')
TypeError: Expected bytes
untitaker commented 11 years ago

I guess you would expect it to convert to string silently? Btw, this is a change that was "accidentally" added because of the Python 3 port, where typing-related problems forced us to be strict about the types accepted by to_bytes.

cecton commented 11 years ago

This does not look consistent:

On the Python 2 code, str is a subclass of bytes and therefore x is casted to bytes:

    def to_bytes(x, charset=sys.getdefaultencoding(), errors='strict'):
        if x is None:
            return None
        if isinstance(x, (bytes, bytearray, buffer)):
            return bytes(x)
        if isinstance(x, unicode):
            return x.encode(charset, errors)
        raise TypeError('Expected bytes')

On the Python 3 code, str is not a subclass of bytes anymore but is encoded to the charset:

    def to_bytes(x, charset=sys.getdefaultencoding(), errors='strict'):
        if x is None:
            return None
        if isinstance(x, (bytes, bytearray, memoryview)):
            return bytes(x)
        if isinstance(x, str):
            return x.encode(charset, errors)
        raise TypeError('Expected bytes')

Is it what you really wanted to do?

ghost commented 11 years ago

I think this is not a regression but a progression: this should never have been working with integers. Seriously, the current behavior is much more pythonic.

@cecton OT: AFAIK, there is no difference in Python 2 between str and bytes, so there's no casting. to_bytes returns a string. In Python 3 instead a str contains Unicode data, which is different from bytes, so there's casting. to_bytes returns... bytes. Working perfectly.

cecton commented 11 years ago

there is no difference in Python 2 between str and bytes, so there's no casting. to_bytes returns a string. In Python 3 instead a str contains Unicode data, which is different from bytes, so there's casting. to_bytes returns... bytes. Working perfectly.

Right! Didn't know that. Thanks for the information.

Now it sounds great to me.

(And yes it makes sense that x should be some kind of string)

untitaker commented 10 years ago

@cecton Should this be closed then?

cecton commented 10 years ago

@untitaker Well... tbh I don't remember that I wrote something here (>_<) What about you @florentx (ticket opener)?

untitaker commented 10 years ago

Closing this for the exact reasons @ghost stated: It was never intended to work that way.