Skip to main content

Bug Tracker

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)
  • argshift.diff (0.6 KB) - added by nathanhammond August 14, 2008 05:07AM UTC.

    Makes sure the argument shift is complete.

Change History (8)

Changed May 15, 2008 03:04PM UTC by flesler comment:1

component: coreajax

Changed May 27, 2008 10:36PM UTC by ygirouard 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 nathanhammon comment:3

I've attached a patch that can close this issue.

Changed August 14, 2008 02:18PM UTC by flesler comment:4

cc: → adsmart, nathanhammond
milestone: 1.2.41.3
need: ReviewCommit
owner: → flesler
priority: criticalmajor
status: newassigned

Ok, thanks for the patch. I'll review this asap.

Changed June 01, 2009 08:59PM UTC by paul.irish 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 JDay 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 flesler comment:7

milestone: 1.31.3.3
resolution: → fixed
status: assignedclosed

Fixed at [6564]

Changed November 02, 2010 05:51PM UTC by rwaldron comment:8

#4785 is a duplicate of this ticket.