| Project: | Drupal core |
| Version: | 6.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
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
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| drupal_json_mime_type.patch | 608 bytes | Idle | Failed: Failed to apply patch. | View details | Re-test |
Comments
#1
The last submitted patch failed testing.
#2
Rerolling the patch - apparently the test bot didn't like git's formatting of it...
#3
AFAICT from RFC 4627, the
charsetparameter is not allowed with application/json. For comparison, see RFC 4329 that explicitly mentionscharsetas an optional parameter.#4
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.
#5
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.
#6
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
#7
The last submitted patch failed testing.
#8
Reroll
#9
The last submitted patch failed testing.
#10
drupal_set_header() was just renamed to drupal_add_http_header() in #451604: Rename drupal_set_header().
#11
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 :)
#12
Elevating to critical.
#13
This patch changes the comment, as in the above version. The patch looks fine.
#14
This doesn't break anything, and as a consequence is not critical.
Otherwise, I'm fine with the change.
#15
The last submitted patch, 2010-05-01-json-content-type.patch, failed testing.
#16
retest
#17
#13: 2010-05-01-json-content-type.patch queued for re-testing.
#18
The last submitted patch, 2010-05-01-json-content-type.patch, failed testing.
#19
#13: 2010-05-01-json-content-type.patch queued for re-testing.
#20
Well, the test checked out, and it’s been manually reviewed, so I suppose this is RTBC.
#21
Is it intentionally that the feedback from #3 has been ignored?
#22
#21: No, I managed to miss that, sorry. Here’s a new patch which should be compliant.
#23
Committed to CVS HEAD. Thanks.
#24
Automatically closed -- issue fixed for 2 weeks with no activity.
#25
Reopening for backport to D6.
#26
This is an API-change so I don't think it should be allowed into D6.
#27
Create for D6 module work with JSON, to not change API))
#28
D6 backport.