Opened 5 years ago

Closed 3 years ago

#17942 closed New feature (fixed)

JSONResponse class for API responses

Reported by: Leah Culver Owned by: Łukasz Balcerzak
Component: HTTP handling Version: master
Severity: Normal Keywords: dceu13
Cc: noah@…, eromijn@…, tom@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no


To make APIs, many developers have created their own JSONResponse classes which are subclasses of Response but returning JSON and with the proper mimetype ("application/json").

Some examples:

Related to this wiki page:

This patch should include:

  • mimetype "application/json"

Optional ideas:

Change History (15)

comment:1 Changed 5 years ago by Leah Culver

Needs documentation: set
Needs tests: set
Patch needs improvement: unset

comment:2 Changed 5 years ago by Leah Culver

Version: 1.3

comment:3 Changed 5 years ago by coderanger

Cc: noah@… added

Additional notes: should include ensure_ascii=False in the dumps call. Should use DjangoJSONEncoder by default to get datetime/decimal serialization.

comment:4 Changed 4 years ago by Jannis Leidel

Component: UncategorizedHTTP handling
Triage Stage: UnreviewedAccepted

Accepting in general as it seems to be a commonly used feature, indeed. About the XML response class I'm a bit reluctant though, as this smells like trouble.

comment:5 Changed 3 years ago by Łukasz Balcerzak

Owner: changed from nobody to Łukasz Balcerzak
Status: newassigned

comment:6 Changed 3 years ago by Łukasz Balcerzak

Has patch: set
Needs documentation: unset
Needs tests: unset
Version: master

Added pull request:

Implementation motiviations:

  1. Trying to support JSONP callbacks would require to somehow allow Cross-origin resource sharing. I would say it's not needed for most use cases.
  2. If extra status codes are needed they can be easily created using status parameter/status_code attribute. It is also not clear if JsonResponse users would want to return json responses on errors (most probably but I guess not always).
  3. Idea of allowing to serialize models/querysets is not bad and this point is actually I am most interested to get feedback on.
  4. I would leave XML for now. Especially as this ticket is specifically about JSON.

Please let me know your thoughts.

comment:7 Changed 3 years ago by Erik Romijn

Cc: eromijn@… added
Keywords: dceu13 added
Patch needs improvement: set

I haven't done a very thorough review right now, but I do notice a few issues:

  • Is the adding of JSON_RESPONSE_DEFAULT_ENCODER really essential? In general, we try not to add new settings unless it is really needed. For this configuration, which can also be done when creating each JSONResponse, I would not add a global setting at this time. We can always add it later, but if we add it now it will be very difficult to remove it at a later time.
  • I don't entirely understand JSON_RESPONSE_ALLOW_DICTS_ONLY. For who is there an attack vector against who when Array() is poisoned? In other words, what risk/breakage is introduced by setting this to False?
  • I'm not entirely convinced that in _default_content_type, the underscore is appropriate. Although I see the point from Python conventions, I rarely see the single underscore prefix in Django, so perhaps there is some intention not to use this in Django.
  • The documentation text doesn't seem to flow very well. It's difficult for me to point out exactly what my issue with it is - I'll happily modify it for you, but I'm not able to do that right now.
  • As far as I know, we have just adopted the plan to make all code samples in Django include their import path. I am not 100% sure of this. If this is the case, it would be good to add them to this patch too.

comment:8 Changed 3 years ago by Łukasz Balcerzak

  • The reason I added JSON_RESPONSE_DEFAULT_ENCODER is that if there is no possibility to set it globally, user would most probably create own subclass of JsonResponse if need to use another encode in most cases (and we are adding this class so user won't have to create their own). I can remove it if someone feels like it's not necessary (or we can mark the ticket for design decision request). This was in fact a setting I was not really sure if is 100% needed but would be nice for users that in example would create own encoder that can consume model or queryset.
  • As for JSON_RESPONSE_ALLOW_DICTS_ONLY: security flaw would be created if i.e. some view is CSRF vulnerable and returns top-level Array object, and user uses pre-EcmaScript5 compliant browser. Attacker could prepare malicious page with a request to that page. Normally, attacker would not be able to retrieve data from such request but with patched Array it is possible. See to get more information and quite precise example.
  • single prefixed names are used through whole HttpResponse constructor. I can make it public but then it becomes part of the interface and changing it would be more difficult in future
  • yep, please update documentation if you can!
  • am not aware of that. Can you point me to the related mails/discussions/ticket or tell that with 100% certainty? In addition - if I would change newly added code snippet now styles would be mixed. I believe this is responsibility of the one who would need to merge changes and find them conflicting with master branch.

comment:9 Changed 3 years ago by Simon Charette

Needs tests: set

As discussed on the PR thread.

  • The class should be named JSONResponse.
  • The _default_content_type attribute should be replaced by kwargs.setdefault('content_type', 'application/json').
  • The JSON_RESPONSE_DEFAULT_ENCODER and JSON_RESPONSE_ALLOW_DICTS_ONLY settings should not be introduced and kwargs should be used instead.
  • The encoder should just default to None, it should be really easy to specify an encoder class without subclassing.
  • Why are you setting ensure_ascii to False? I've never had any issues dealing with non-ascii escaped data?

comment:10 Changed 3 years ago by Łukasz Balcerzak

  • Well, name of the class should correspond to the nearest classes in my view. As already noticed at those Xml/Html/Http names are used inconsistently however it would look odd seeing JSONResponse near to HttpResponse
  • I can move content type default value to the class implementation (as opposed to HttpResponseBase attribute) but then we have 2 places where we need to compute full header's value (including charset)
  • Ok, seeing settings generate a lot of comments I'm going to remove them entirely in favor of encoder and safe parameters
  • Still, encoder's default value should be DjangoJSONEncoder
  • ensure_ascii=False is there to allow non ascii chars to be pushed into response's content. On second thought, though, I actually believe it wasn't good idea. Am going to remove that.

comment:11 Changed 3 years ago by Łukasz Balcerzak

I've pushed changes. One particular thing I don't really like is encoder import inside JsonResponse.__init__ method. Should we put it at the top of the module instead? (serializers package imports django.db so importing django.http would not be allowed unless Django context is configured - this was not needed before)

comment:12 Changed 3 years ago by Tom Christie

Cc: tom@… added

comment:13 Changed 3 years ago by Łukasz Balcerzak

I've just updated PR (

Let's give it another shot and maybe include in 1.7.

Let me know if you believe there are any issues - I'd be happy to fix them.

comment:14 Changed 3 years ago by Hiroki Kiyohara

+1 for this PR.

comment:15 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 0242134d32aa99a54442211ed05576b7061866d1:

Fixed #17942 -- Added a JsonResponse class to more easily create JSON encoded responses.

Thanks leahculver for the suggestion and Erik Romijn,
Simon Charette, and Marc Tamlyn for the reviews.

Note: See TracTickets for help on using tickets.
Back to Top