Opened 7 years ago

Closed 7 years ago

Last modified 7 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 Changed 7 years ago by John Gresty

Keywords: vendor added; jsonp json_api removed

comment:2 Changed 7 years ago by 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?

comment:3 in reply to:  2 Changed 7 years ago by John Gresty

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 Changed 7 years ago by Tim Graham

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 Changed 7 years ago by Adam Johnson

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 Changed 7 years ago by Anton Samarchyan

Cc: desecho@… added
Has patch: set

Added PR

comment:7 Changed 7 years ago by Tim Graham

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

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

Resolution: fixed
Status: newclosed

In 0b5d4c49:

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

comment:9 Changed 7 years ago by Claude Paroz

I spotted a regression introduced by this change: PR

comment:10 Changed 7 years ago by Claude Paroz <claude@…>

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 Changed 7 years ago by Claude Paroz <claude@…>

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