Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#6601 closed defect (fixed)

[CLA] - Syntax error with xhrGet and JSON handling with 20x and no content

Reported by: Chris Mitchell Owned by: Adam Peller
Priority: high Milestone: 1.1.1
Component: Core Version: 1.1.0
Keywords: Cc: James Burke
Blocked By: Blocking:

Description (last modified by Adam Peller)

I am calling dojo.xhrGet with handleAs: "json-comment-optional" and the service is giving back "". which is causing

dojo.fromJson = function(/*String*/ json){
	// summary:
	// 		Parses a [JSON](http://json.org) string to return a JavaScript object.
	// json: 
	//		a string literal of a JSON item, for instance:
	//			`'{ "foo": [ "bar", 1, { "baz": "thud" } ] }'`
	return eval("(" + json + ")"); // Object
}

In 20x responses with no content, json is "", resulting in the handler throwing a syntax error. Throwing a syntax error with no content is not correct behavior, and it previously worked in 1.0.x.

To fix the problem, we're running with the following patch to the above function in dojo/_base/xhr.js:

return json==="" ? "" : eval("("+json+")");

This patch is submitted under CCLA by Chris Mitchell.

Can we get this in for 1.1.1?

Attachments (1)

6601.patch (1.0 KB) - added by Adam Peller 12 years ago.
return undefined when xhr results in a 204

Download all attachments as: .zip

Change History (5)

comment:1 Changed 12 years ago by Adam Peller

Description: modified (diff)
Milestone: 1.1.1
Owner: changed from anonymous to Adam Peller
Reporter: changed from guest to Chris Mitchell

Changed 12 years ago by Adam Peller

Attachment: 6601.patch added

return undefined when xhr results in a 204

comment:2 Changed 12 years ago by Adam Peller

Cc: James Burke added

So I don't think we should allow an empty string in fromJson, since it's not valid. We can special case in xhr for json, but probably only for http error 204, where an empty content type is required, and in that case we still wouldn't want to pass back "" or anything which might be a valid JSON return type, because you couldn't tell the difference. We could return 'undefined', or just throw an exception, which the caller could handle and inspect the thrown error object for status. The attached patch returns undefined. Thoughts?

comment:3 Changed 12 years ago by Adam Peller

Resolution: fixed
Status: newclosed

(In [13457]) Return undefined for JSON requests with HTTP Status 204, rather than throw, since it's successful by definition. Fixes #6601 Fix typo in dojo.query comments and style of assignments in conditionals. !strict

comment:4 Changed 12 years ago by Adam Peller

(In [13458]) Return undefined for JSON requests with HTTP Status 204, rather than throw, since it's successful by definition. (1.1 branch) Refs #6601

Note: See TracTickets for help on using tickets.