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.

CommentFileSizeAuthor
#6 1751328-05-json-response.patch678 bytesHanspolo
#2 1751328-02-json-response.patch586 byteslinclark
#1 1751328-01-json-response.patch609 byteslinclark

Comments

Anonymous’s picture

Anonymous (not verified) commented
Issue tags: +Novice
StatusFileSize
new 1751328-01-json-response.patch609 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.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new 1751328-02-json-response.patch586 bytes

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

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Active » Needs review
Issue tags: +Needs tests
Anonymous’s picture

Anonymous (not verified) commented
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.

Crell’s picture

Crell commented

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.

Hanspolo’s picture

Hanspolo commented
Status: Needs work » Needs review
StatusFileSize
new 1751328-05-json-response.patch678 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.

patrickd’s picture

patrickd commented
Status: Needs review » Reviewed & tested by the community

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

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed #2 to 8.x. Thanks!

smartinm’s picture

smartinm commented
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",
              // ...
        }
    } }
smartinm’s picture

smartinm commented
Issue tags: +Needs tests
Crell’s picture

Crell commented

I'm not clear how this issue introduces a sec. hole. This just fixes a bug. Right now, er, I'm not actually sure how we're returning JSON forms of nodes yet. :-)

smartinm’s picture

smartinm commented

You're right, the bugfix for this issue is correct but the patch has highlighted the security hole. I think the behavior of the new routing system when matching by HTTP Accept header can be an information disclosure threat.

Crell’s picture

Crell commented

We're not matching by accept header yet.

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.

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Updated the issue summary with new information.