[Django] #27895: Test Client fails to decode json response with unicode characters
Django
---------------------------------------------+------------------------
Reporter: Aniruddha Maru | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+------------------------
Since `application/json` response ought to be encoded with `utf-8` charset
per spec [https://tools.ietf.org/html/rfc7159#page-11 RFC 7159], they
should be decoded as such.
The problematic line is
[https://github.com/django/django/blob/master/django/test/client.py#L682
here]:
{{{#!python
response._json = json.loads(response.content.decode(),
**extra)
}}}
Which ought to be changed to:
{{{#!python
response._json = json.loads(response.content.decode('utf-8'),
**extra)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27895>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Aniruddha Maru | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: 1.10
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Aniruddha Maru):
* Attachment "patch.diff" added.
patch.diff
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Type: Bug | Status: assigned
Component: Testing framework | Version: 1.10
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Aniruddha Maru):
* owner: nobody => Aniruddha Maru
* status: new => assigned
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:1>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Type: Bug | Status: assigned
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aniruddha Maru):
This is only relevant for Py2.
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:2>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Type: Bug | Status: assigned
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Aniruddha Maru):
* Attachment "patch.diff" added.
patch.diff
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Type: Bug | Status: assigned
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Aniruddha Maru):
* Attachment "patch_stable:1.10.x.diff" added.
patch_stable/1.10.x.diff
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Type: Bug | Status: assigned
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz):
I think Django makes the assumption that when Unicode characters are sent
JSON-encoded on the wire, those characters are escaped (`\\u00e9`).
Is that a wrong assumption?
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:3>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Type: Bug | Status: assigned
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
Claude, could you provide an explanation of why Django would make that
assumption? I'm not knowledgeable to confirm or reject it.
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:4>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Type: Bug | Status: assigned
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz):
Tim, I'm not talking about master or Python 3, but with Python 2, calling
`decode() ` without specifying an encoding will default to 'ascii'
decoding.
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:5>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Component: Testing framework | Version: 1.10
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: assigned => closed
* resolution: => fixed
Comment:
Since `response.json` is new in 1.9, even if this is valid, it doesn't
qualify for a backport at this time. Marking as "fixed" since Django's
master branch only supports Python 3.
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:6>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Component: Testing framework | Version: 1.10
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* status: closed => new
* resolution: fixed =>
* stage: Unreviewed => Accepted
Comment:
I do not totally agree. If this is recognized as a bug, as 1.11 still
supports Python 2 and considering we are still in beta phase, this should
be fixed for 1.11.
The proposed patch is simple and has a test. I'm available to create the
PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:7>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Component: Testing framework | Version: 1.10
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
The guideline for the post-beta period is only to backport release
blocking bugs (if this is considered a blocker, it would normally also be
fixed in 1.10) but I'm fine with whatever you want to do here.
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:8>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Component: Testing framework | Version: 1.10
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
Aniruddha, could you gave the use case of using `ensure_ascii=False` in
the `JsonResponse`?
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:9>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Component: Testing framework | Version: 1.10
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aniruddha Maru):
Tim, it's really straightforward: we have browser based API documentation,
and unicode data. Since json supports utf-8 encoding, it's pretty standard
to always return unicode, rather than have an encode-decode cycle.
Here's some more context of other people who've talked about this bug:
https://github.com/tomchristie/django-rest-framework/issues/2891
https://github.com/tomchristie/django-rest-
framework/issues/2891#issuecomment-178932902
Django rest framework also offers a flag to disable this behavior:
http://www.django-rest-framework.org/api-guide/settings/#unicode_json,
which is enabled by default.
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:10>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Component: Testing framework | Version: 1.10
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
Okay, thanks. Can you create the pull request against stable/1.11.x? I
think you could modify the existing json view to return some non-ASCII
content rather than adding another view.
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:11>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Component: Testing framework | Version: 1.10
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* cc: Claude Paroz (added)
Comment:
Claude, do you plan to address this for 1.11?
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:12>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Component: Testing framework | Version: 1.10
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: new => closed
* resolution: => fixed
Comment:
Reclosing. A pull request was never provided for 1.11.
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:13>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Component: Testing framework | Version: 1.10
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Fabio Bonelli):
* cc: Fabio Bonelli (added)
* status: closed => new
* resolution: fixed =>
Comment:
Pull request for 1.11: https://github.com/django/django/pull/9061
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:14>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Component: Testing framework | Version: 1.10
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: new => closed
* resolution: => wontfix
Comment:
As this isn't a regression or a bug in a new feature in 1.11, it doesn't
qualify for a backport to stable/1.11.x. See our
[https://docs.djangoproject.com/en/dev/internals/release-process
/#supported-versions supported versions policy].
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:15>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Type: Bug | Status: closed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Fabio Bonelli):
You gave somewhat of a mixed signal with comment:11.
Anyway, this bug report has still a patch attached for master. Would you
accept a PR against that branch?
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:16>
Django
Reporter: Aniruddha Maru | Owner: Aniruddha
| Maru
Type: Bug | Status: closed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
Sorry for the confusion. 6 months ago when I made that comment, 1.11
wasn't released so less critical bug fixes were more acceptable.
Master doesn't support Python 2 so as far as I understand, this issue
doesn't affect it.
--
Ticket URL: <https://code.djangoproject.com/ticket/27895#comment:17>