Skip to main content

Bug Tracker

Side navigation

#11137 closed bug (invalid)

Opened January 06, 2012 08:35PM UTC

Closed January 09, 2012 03:25PM UTC

Last modified March 14, 2012 10:21AM UTC

Jquery doesn't parse a rare selector correctly

Reported by: fam.lam Owned by: fam.lam
Priority: low Milestone: None
Component: selector Version: 1.7.1
Keywords: Cc:
Blocked by: Blocking:
Description

If you execute this code (in Chrome, you can just paste it in the console), JQuery will hang for a very long time.

(If you leave out the 'window.open', then you'll just get a hang for about 5-10 seconds).

var selector = "[onclick^*=\\"window.open('http://www.firstload.de/\\"]"
$(selector)
Attachments (0)
Change History (15)

Changed January 06, 2012 08:38PM UTC by fam.lam comment:1

*in the console of a jQuery using page* of course, otherwise the console uses it's own $ selector, which does return null. Or replace the '$' with 'jQuery' if you want to be sure.

Changed January 06, 2012 08:57PM UTC by fam.lam comment:2

_comment0: Oh... sorry... although it is (I think?) a bug that such an invalid selector hangs jQuery so long, I hadn't noticed I introduced a bug in the selector myself... \ \ Corrected reproduction steps: \ 1. go to battle.net (uses an old version of jQuery, but also with a newer version (which you can insert by content scripts if you like) it fails the test \ 2. in Chrome's console, insert \ {{{ \ var selector = "[onclick^=\\"window.open('http://www.firstload.de/\\"]" \ jQuery(selector) \ }}} \ 3. wait forever...1325884248348624

Oh... sorry... although it is (I think?) a bug that such an invalid selector hangs jQuery so long, I hadn't noticed I introduced a bug in the selector myself...

Corrected reproduction steps:

1. go to battle.net (uses an old version of jQuery, but also with a newer version (which you can insert by content scripts if you like) it fails the test

2. in Chrome's console, insert

var selector = "[onclick^=\\"window.open('http://www.firstload.de/\\"]"
jQuery(selector)

3. wait forever...

I've created a content script for you:

Install http://solidfiles.com/d/b03088d9ff/ in Chrome and visit battle.net

It'll first alert the jQuery version it uses (to prove it's not the old version), then run the code from step 2.

Changed January 06, 2012 09:29PM UTC by fam.lam comment:3

It seems to be this code that hangs:

/((?:\\((?:\\([^()]+\\)|[^()]+)+\\)|\\[(?:\\[[^\\[\\]]*\\]|['"][^'"]*['"]|[^\\[\\]'"]+)+\\]|\\\\.|[^ >+~,(\\[\\\\]+)+|[>+~])(\\s*,\\s*)?((?:.|\\r|\\n)*)/g.exec("[onclick^=\\"window.open('http://www.firstload.de/\\"]");

The actual line where this happens is

m = chunker.exec( soFar );

Changed January 06, 2012 10:37PM UTC by sindresorhus comment:4

owner: → fam.lam
status: newpending

Thanks for taking the time to contribute to the jQuery project! Please provide a complete reduced test case on jsFiddle to help us assess your ticket.

Additionally, be sure to test against the jQuery Edge version to ensure the issue still exists. To get you started, use this boilerplate: http://jsfiddle.net/FrKyN/ Open the link and click to "Fork" (in the top menu) to get started.

Changed January 07, 2012 01:13AM UTC by fam.lam comment:5

status: pendingnew

Even if I just copy-paste the full web page and required reproduction steps into jsfiddle (removed the comments and made the links absolute: http://jsfiddle.net/EWPLb/5/ and used the latest jQuery or the 1.7.1 release), I do not get any hangs. (Don't ask me why, the steps above cause it 100% of the time)

However, you might want to have a look at

http://code.google.com/p/v8/issues/detail?id=1888

(The issue tracker for Chromes javascript kit V8). It explains exactly why it can hang in Chrome, but doesn't in Safari or FireFox: the regex too many repeating quantifiers, which may hang for quite a while.

The regex proposed by that V8 programmer

/((?:\\((?:\\([^()]+\\)|[^()])+\\)|\\[(?:\\[[^\\[\\]]*\\]|['"][^'"]*['"]|[^\\[\\]'"])+\\]|\\\\.|[^ >+~,(\\[\\\\])+|[>+~])(\\s*,\\s*)?((?:.|\\r|\\n)*)/g

indeed does the same thing. (Verified manually and I've also ran it through the jQuery test suite which told me there were no errors:

''Tests completed in 45812 milliseconds.

4919 tests of 4919 passed, 0 failed.''

The only change is in the three +)+ blocks, which are (as explained by that programmer) not necessarily (the blocks are all non-capturing).

Changed January 08, 2012 07:12PM UTC by fam.lam comment:6

Or do I have to report this to Sizzle?

This code seems to reproduce it 100% of the time (if Sizzle is in your current directory)?

<html><head><script src="sizzle.js"></script><script>
Sizzle("[onclick^*=\\"window.open('http://www.firstload.de/\\"]");
alert('Done');
</script></head><body></body></html>

Changed January 08, 2012 09:09PM UTC by rwaldron comment:7

indeed does the same thing. (Verified manually and I've also ran it through the jQuery test suite which told me there were no errors: ''Tests completed in 45812 milliseconds. 4919 tests of 4919 passed, 0 failed.''

Something isn't right, because jQuery should run:

Tests completed in 49876 milliseconds.
5876 tests of 5876 passed, 0 failed.

Changed January 08, 2012 10:20PM UTC by fam.lam comment:8

Hmmm... Can't get it higher than 4919. I ran the index.html file in /test/ after

1. downloading the full repository from https://github.com/jquery/jquery/downloads

2. adding the missing files that showed up in index.html's console (qunit.js/css from http://docs.jquery.com/QUnit, the two sizzle files from sizzles repository)

3. replace the incorrect regexes with the proposed fix

4. run index.html -> 4919 tests...

What do I do wrong?

Also, should I report it to sizzle as it is reproducable 100% of the time for sizzle itself?

Changed January 09, 2012 02:02AM UTC by rwaldron comment:9

Are you running it from file:// ? If so, that would explain the short numbers. When the suite is run from file:// several tests are switch "off"

Changed January 09, 2012 06:18AM UTC by fam.lam comment:10

Yes, I am. I do not have some server to put them on.

Changed January 09, 2012 01:27PM UTC by rwaldron comment:11

You can easily run a localhost server, but that's irrelevant to this issue

Changed January 09, 2012 03:25PM UTC by timmywil comment:12

component: unfiledselector
priority: undecidedlow
resolution: → invalid
status: newclosed

A few things here:

1. If the bug cannot be reproduced on jsfiddle, it is usually not a jQuery issue but an issue with user's environment.

2. We shouldn't spend too much time verifying selectors that use on* attributes, especially the onclick attribute.

3. onclick^*=... is invalid. Either use ^= or *=, but not both. This could be the cause of the loop.

Changed January 09, 2012 03:43PM UTC by fam.lam comment:13

You didn't read comment 2...

Filed it to Sizzle itself, as it's 100% reproducible for them.

Changed January 09, 2012 03:48PM UTC by fam.lam comment:14

Ok... have to apologize. Copied the wrong one again :(

Changed January 09, 2012 03:57PM UTC by timmywil comment:15

Neither selector is valid, but the reason is less obvious for the selector in comment two. To escape any character, one must insert two backslashes before the character that needs to be escaped (such as parenthesis and periods):

var selector = '[onclick^="window\\\\.open\\\\(\\'http:\\\\/\\\\/www\\\\.firstload\\\\.de\\\\/"]'

These are the same escaping rules that apply to querySelectorAll:

http://www.w3.org/TR/CSS21/syndata.html#characters