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 comment:1
Changed January 06, 2012 08:57PM UTC by 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 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 comment:4
| owner: | → fam.lam |
|---|---|
| status: | new → pending |
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 comment:5
| status: | pending → new |
|---|
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 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 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 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 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 comment:10
Yes, I am. I do not have some server to put them on.
Changed January 09, 2012 01:27PM UTC by comment:11
You can easily run a localhost server, but that's irrelevant to this issue
Changed January 09, 2012 03:25PM UTC by comment:12
| component: | unfiled → selector |
|---|---|
| priority: | undecided → low |
| resolution: | → invalid |
| status: | new → closed |
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 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 comment:14
Ok... have to apologize. Copied the wrong one again :(
Changed January 09, 2012 03:57PM UTC by 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:
*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.