Skip to content

Commit

Permalink
When detecting html in init, ignore html characters within quotes, br…
Browse files Browse the repository at this point in the history
…ackets, and parens as well as escaped characters which are valid in selectors. Fixes #11290.
  • Loading branch information
timmywil committed Jun 19, 2012
1 parent 868a9ce commit 7692ae4
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
3 changes: 2 additions & 1 deletion src/core.js
Expand Up @@ -41,7 +41,8 @@ var

// A simple way to check for HTML strings
// Prioritize #id over <tag> to avoid XSS via location.hash (#9521)
rhtmlString = /^(?:[^#<]*(<[\w\W]+>)[^>]*$)/,
// Ignore html if within quotes "" '' or brackets/parens [] ()
rhtmlString = /^(?:[^#<\\]*(<[\w\W]+>)(?![^\[]*\])(?![^\(]*\))(?![^']*')(?![^"]*")[^>]*$)/,

// Match a standalone tag
rsingleTag = /^<(\w+)\s*\/?>(?:<\/\1>)?$/,
Expand Down
7 changes: 6 additions & 1 deletion test/unit/core.js
Expand Up @@ -605,7 +605,7 @@ test("isWindow", function() {
});

test("jQuery('html')", function() {
expect(18);
expect( 22 );

QUnit.reset();
jQuery.foo = false;
Expand Down Expand Up @@ -638,6 +638,11 @@ test("jQuery('html')", function() {
ok( jQuery("<div></div>")[0], "Create a div with closing tag." );
ok( jQuery("<table></table>")[0], "Create a table with closing tag." );

equal( jQuery("element[attribute='<div></div>']").length, 0, "When html is within brackets, do not recognize as html." );
equal( jQuery("element[attribute=<div></div>]").length, 0, "When html is within brackets, do not recognize as html." );
equal( jQuery("element:not(<div></div>)").length, 0, "When html is within parens, do not recognize as html." );
equal( jQuery("\\<div\\>").length, 0, "Ignore escaped html characters" );

// Test very large html string #7990
var i;
var li = "<li>very large html string</li>";
Expand Down

8 comments on commit 7692ae4

@gibson042
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it's gone too far the other way... http://jsfiddle.net/HCa89/1/

@timmywil
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timmywil
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edited link

@timmywil
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, it would be an issue with something like: http://jsfiddle.net/timmywil/HCa89/4/. Let's see if we can modify.

@gibson042
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regular expressions cannot match context-free grammars. I'm pretty sure the task is impossible; we must give up either rhtmlString or backwards compatibility.

@davidmurdoch
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timmywil
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gibson042: I don't think that's necessarily true. There is a middle ground. The current behavior allows unexpected things to happen. All we need to do is remove those, which will both avoid the "starts-with" technique and maintain backwards compatibility (meaning we continue to support what we meant to support, not necessarily what was accidentally allowed, such as elem[attr="<div/>"]). I think we can agree these don't need to be supported. It sounds like you are more in favor of the even more strict rule, starts-with, but I think this would cause even more problems. Here's another pass, it is closer to the behavior I'm looking for, but size is a concern: 041858ecc

@gibson042
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reply there.

Please sign in to comment.