-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not restrict +json media type suffix to application/ #5894
Conversation
cbee3bf to
212cb40
Compare
Per RFC 6839, section 3.1.
212cb40 to
b0eaa72
Compare
Codecov Report
@@ Coverage Diff @@
## master #5894 +/- ##
==========================================
- Coverage 96.75% 95.90% -0.85%
==========================================
Files 44 102 +58
Lines 9851 31442 +21591
Branches 1591 3218 +1627
==========================================
+ Hits 9531 30155 +20624
- Misses 182 1099 +917
- Partials 138 188 +50
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Sorry, I think that the PR should be rejected. If in your particular case |
|
Can you cite a reference that says +json might not be JSON compatible? The suffix registration is not restricted to the application/ main type. https://www.rfc-editor.org/rfc/rfc6839.html#section-3.1 says:
Additionally, from https://www.rfc-editor.org/rfc/rfc6839.html#section-2
This is exactly the case for aiohttp. I think that's guarantee enough, as much as there is guarantee that anything served as application/json is JSON compatible (there is no guarantee about that either, but both that and +json are reasonable expectations). |
|
Thank you for clarification. |
|
I've changed my mind, the PR is correct. |
Backport to 3.8: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply a82d289 on top of patchback/backports/3.8/a82d2892dbe1266f248362725be0bb338069fbf7/pr-5894 Backporting merged PR #5894 into master
🤖 @patchback |
💔 Backport was not successfulThe PR was attempted backported to the following branches:
|
|
Sorry, the backporting is very hard since previous pull requests that introduced and updated |
What do these changes do?
Fix +json media type suffix handling to apply to other main types besides application/.
An example of a registered media type triggering this issue is https://www.iana.org/assignments/media-types/model/gltf+json
Are there changes in behavior for the user?
Media types with main type other than
applicationusing the '+json` suffix are accepted as JSON now.Related issue number
Checklist
CONTRIBUTORS.txtCHANGESfolder<issue_id>.<type>for example (588.bugfix)issue_idchange it to the pr id after creating the pr.feature: Signifying a new feature..bugfix: Signifying a bug fix..doc: Signifying a documentation improvement..removal: Signifying a deprecation or removal of public API..misc: A ticket has been closed, but it is not of interest to users.