Skip to content

Make sure JSON string values come through consistently as unicode.#366

Merged
leplatrem merged 1 commit intoCornices:masterfrom
thruflo:json-value-encoding
Nov 25, 2016
Merged

Make sure JSON string values come through consistently as unicode.#366
leplatrem merged 1 commit intoCornices:masterfrom
thruflo:json-value-encoding

Conversation

@thruflo
Copy link
Contributor

@thruflo thruflo commented Aug 12, 2016

If you look at the current impl of util.extract_json_data it passes request.body directly to simplejson.loads.

Under PY2, when passed a string, loads has the documented behaviour of varying the type of returned string values depending on the content of the strings:

If s is a str then decoded JSON strings that contain only ASCII characters may be parsed as str for performance and memory reasons. If your code expects only unicode the appropriate solution is decode s to unicode prior to calling loads.

You can see the behaviour with e.g.:

>>> import simplejson
>>> simplejson.loads('{"foo":"ear"}')
{'foo': 'ear'}
>>> simplejson.loads('{"foo":"€ar"}')
{'foo': u'\u20acar'}
>>> simplejson.loads(u'{"foo":"ear"}')
{u'foo': u'ear'}
>>> simplejson.loads(u'{"foo":"€ar"}')
{u'foo': u'\u20acar'}

The commit attached follows the advice of "decode s to unicode prior to calling loads" (when running under PY2 and when request.body is a str).

@leplatrem
Copy link
Contributor

Thanks for this PR @thruflo ! The changes look good to me, as far as I understand. Let's ask feedback from our unicode champion @Natim :)

Also do you think you could rebase it to the new master please? (tests will be green)

Thanks again!

Copy link
Contributor

@Natim Natim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nitpick but it looks good to me thanks.

are unicode when running using PY2.
"""

def test_extracted_json_values(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of doing the if not util.PY2 can we use the skipIf decorator here?

@leplatrem leplatrem merged commit 02018a5 into Cornices:master Nov 25, 2016
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

Successfully merging this pull request may close these issues.

3 participants