I came across a problem where my code was working on one server but not another and traced it down to different PHP versions on the two servers. On one it had > 5.2 so and the other was 5.1.

After a bit of debugging I found the use of json_decode in the 5.2 version which was failing for my methods that took array parameters. Further investigation revealed that arrays were being JSON encoded as objects and then decoded into PHP arrays. This was "working" for the when json_decode was not available.

Long story short, this patch changes the client and server side handling of array values and should work with both json_decode and the non php 5.2 handling. Using non array object as parameters is not supported but I don't see that really coming up as a use case actually.

Comments

dldege’s picture

Status: Active » Needs review
StatusFileSize
new1.99 KB

The patch

dldege’s picture

StatusFileSize
new1.87 KB

Here's a better patch, I didn't notice that the module already was trying to decode arrays so I had modified this part of the if/else instead of the object case (which needs to be changed to properly decode to objects at some time).

Notice also that that I changed the substr to not use -2 as the position since I changed the client side to not send trailing commas.

dmitrig01’s picture

Why don't we support objects? I think it's a much better approach. Oh, and a hint for what's coming: no json parsing on the server side!

dldege’s picture

We don't support objects on the client side with this patch but we probably should - I didn't have time to implement that. The issue is that array and objects need to be treated differently then objects in how the are encoded in JSON and decoded into PHP arrays or objects on the server. Before this patch you were encoding JS arrays into JSON object notation and then back into arrays on the server side. This was working because the keys where all integers 0,1,2.

But this was not correct and doesn't work with json_decode() in php 5.

What is needed is this patch + correct object (non array) handling on both sides.

Lets discuss.

dldege’s picture

More details.

say are calling node.load and passing in the fields argument

('node.load',
      {nid: nid, fields: ["nid", "title", "body"]},

This should be encoded into JSON as (see http://json.org/)

["nid", "title", "body"]

prior to this patch it was being encoded as an object

{ 0 : "nid", 1 : "title", 2 : "body" }

this was handled by your custom JSON parser by converting this back to an array on the server side and it "just worked" because the indices of 0,1,2, did not create an associative array. This did not work with json_decode() in PHP 5.2. Because its not valid.

Object encoding needs to be supported but right now its more important to properly encode arrays as the main service methods take array parameters. I simply put the check for arrays only as a safe guard but I found that is_array() code does not work in Safari.

It might be best to bundle json_server with some other third party JSON parser? Maybe that would be overkill?

dmitrig01’s picture

As a matter of fact, I want to change the module to not send JSON to the server. It would instead get url-encoded and POST'd to the server

('node.load',
      {nid: 1 fields: ["nid", "title", "body"], something: {one: "ab", two: "cd" }},

Would become

method=node.load&nid=1&fields[]=nid&fields[]=title&fields[]=body&something[one]=ab&something[two]=cd

This is what happens with FormAPI now. This gets fed into post. I just need to make the recursive iterator, and then I'm done.

dldege’s picture

OK, eliminating JSON in the method posting sounds like a good idea and should fix the issue completely as you detailed.

Thanks,

AmrMostafa’s picture

Status: Needs review » Active

I have the same problem, it works locally (5.2.5, Ubuntu Gutsy) and doesn't work on the server (5.2.0, Debian Etch).

I've made tests and it does seem that 5.2.0's json_decode() has a bug, which was fixed in later version(s).

Commenting out the json_decode() part in drupal_parse_json() solved the problem for me, but I believe that taking out json parsing would be better as it doesn't seem much needed.

Thank you..

dmitrig01’s picture

Status: Active » Fixed

Fixed in the 1.0 version!

Anonymous’s picture

Status: Fixed » Closed (fixed)

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