jquery hangs in $.param when passed a jquery object

[jsuder]

I found that there is a very easy way to make the browser hang in an infinite loop using latest jQuery. To do that, you need to pass an object containing a jQuery collection to $.param.

The way I found it is that I wasn't aware that $.fn.map returns a jQuery collection, and I was passing it to $.post as data. This code has worked fine before on jQuery 1.4.2, and then stopped working when I've upgraded jQuery to 1.7.1.

Here's a JSFiddle test with an example: ​http://jsfiddle.net/gfzSU/ - if you run it as is and click the button, it will just hang. This must have been changed somewhere between 1.4.4 and 1.5.2, because it works on 1.4.4 and starts behaving like this on 1.5.2.

[jsuder]

Finally... guys, sorry to say this but this bug tracker software is a piece of sh*.*... First I tried to report this bug without logging in, but said it was spam and I had to fill a captcha. I did, but then it told me it was spam anyway. So I tried to log in but none of my passwords worked, so I filled the forgot password form and waited for an email. I never got the email. So today I've registered a second account, and it told me simultaneously that it didn't work because someone is using the same name *and* that it did work (I got two messages, one in green and one in red). And then when I logged in and re-submitted this, guess what? It told me I needed to fill a captcha because it was spam. Luckily, it worked this time after filling the captcha...

[Rick Waldron]

If you need an array, use toArray(). Note the docs don't mention support for serializing jQuery objects ​http://api.jquery.com/jQuery.param/ - only array and object

​http://jsfiddle.net/rwaldron/SV8eX/

[jsuder]

Wouldn't e.g. throwing an error be a better solution than making the browser hang? Imagine parseInt() punished you with hanging the browser for forgetting to add 10 as the second argument...

[Rick Waldron]

parseInt doesn't throw when radix is omitted. Additionally, it's well known that circular structures cannot be serialized. Please use the forum for support questions

[gibson042]

Throwing an error is a better response to circular input than hanging the browser. A fix is ready if you want it

​https://github.com/jquery/jquery/pull/665 (-57/+113/+48)

[dmethvin]

That is a big amount of code for a bug that should be found before the page is deployed. I agree that theoretically it would be great to diagnose and clearly explain every misuse of the API with a good error message, but it just has too high a cost.

[Rick Waldron]

jQuery.param is meant to be used for serializing objects that will be sent to a server - it should be assumed that this means it is limited to circular reference safe values. It's smarter to write code that doesn't make this mistake, then to modify jQuery core and burden all it's other users with the additional file size.

[gibson042]

Out of curiosity, would you accept it at half the size?

[dmethvin]

I think it's more a philosophical thing. Why would we check for this condition and throw an error when we don't do it for many others? What is special about this error?

[jsuder]

I might be biased here, but I'd say it's rather easy to cause this error, because jQuery collections behave just like arrays in most cases (if you print them in the console you can't see any difference), and you might not notice that the thing you got from map() and looks like an array of integers is not a real array. It wouldn't even have to check for every possible circular reference, just if there aren't any jQuery collections inside the passed object, if they are known to be a problem in this case.

Also, hanging the browser is a pretty brutal reaction, most errors that you can generate for using the API in a wrong way result in either an exception (with stack trace and/or other details) or nothing happening at all when it should have.

[gibson042]

Replying to dmethvin:

I think it's more a philosophical thing. Why would we check for this condition and throw an error when we don't do it for many others? What is special about this error?

That running afoul of it hangs the client in an infinite loop, as opposed to other issues that just yield unexpected results.

[dmethvin]

So we just need to break it so that passing a jQuery object won't cause a loop? How about this?

​https://github.com/dmethvin/jquery/commit/90a5b4486fdcd0a90386723c49df660619bcd534

However, there are clearly an infinite number of other bad inputs that can cause similar problems. Should we guard against those as well? If so, why?

[gibson042]

Like you said, it's a philosophical thing. I think API calls should stay on the good side of the halting problem and always terminate. You agreed above, but considered 48 bytes too steep a price for that guarantee. So now I'm asking about 24.

[Rick Waldron]

Can't wait for the bugs on this one... "my object has a property called 'jquery' that $.param won't serialize". How about this, try being more accountable for your own code and stop burdening the rest of jQuery users with cruft to protect your programs from YOU.

I'm asking for zero bytes, in return I promise to write smarter programs that don't try to serialize lists of nodes.

[gibson042]

jQuery.param already has special code for differentiating jQuery objects from objects with a ' jquery' property. Not that it's relevant anyway; my PR was for terminating on all input, not for specifically rejecting jQuery.

But if that's how you feel about bad input, why not go all the way? I bet I can save 100 bytes by getting rid ofjQuery.error.