Side navigation
#2452 closed bug (fixed)
Opened March 03, 2008 09:58PM UTC
Closed September 15, 2009 03:31PM UTC
Last modified November 02, 2010 05:51PM UTC
$.post and $.get are probably not handling the abcense of data well
Reported by: | adsmart | Owned by: | flesler |
---|---|---|---|
Priority: | major | Milestone: | 1.4 |
Component: | ajax | Version: | 1.2.3 |
Keywords: | Cc: | adsmart, nathanhammond | |
Blocked by: | Blocking: |
Description
I was looking for the post and get code today (I'm planning on replicating them for the DELETE and PUT verbs... those should be added) and I noticed a funny problem with the following (GET has the same problem):
post: function( url, data, callback, type ) { if ( jQuery.isFunction( data ) ) { callback = data; data = {}; } return jQuery.ajax({ type: "POST", url: url, data: data, success: callback, dataType: type }); },
You make a check to see if the second argument is a function, indicating that no data is being sent. That's fine but it ignores the fact that the type might also be passed. As a result, the following would have unintended effects:
$.post("/sample/test", do_stuff_function, "text/html");
It also doesn't take into account
$.post("/sample/test", "text/html");
Which, while probably silly in most cases, is legitimate within the published docs for the functions.
I suspect the following would be a better solution (depending on how you feel about using arguments.length, you could use other methods to identify things):
if (arguments.length == 2 ) { type = data; callback = function () {}; data = {}; } else if (arguments.length == 3) { type = callback; callback = data; data = {}; };
Attachments (1)
Change History (8)
Changed May 15, 2008 03:04PM UTC by comment:1
component: | core → ajax |
---|
Changed May 27, 2008 10:36PM UTC by comment:2
A fullproof solution would probably be to force users to pass an array of options (array object or json), much like you do for $.ajax(); That way it avoids having to deal with omited arguments since you reference each as an object property.
For example:
$.get({url: "/sample/test", callback: do_stuff_function, type: "html"});
Then you can test each parameter individually, and if set to 'undefined' you can ignore it or set a default.
i.e.:
post: function(params) { if (params.data == undefined || params.data == ""){params.data = {};} // Do other testing here if needed... return jQuery.ajax({ type: "POST", url: params.url, data: params.data, success: params.callback, dataType: params.type }); },
Changed August 14, 2008 05:09AM UTC by comment:3
I've attached a patch that can close this issue.
Changed August 14, 2008 02:18PM UTC by comment:4
cc: | → adsmart, nathanhammond |
---|---|
milestone: | 1.2.4 → 1.3 |
need: | Review → Commit |
owner: | → flesler |
priority: | critical → major |
status: | new → assigned |
Ok, thanks for the patch. I'll review this asap.
Changed June 01, 2009 08:59PM UTC by comment:5
Ticket #3775 has gotten some attention on this issue, too. I think we can close it as a dupe.
Ariel, can we drop in Nathan's non-intrusive patch?
Changed June 17, 2009 11:34PM UTC by comment:6
A patch is included with dupe #4384 with a test case and a fix for the comment typo.
Changed September 15, 2009 03:31PM UTC by comment:7
milestone: | 1.3 → 1.3.3 |
---|---|
resolution: | → fixed |
status: | assigned → closed |
Fixed at [6564]