Bug Tracker

Ticket #11137 (closed bug: invalid)

Opened 3 years ago

Last modified 3 years ago

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:
Blocking: Blocked by:

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)

Change History

comment:1 Changed 3 years ago by fam.lam

*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.

comment:2 Changed 3 years ago by fam.lam

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...
Version 0, edited 3 years ago by fam.lam (next)

comment:3 Changed 3 years ago by fam.lam

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 );

comment:4 Changed 3 years ago by sindresorhus

  • Owner set to fam.lam
  • Status changed from new to 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.

comment:5 follow-up: ↓ 7 Changed 3 years ago by fam.lam

  • Status changed from pending to 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).

comment:6 Changed 3 years ago by fam.lam

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>

comment:7 in reply to: ↑ 5 Changed 3 years ago by rwaldron

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.

comment:8 Changed 3 years ago by fam.lam

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?

comment:9 Changed 3 years ago by rwaldron

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"

comment:10 Changed 3 years ago by fam.lam

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

comment:11 Changed 3 years ago by rwaldron

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

comment:12 Changed 3 years ago by timmywil

  • Priority changed from undecided to low
  • Resolution set to invalid
  • Status changed from new to closed
  • Component changed from unfiled to selector

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.

comment:13 Changed 3 years ago by fam.lam

You didn't read comment 2... Filed it to Sizzle itself, as it's 100% reproducible for them.

comment:14 Changed 3 years ago by fam.lam

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

comment:15 Changed 3 years ago by timmywil

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

Note: See TracTickets for help on using tickets.