Skip to main content

Bug Tracker

Side navigation

#12233 closed bug (fixed)

Opened August 10, 2012 04:23AM UTC

Closed August 16, 2012 05:13PM UTC

Last modified October 15, 2012 10:15PM UTC

jQuery.post() raises "RangeError: Maximum call stack size exceeded"

Reported by: gtr053@gmail.com Owned by:
Priority: low Milestone: 1.8.1
Component: ajax Version: 1.8.0
Keywords: Cc: jaubourg
Blocked by: Blocking:
Description

Specifying the url, data, and dataType parameter without specifying the success parameter (which is of course optional) raises "RangeError: Maximum call stack size exceeded" in Chrome 21.0.1180.77 beta. A similar error occurs in Firefox 14.0.1. Both were tested on Mac OS X 10.8. Here is a "working" example: http://jsfiddle.net/TAVKB/

Attachments (0)
Change History (17)

Changed August 10, 2012 04:24AM UTC by gtr053@gmail.com comment:1

Also, this worked perfectly in jQuery 1.7.2.

Changed August 10, 2012 02:37PM UTC by dmethvin comment:2

_comment0: It didn't work perfectly in 1.7.2, it was ignoring the `dataType` parameter which it thought was a (bad) `success` callback. The consequences are just worse in 1.8. Here's the documented signature: \ {{{ \ jQuery.post( url [, data] [, success(data, textStatus, jqXHR)] [, dataType] ) \ }}} \ That says the `data`, `success` and `dataType` arguments are optional and independent of each other. However, there are some ambiguities; what if you wanted to specify a `dataType` but no `data` or `success` callback? Both are strings so there's no way to know which one you mean. \ \ If you just add a `null` for the callback it works fine: \ {{{ \ jQuery.post('/echo/text/', 'test=test', null, 'text'); \ }}} \ \ Given the docs and the consequences, I'm inclined to fix this by trying to improve the parameter hockey we play. But really, if you're doing tricky stuff you're much better off making it explicit by adding the `null` or better, using `$.ajax` rather than the shortcut methods. \ \ Alternatively, we can just document the `success` argument as manditory. \ {{{ \ jQuery.post( url [, data] [, success(data, textStatus, jqXHR) [, dataType] ] ) \ }}} \ 1344609607327703
cc: → jaubourg
component: unfiledajax
priority: undecidedlow
status: newopen

It didn't work perfectly in 1.7.2, it was ignoring the dataType parameter which it thought was a (bad) success callback. The consequences are just worse in 1.8. Here's the documented signature:

jQuery.post( url [, data] [, success(data, textStatus, jqXHR)] [, dataType] )

That says the data, success and dataType arguments are optional and independent of each other. However, there are some ambiguities; what if you wanted to specify a dataType but no data or success callback? Both are strings so there's no way to know which one you mean.

If you just add a null for the callback it works fine:

jQuery.post('/echo/text/', 'test=test', null, 'text');

Given the docs and the consequences, I'm inclined to fix this by trying to improve the parameter hockey we play. But really, if you're doing tricky stuff you're much better off making it explicit by adding the null or better, using $.ajax rather than the shortcut methods.

Alternatively, we can just document the success argument as manditory (if you want to provide dataType).

jQuery.post( url [, data] [, success(data, textStatus, jqXHR) [, dataType] ] )

Changed August 11, 2012 02:10AM UTC by gtr053@gmail.com comment:3

Shouldn't data always have an equal sign in it? See what I'm getting at?

Changed August 11, 2012 02:12AM UTC by gtr053@gmail.com comment:4

Or an empty string

Changed August 16, 2012 02:53PM UTC by jaubourg comment:5

We should probably look into handling parameters better, but something is fishy in the Callbacks implementation, it shouldn't go "berserk" like it does.

Changed August 16, 2012 04:41PM UTC by robert@accettura.com comment:6

I'm actually getting the same thing, but using $.ajax. Not sure what the best path from here is.

async: true
cache: true
dataType: "script"
success: "x.y.handleResponse"
url: "http://localhost&callback=x.y.handleResponse"
__proto__: Object

Like others, it worked fine in 1.7.x

Changed August 16, 2012 04:51PM UTC by gtr053@gmail.com comment:7

This problem occurs when jQuery interprets success as a string. Robert, this is likely where your problem is coming from. Your problem is not a bug in jQuery.

Changed August 16, 2012 04:59PM UTC by anonymous comment:8

Yup. You're correct. Strangely, I actually had a check from the 1.6 days in there like this:

if (this.callback != 'string')

all I had to do was update to:

if (typeof this.callback != 'string'){

Anyway, now working for me. Would be nice if this failed more gracefully however, Chrome handles this better than Firefox which becomes unresponsive in at least some cases.

Changed August 16, 2012 05:13PM UTC by jaubourg comment:9

resolution: → fixed
status: openclosed

Makes sure "adding" a string to a Callbacks object doesn't cause a stack overflow, just ignore the value like 1.7.x righfully did. Fixes #12233. Unit tests added.

Changeset: 9d07525a71e7bc12f606d8015d21425c5580e262

Changed August 16, 2012 05:16PM UTC by jaubourg comment:10

milestone: None1.8.1

Now, do we also fix "the parameter hockey we play" or do we document?

Changed August 16, 2012 05:21PM UTC by dmethvin comment:11

I think we should document that the callback must be there, even if it's just a null placeholder. That way the string following it is always the dataType and can't be mistaken for the data arg. Does that seem reasonable? If so add a needsdocs.

Changed August 16, 2012 05:30PM UTC by jaubourg comment:12

keywords: → needsdocs

Seems reasonable enough to me... put null in place of the callback if you wanna provide a dataType.

Changed August 16, 2012 05:42PM UTC by gtr053@gmail.com comment:13

IMHO, it's hard to get them mixed up. As I was saying earlier, the data parameter, when a string, can either be empty or have an equal sign in it? Neither of which dataType should be. I can't imagine it being much more difficult than normal "parameter hockey". Requiring one ''optional'' argument present in order for another optional argument to be allowed is silly IMO. If we are going down the use null as a placeholder road, what would happen if the data parameter is omitted? We'd be back at square one.

Changed August 31, 2012 02:10AM UTC by anonymous comment:14

I'll offer a note here: overloading methods in JavaScript is inadvisable. Not trying to flame-bait, but the simple facts are that the resolving the behavior based on so-called "parameter hockey" incurs runtime overhead, is hard to reason about, and often results in flaky, brittle code. IMO, overloading should be kept to an absolute minimum, and I'll offer a rule-of-thumb for situations like these: if you can't simply look at the number of arguments, or use a simple, reliable type-check of some sort to resolve the proper behavior, don't overload the method.

Changed August 31, 2012 02:14AM UTC by dmethvin comment:15

@anonymous, totally agreed. That's why I'm not inclined to "fix" this any further in code. The current messy signature of jQuery.post() is what it is, however; there's no way to fix it retroactively without causing even more pain. Unless you happen to have a time machine.

Changed September 01, 2012 04:44AM UTC by anonymous comment:16

@dmethvin - deprecate the signature but leave the implementation as-is for legacy reasons?

Changed October 15, 2012 10:15PM UTC by mikesherov comment:17

keywords: needsdocs