Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#27622 closed New feature (fixed)

Test client should accept vendor tree json variants

Reported by: John Gresty Owned by: nobody
Component: Testing framework Version: dev
Severity: Normal Keywords: json vendor testing_client
Cc: desecho@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

The test client currently only supports parsing json with content-type 'application/json'.

Any variant on json (eg json api)which uses a different content-type will cause a ValueError, the client should be configurable to accept a user defined content-type, or accept "/application\/(vnd\.([a-z]*)\+)?json/"

Change History (11)

comment:1 by John Gresty, 8 years ago

Keywords: vendor added; jsonp json_api removed

comment:2 by Tim Graham, 8 years ago

I'm not familiar with vendor tree json variants. Could you explain that a bit more and give a use case in the Django ecosystem? Is it common practice and/or a standard?

in reply to:  2 comment:3 by John Gresty, 8 years ago

Replying to Tim Graham:

I'm not familiar with vendor tree json variants. Could you explain that a bit more and give a use case in the Django ecosystem? Is it common practice and/or a standard?

Vendor trees are defined in section 3.2 of RFC 6838 (https://tools.ietf.org/html/rfc6838#section-3.2). My use case was trying to test a json api (http://jsonapi.org) implemented using https://github.com/django-json-api/django-rest-framework-json-api which returns the IANA registered content-type 'application/vnd.api+json'

comment:4 by Tim Graham, 8 years ago

Triage Stage: UnreviewedAccepted

Thanks, I guess it's okay to accept application/json variants then. I didn't verify your regex is correct.

comment:5 by Adam Johnson, 8 years ago

The regex mentions vnd.bigcompany.funnypictures as a possible vendor name which the suggested regex won't parse. Maybe django can use a looser regex like r'^application\/(vnd\..+\+)?json$' as per the robustness principle.

Or maybe even drop the header check - if a test calls resp.json() it's already 99.99% sure the response is in JSON, and if it's not, what's wrong with the test giving a JSONDecodeError? (Maybe heading into backwards incompatible territory there though).

comment:6 by Anton Samarchyan, 8 years ago

Cc: desecho@… added
Has patch: set

Added PR

comment:7 by Tim Graham, 8 years ago

Summary: Testing client should accept vendor tree json variantsTest client should accept vendor tree json variants
Triage Stage: AcceptedReady for checkin

comment:8 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: newclosed

In 0b5d4c49:

Fixed #27622 -- Allowed test client to accept vendor tree JSON content types.

comment:9 by Claude Paroz, 8 years ago

I spotted a regression introduced by this change: PR

comment:10 by Claude Paroz <claude@…>, 8 years ago

In 145f6c3:

Refs #27622 -- Fixed a regression in JSON content-type detection

A JSON Content-Type can contain further content, like charset for example.

comment:11 by Claude Paroz <claude@…>, 8 years ago

In ca58a405:

[1.11.x] Refs #27622 -- Fixed a regression in JSON content-type detection

A JSON Content-Type can contain further content, like charset for example.
Backport of 145f6c3ed6e8856078e2d04ff2567e9fb4a17930 from master.

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