dojo.rpc.* handling of Json should probably use: json-comment-optional

[jaredj]

dojo.rpc.* handling of Json should probably use: json-comment-optional

Dmachi,

This was brought up to me by a couple co-workers that the Rpc code in dojo.rpc didn't support running in the more secure mode of comment filtered JSON. To support this, it's an absolutely trivial change to the three files to change the handleAs to:

json-comment-optional

I'll be attaching a patch for this. If you don't mind me just applying the patch and committing the enhancement, I'll go ahead and do it. I just wanted your blessing first, since this is your module. Thanks!

-- Jared

[jaredj]

Simple patch to dojo.rpc code for supporting comment filtered JSON RPC services.

[dmachi]

Go right ahead, the change is fine with me.

[jaredj]

Excellent. Thanks dmachi!

[jaredj]

Running a few tests on it. I think a change peller made to xhr is causing a problem. He's using a regular expression to trim off the start and end comments ... but I think it's breaking on comments inside the JSON. I don't know if it's even valid to have comments inside JSON, but.

In specific, the yahoo SMD file has comments embedded in it There at:

SyntaxError?: unterminated comment message=unterminated commentbootstrap.js (line 183) Objectbootstrap.js (line 283) Run Test: Objectbootstrap.js (line 294) TypeError?: this.svc.webSearch is not a function message=this.svc.webSearch is not a functionbootstrap.js (line 263) ERROR IN: (function () {var d = new doh.Deferred;if (window.location.protocol == "file:") {var err = new Error("This Test requires a webserver and will fail intentionally if loaded from file://");d.errback(err);return d;}console.debug("Run Test: ", this.svc);var td = this.svc.webSearch({[Error: Query filter requires field and constraints separated by a "="]});td.addCallbacks(function (result) {return true;if (result.ResultSet?.Result[0].DisplayUrl? == "dojotoolkit.org/") {return true;} else {return new Error("JsonRpc?_SMD_Loading_Test failed, resultant content didn't match");}}, function (result) {return new Error(result);});td.addBoth(d, "callback");return d;})bootstrap.js (line 263) FAILED test: JsonP_test

Which I think is cased by the SMD file having:

]

}

/*

{

//

"name":"",

"serviceURL": "",

"parameters":[

{ "name":"street", "type":"STRING" },

]

}

*/

]

}

A commented block in it.

Routing to Peller for his take, since he made the change to base xhr with the regexp.

Won't commit this till we know how this should be properly handled.

Peller,

Input? I think that regexp has another issue to deal with. Argh. :-/

[peller]

Crockford said today that comments are not valid in JSON, yet his own company is using them. Clearly, we have to be tolerant of this, so I'll probably just revert the regexp change. It was an attempt to reduce the code to something more clear and efficient.

I'm a little confused as to why this is taking place in this ticket, though. We've been trying to keep Dojo trac tickets short and focused.

[peller]

(In [11157]) I give up. Put back the indexOf searches for comments in place of regexp attempt. Refs #3961, #4829, #4888 Reverts part of change in [10834]

[jaredj]

(In [11190]) Committing in minor enhancement for RPC. fixes #4888

[jaredj]

(In [11191]) After more looking, I don't think you can do jsonp comment filtered. So reverting #4888. refs #4888