Requests for JSON result in fatal error [#1751328]

Problem/Motivation

When you send a request using Accept: application/json, the response is "A fatal error occurred: The Response content must be a string or object implementing __toString()".

In the similar onAjax function, the page_callback_result is sent through ajax_render(). Doing the same in the onJson function removes the error. This is because the last thing ajax_render does is pass the object through drupal_json_encode.

Proposed resolution

Pass the object through drupal_json_encode.

Comment	File	Size	Author
#6	1751328-05-json-response.patch	678 bytes	Hanspolo
#6
#2	1751328-02-json-response.patch	586 bytes	linclark
#2
#1	1751328-01-json-response.patch	609 bytes	linclark
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Anonymous (not verified) CreditAttribution: Anonymous commented 24 August 2012 at 15:33

Issue tags:

+Novice

File	Size
1751328-01-json-response.patch	609 bytes

This patch resolves the issue.

It can be tested by running "curl -H "Accept: application/json" http://localhost/drupal/node/1" in the command line.

Comment #2

Anonymous (not verified) CreditAttribution: Anonymous commented 24 August 2012 at 17:03

File	Size
1751328-02-json-response.patch	586 bytes

Another way (probably better) to solve this is to use the JsonResponse object's setData() instead of setContent().

Comment #3

larowlan

🇦🇺🏝.au GMT+10

CreditAttribution: larowlan commented 25 August 2012 at 07:23

Status:	Active	» Needs review
Issue tags:		+Needs tests

Comment #4

Anonymous (not verified) CreditAttribution: Anonymous commented 25 August 2012 at 14:44

Status:

Needs review

» Needs work

Instead of calling setData, we can actually pass the array in as the first parameter when constructing the Response object.

Comment #5

Crell CreditAttribution: Crell commented 28 August 2012 at 22:52

If we have an array, yes. Are you saying #2 doesn't work? The bot says it does, and it seems a simple enough fix.

Comment #6

Hanspolo CreditAttribution: Hanspolo commented 2 November 2012 at 13:22

Status:

Needs work

» Needs review

File	Size
1751328-05-json-response.patch	678 bytes

Here is another patch.
It calls the constructor with the data.

Using the data instead of content works fine with arrays.
Non-array data works as with the setContent() method.

Comment #7

patrickd CreditAttribution: patrickd commented 2 November 2012 at 15:23

Status:

Needs review

» Reviewed & tested by the community

Tested:
#1 works
#2 works <-- I personally prefere this one
#6 works

Comment #8

Dries CreditAttribution: Dries commented 2 November 2012 at 17:26

Status:

Reviewed & tested by the community

» Fixed

Committed #2 to 8.x. Thanks!

Comment #9

smartinm CreditAttribution: smartinm commented 2 November 2012 at 17:53

Issue tags:

-Needs tests, -Novice

IMHO this patch could introduce a security flaw: anyone can view the internal structure of a node. For example:

$ curl -H "Accept: application/json" http://localhost/sandbox/drupal-8.x/node/7

Returns a string that contains a JSON representation of the node object:

{ "nodes" : { "#sorted" : true,
      "7" : { "#bundle" : "article",
          "#entity_type" : "node",
          "#langcode" : "en",
          "#node" : { "body" : [  ],
              "changed" : "1351878636",
              "cid" : "0",
              "comment" : "2",
              "comment_count" : "0",
              "created" : "1351878636",
              "data" : "b:0;",
              "entity_view_prepared" : true,
              "field_image" : [  ],
              "field_tags" : [  ],
              "isDefaultRevision" : "1",
              "langcode" : "en",
              "last_comment_name" : null,
              "last_comment_timestamp" : "1351878636",
              "last_comment_uid" : "1",
              "log" : "",
              "name" : "admin",
              "nid" : "7",
              "picture" : "0",
              "promote" : "1",
              "rdf_mapping" : { "body" : { "predicates" : [ "content:encoded" ] },
                  "changed" : { "callback" : "date_iso8601",
                      "datatype" : "xsd:dateTime",
                      "predicates" : [ "dc:modified" ]
                    },
                    // ...
                },
              "revision_timestamp" : "1351878636",
              "revision_uid" : "1",
              "status" : "1",
              "sticky" : "0",
              "title" : "Lorem ipsum",
              "tnid" : "0",
              "translate" : "0",
              "type" : "article",
              "uid" : "1",
              "uuid" : "67a1a948-c5ea-48c9-9dd2-5dd42e4f5ed8",
              "vid" : "7"
            },
          "#pre_render" : [ "_field_extra_fields_pre_render" ],
          "#theme" : "node",
          "#view_mode" : "full",
          "#weight" : 0,
          "comments" : [  ],
          "language" : { "#markup" : "English",
              "#prefix" : "<div id=\"field-language-display\">",
              "#suffix" : "</div>",
              "#title" : "Language",
              "#type" : "item",
              "#weight" : 0
            },
          "links" : { "#attributes" : { "class" : [ "links",
                      "inline"
                    ] },
              "#pre_render" : [ "drupal_pre_render_links" ],
              "#theme" : "links__node",
              // ...
        }
    } }

Comment #10

smartinm CreditAttribution: smartinm commented 2 November 2012 at 17:54

Issue tags:

+Needs tests

This sounds like a new issue is needed. If you can, please make a test case that shows the hole and file it as a new issue, tagging it WSCCI and security. Thanks.