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

Files: 
CommentFileSizeAuthor
#34 json_content_type-341588-d6-34.patch462 bytesasvsot
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#30 json_content_type-341588-d6-30.patch593 bytesAlbert Volkman
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#28 json_content_type-341588-d6-28.patch600 bytesAlbert Volkman
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#22 2010-05-04-json-content-type.patch836 bytesmikl
PASSED: [[SimpleTest]]: [MySQL] 20,154 pass(es).
[ View ]
#13 2010-05-01-json-content-type.patch633 bytesdawehner
PASSED: [[SimpleTest]]: [MySQL] 20,155 pass(es).
[ View ]
#11 2010-05-01-json-content-type.patch1.09 KBmikl
PASSED: [[SimpleTest]]: [MySQL] 20,157 pass(es).
[ View ]
#8 drupal_json_mime_type_4.patch587 bytesvoxpelli
Failed: Failed to install HEAD.
[ View ]
#4 drupal_json_mime_type_3.patch504 bytesvoxpelli
Failed: 10267 passes, 1 fail, 0 exceptions
[ View ]
#2 drupal_json_mime_type.patch489 bytesvoxpelli
Failed: Failed to install HEAD.
[ View ]
drupal_json_mime_type.patch608 bytesvoxpelli
Failed: Failed to apply patch.
[ View ]

Comments

Status:Needs review» Needs work

The last submitted patch failed testing.

voxpelli’s picture

Status:Needs work» Needs review
StatusFileSize
new489 bytes
Failed: Failed to install HEAD.
[ View ]

Rerolling the patch - apparently the test bot didn't like git's formatting of it...

c960657’s picture

Status:Needs review» Needs work

AFAICT from RFC 4627, the charset parameter is not allowed with application/json. For comparison, see RFC 4329 that explicitly mentions charset as an optional parameter.

voxpelli’s picture

Status:Needs work» Needs review
StatusFileSize
new504 bytes
Failed: 10267 passes, 1 fail, 0 exceptions
[ View ]

Ah - 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.

c960657’s picture

Appearently 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.

voxpelli’s picture

I 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

Status:Needs review» Needs work

The last submitted patch failed testing.

voxpelli’s picture

Status:Needs work» Needs review
StatusFileSize
new587 bytes
Failed: Failed to install HEAD.
[ View ]

Reroll

Status:Needs review» Needs work

The last submitted patch failed testing.

c960657’s picture

drupal_set_header() was just renamed to drupal_add_http_header() in #451604: Rename drupal_set_header().

mikl’s picture

Title:drupal_json() should use correct mime-type» drupal_json_output() should use correct mime-type
Status:Needs work» Needs review
StatusFileSize
new1.09 KB
PASSED: [[SimpleTest]]: [MySQL] 20,157 pass(es).
[ View ]

Given 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 :)

mikl’s picture

Title:drupal_json_output() should use correct mime-type» drupal_json_output() must use correct mime-type
Priority:Normal» Critical

Elevating to critical.

dawehner’s picture

StatusFileSize
new633 bytes
PASSED: [[SimpleTest]]: [MySQL] 20,155 pass(es).
[ View ]

This patch changes the comment, as in the above version. The patch looks fine.

Damien Tournoud’s picture

Priority:Critical» Normal
Status:Needs review» Reviewed & tested by the community

This doesn't break anything, and as a consequence is not critical.

Otherwise, I'm fine with the change.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 2010-05-01-json-content-type.patch, failed testing.

dawehner’s picture

Status:Needs work» Needs review

retest

Damien Tournoud’s picture

#13: 2010-05-01-json-content-type.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, 2010-05-01-json-content-type.patch, failed testing.

dawehner’s picture

Status:Needs work» Needs review

#13: 2010-05-01-json-content-type.patch queued for re-testing.

mikl’s picture

Status:Needs review» Reviewed & tested by the community

Well, the test checked out, and it’s been manually reviewed, so I suppose this is RTBC.

voxpelli’s picture

Is it intentionally that the feedback from #3 has been ignored?

mikl’s picture

StatusFileSize
new836 bytes
PASSED: [[SimpleTest]]: [MySQL] 20,154 pass(es).
[ View ]

#21: No, I managed to miss that, sorry. Here’s a new patch which should be compliant.

Dries’s picture

Status:Reviewed & tested by the community» Fixed

Committed to CVS HEAD. Thanks.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Heine’s picture

Title:drupal_json_output() must use correct mime-type» json output must use correct mime-type
Version:7.x-dev» 6.x-dev
Status:Closed (fixed)» Patch (to be ported)

Reopening for backport to D6.

voxpelli’s picture

This is an API-change so I don't think it should be allowed into D6.

kratkar’s picture

Create for D6 module work with JSON, to not change API))

Albert Volkman’s picture

Status:Patch (to be ported)» Needs review
StatusFileSize
new600 bytes
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

D6 backport.

c960657’s picture

Status:Needs review» Needs work
-  // We are returning JavaScript, so tell the browser.
-  drupal_set_header('Content-Type: text/javascript; charset=utf-8');
+  // We are returning JSON, so tell the browser.
+  drupal_add_http_header('Content-Type', 'application/json');

drupal_add_http_header() is not defined in D6.

Albert Volkman’s picture

Status:Needs work» Needs review
StatusFileSize
new593 bytes
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

Oops.

c960657’s picture

Status:Needs review» Reviewed & tested by the community
Gábor Hojtsy’s picture

Status:Reviewed & tested by the community» Fixed

Thanks, pushed, committed.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

asvsot’s picture

Status:Closed (fixed)» Active
StatusFileSize
new462 bytes
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

In some cases in different browsers encoding is broken without specifying utf-8 charset. Considering #3 it is wrong to include charset in header, but following common sense and on practice it would be usefull to include it. Furthermore RFC does not dissallow charset parameter, it just haven't mentioned about it's usage.

asvsot’s picture

Status:Active» Needs review
mikl’s picture

#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.

asvsot’s picture

#36: Here is some example:
Valid case:

drupal_set_header('Content-Type: application/json; charset=utf-8');
print drupal_to_js(array('åæéø'));

Broken utf encoding:
print drupal_json(array('åæéø'));
- could be reproduced in some browsers, randomly appears on different clients according to OS, etc.

mikl’s picture

Ok, 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:

Internationalization test string is:
Iñtërnâtiônàlizætiøn

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.

mikl’s picture

Status:Needs review» Closed (fixed)

Closing this, unless you have a test case where this actually fails.