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

Default charset does not apply to JSON responses #298

Closed
href opened this issue Nov 21, 2016 · 6 comments
Closed

Default charset does not apply to JSON responses #298

href opened this issue Nov 21, 2016 · 6 comments

Comments

@href
Copy link

href commented Nov 21, 2016

I've tested WebOb 1.7rc1 with Morepath and I've noticed that the default UTF-8 charset of the response only applies to text responses:

>>> from webob import Response
>>> Response("{}", content_type="text/plain")
<Response at 0x100f188d0 200 OK>
>>> Response("{}", content_type="application/json")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/denis/.virtualenvs/morepath/lib/python3.5/site-packages/webob/response.py", line 311, in __init__
    "You cannot set the body to a text value without a "
TypeError: You cannot set the body to a text value without a charset

Now I have no problem setting the charset of JSON responses explicitly (and as of today Morepath does), but I was wondering if this might be a bug.

I would have expected that the JSON response also uses the default charset defined on the Response class.

@ztane
Copy link

ztane commented Nov 21, 2016

#196 and #197

@mmerickel
Copy link
Member

It appears that default_body_encoding is not being used in the __init__. This is probably a bug. If you were doing response.text = "{}" things should work fine.

As far as the charset, you can see the linked issues from @ztane to understand that 1.7 explicitly changes this behavior in webob with regard to application/json.

@digitalresistor
Copy link
Member

default_body_encoding is not used in in Response() on purpose. This is not a bug. Passing a body that is "text" is not supported if the Content-Type doesn't have a charset. Pass it as text instead

Like so:

>>> res = Response(text='{}', content_type='application/json') 
>>> res.body
b'{}'

When passing JSON, I would recommend:

>>> res = Response(json_body={}, content_type='application/json')
>>> res.body
b'{}'

Or

>>> res = Response(json_body={})
>>> res.content_type
'application/json'
>>> res.body
b'{}'

@digitalresistor
Copy link
Member

digitalresistor commented Nov 21, 2016

Response("{}", content_type="text/plain")

Automatically adds the default charset:

>>> res = Response("{}", content_type="text/plain")
>>> res.headers['Content-Type']
'text/plain; charset=UTF-8'

Which allows you to set body to a "text" object to not break what has been allowed since WebOb 1.1. However I still fully believe that passing a body that is not a bytes object is something that might cause more issues than it is worth and it was disallowed explicitly on Content-Types that don't have a charset... so that things like this don't accidentally cause breakage:

Response('some text', content_type='image/jpeg')

I'd rather that raise an error than silently setting the body.


WebOb 1.6.x treats application/json as special, that special casing has been removed and it is now treated like all other non-text Content-Type's.

This meant there was inconsistent handling, for example:

>>> res = Response('{}', content_type='application/json')
>>> res.headers['Content-Type']
'application/json'
>>> res.text = '{}'
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/~/python3.5/site-packages/webob/response.py", line 427, in _text__set
    "You cannot access Response.text unless charset is set")
AttributeError: You cannot access Response.text unless charset is set
>>>
>>> res = Response('test', content_type='image/jpeg')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/~/python3.5/site-packages/webob/response.py", line 139, in __init__
    "You cannot set the body to a text value without a "
TypeError: You cannot set the body to a text value without a charset

@href
Copy link
Author

href commented Nov 21, 2016

Ok I understand. Thanks for clearing this up for me - and sorry for missing the existing issues.

@href href closed this as completed Nov 21, 2016
@digitalresistor
Copy link
Member

I've documented this better in: http://docs.webob.org/en/1.7-branch/whatsnew-1.7.html#backwards-incompatibility as well documenting it in http://docs.webob.org/en/1.7-branch/api/response.html#webob.response.Response

Hopefully this will help other users that may run into the same issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants