The function drupal_json() in common.inc sets the headers for a JSON response. Currently it uses an old mime-type for javascript, text/javascript, when it's actually a JSON response which I think should be using the JSON-mime-type, application/json.
JSON has it's origin in javascript - but is a format understandable by practically all languages. The mime-type should reflect that so consumers of the content will be given the right signal about what content it has recieved - because consumers of real JSON shouldn't treat it as javascript because of security reasons.
I'm attaching a patch that changes this.
If the mime-type for some reasons should be kept as javascript - then the mime-type should at least be changed to application/javascript - since text/javascript is considered obsolete. Read more about that in: http://en.wikipedia.org/wiki/Internet_media_type
Comment | File | Size | Author |
---|---|---|---|
#34 | json_content_type-341588-d6-34.patch | 462 bytes | asvsot |
#30 | json_content_type-341588-d6-30.patch | 593 bytes | Albert Volkman |
#28 | json_content_type-341588-d6-28.patch | 600 bytes | Albert Volkman |
#22 | 2010-05-04-json-content-type.patch | 836 bytes | mikl |
#13 | 2010-05-01-json-content-type.patch | 633 bytes | dawehner |
Comments
Comment #2
voxpelli CreditAttribution: voxpelli commentedRerolling the patch - apparently the test bot didn't like git's formatting of it...
Comment #3
c960657 CreditAttribution: c960657 commentedAFAICT from RFC 4627, the
charset
parameter is not allowed with application/json. For comparison, see RFC 4329 that explicitly mentionscharset
as an optional parameter.Comment #4
voxpelli CreditAttribution: voxpelli commentedAh - that's interesting - great that valid JSON needs to be UTF-8 encoded!
I'm attaching a new patch that removes the charset from the header.
Comment #5
c960657 CreditAttribution: c960657 commentedAppearently IE has some issues with application/json:
http://www.entwicklungsgedanken.de/2008/06/06/problems-with-internet-exp...
Not sure whether this is relevant for Drupal, though.
Comment #6
voxpelli CreditAttribution: voxpelli commentedI haven't come across any problems with IE, AJAX-requests and application/json - I believe that blogger does something wrong.
You can try out a non-drupal site I've built that uses application/json (although with the incorrect charset which I hadn't noticed when the site was built): http://www.bergkrantz.se/projekt/lista
Comment #8
voxpelli CreditAttribution: voxpelli commentedReroll
Comment #10
c960657 CreditAttribution: c960657 commenteddrupal_set_header() was just renamed to drupal_add_http_header() in #451604: Rename drupal_set_header().
Comment #11
mikl CreditAttribution: mikl commentedGiven that Drupal’s current behaviour is in violation of RFC 4627 that clearly states “The MIME media type for JSON text is application/json.” as defined by IANA, I’m taking the liberty of elevating this to critical.
I’ve attached a new patch. Easy fix :)
Comment #12
mikl CreditAttribution: mikl commentedElevating to critical.
Comment #13
dawehnerThis patch changes the comment, as in the above version. The patch looks fine.
Comment #14
Damien Tournoud CreditAttribution: Damien Tournoud commentedThis doesn't break anything, and as a consequence is not critical.
Otherwise, I'm fine with the change.
Comment #16
dawehnerretest
Comment #17
Damien Tournoud CreditAttribution: Damien Tournoud commented#13: 2010-05-01-json-content-type.patch queued for re-testing.
Comment #19
dawehner#13: 2010-05-01-json-content-type.patch queued for re-testing.
Comment #20
mikl CreditAttribution: mikl commentedWell, the test checked out, and it’s been manually reviewed, so I suppose this is RTBC.
Comment #21
voxpelli CreditAttribution: voxpelli commentedIs it intentionally that the feedback from #3 has been ignored?
Comment #22
mikl CreditAttribution: mikl commented#21: No, I managed to miss that, sorry. Here’s a new patch which should be compliant.
Comment #23
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #25
Heine CreditAttribution: Heine commentedReopening for backport to D6.
Comment #26
voxpelli CreditAttribution: voxpelli commentedThis is an API-change so I don't think it should be allowed into D6.
Comment #27
kratkar CreditAttribution: kratkar commentedCreate for D6 module work with JSON, to not change API))
Comment #28
Albert Volkman CreditAttribution: Albert Volkman commentedD6 backport.
Comment #29
c960657 CreditAttribution: c960657 commenteddrupal_add_http_header() is not defined in D6.
Comment #30
Albert Volkman CreditAttribution: Albert Volkman commentedOops.
Comment #31
c960657 CreditAttribution: c960657 commentedComment #32
Gábor HojtsyThanks, pushed, committed.
Comment #34
asvsot CreditAttribution: asvsot commentedIn some cases in different browsers encoding is broken without specifying utf-8
charset
. Considering #3 it is wrong to includecharset
in header, but following common sense and on practice it would be usefull to include it. Furthermore RFC does not dissallowcharset
parameter, it just haven't mentioned about it's usage.Comment #35
asvsot CreditAttribution: asvsot commentedComment #36
mikl CreditAttribution: mikl commented#34: I'd like to have some more concrete evidence of this. To my knowledge, all major browsers support the application/json mime type correctly.
Comment #37
asvsot CreditAttribution: asvsot commented#36: Here is some example:
Valid case:
Broken utf encoding:
print drupal_json(array('åæéø'));
- could be reproduced in some browsers, randomly appears on different clients according to OS, etc.
Comment #38
mikl CreditAttribution: mikl commentedOk, I've made a small test case module: https://github.com/mikl/drupal-json_test
You can see it in action at http://hoegh.org/json_test
Once the JSON data is loaded, it's supposed to say:
Amongst my devices (mostly Apple stuff), I haven't been able to find a browser that doesn't render that correctly.
Please provide examples of browsers that can't render that page correctly.
Comment #39
mikl CreditAttribution: mikl commentedClosing this, unless you have a test case where this actually fails.