Changeset 141686 in webkit

Timestamp:

Feb 2, 2013 12:25:21 AM (11 years ago)

Author:

tonyg@chromium.org

Message:

Continue making XSSAuditor thread safe: Remove unsafe AtomicString compares
​https://bugs.webkit.org/show_bug.cgi?id=108557

Reviewed by Adam Barth.

Unfortunately HTMLNames comparisons will always be false on a non-main thread
with our current design, so we have to use some "threadSafeMatch" helpers written
for the HTMLBackgroundParser.

Also factor out threadSafeMatch() methods to HTMLParserIdioms.

No new tests because no new functionality.

• html/parser/BackgroundHTMLParser.cpp:

(WebCore):

• html/parser/HTMLParserIdioms.cpp:

(WebCore::threadSafeEqual):
(WebCore):
(WebCore::threadSafeMatch):

• html/parser/HTMLParserIdioms.h:

(WebCore):

• html/parser/XSSAuditor.cpp:

(WebCore::XSSAuditor::eraseAttributeIfInjected):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/parser/BackgroundHTMLParser.cpp (modified) (2 diffs)
• html/parser/HTMLParserIdioms.cpp (modified) (2 diffs)
• html/parser/HTMLParserIdioms.h (modified) (2 diffs)
• html/parser/XSSAuditor.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-02-02  Tony Gentilcore  <tonyg@chromium.org>
+
+        Continue making XSSAuditor thread safe: Remove unsafe AtomicString compares
+        https://bugs.webkit.org/show_bug.cgi?id=108557
+
+        Reviewed by Adam Barth.
+
+        Unfortunately HTMLNames comparisons will always be false on a non-main thread
+        with our current design, so we have to use some "threadSafeMatch" helpers written
+        for the HTMLBackgroundParser.
+
+        Also factor out threadSafeMatch() methods to HTMLParserIdioms.
+
+        No new tests because no new functionality.
+
+        * html/parser/BackgroundHTMLParser.cpp:
+        (WebCore):
+        * html/parser/HTMLParserIdioms.cpp:
+        (WebCore::threadSafeEqual):
+        (WebCore):
+        (WebCore::threadSafeMatch):
+        * html/parser/HTMLParserIdioms.h:
+        (WebCore):
+        * html/parser/XSSAuditor.cpp:
+        (WebCore::XSSAuditor::eraseAttributeIfInjected):
+
 2013-02-01  James Simonsen  <simonjam@chromium.org>
 

trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp

@@ -32 +32 @@
 #include "HTMLDocumentParser.h"
 #include "HTMLNames.h"
+#include "HTMLParserIdioms.h"
 #include "HTMLParserThread.h"
 #include "HTMLTokenizer.h"
@@ -56 +57 @@
 // FIXME: Tune this constant based on a benchmark. The current value was choosen arbitrarily.
 static const size_t pendingTokenLimit = 4000;
-
-static bool threadSafeEqual(StringImpl* a, StringImpl* b)
-{
-    if (a->hash() != b->hash())
-        return false;
-    return StringHash::equal(a, b);
-}
-
-static bool threadSafeMatch(const String& localName, const QualifiedName& qName)
-{
-    return threadSafeEqual(localName.impl(), qName.localName().impl());
-}
 
 ParserMap& parserMap()

trunk/Source/WebCore/html/parser/HTMLParserIdioms.cpp

@@ -27 +27 @@
 
 #include "Decimal.h"
+#include "QualifiedName.h"
 #include <limits>
 #include <wtf/MathExtras.h>
 #include <wtf/text/AtomicString.h>
 #include <wtf/text/StringBuilder.h>
+#include <wtf/text/StringHash.h>
 
 namespace WebCore {
@@ -276 +278 @@
 }
 
-}
+static bool threadSafeEqual(StringImpl* a, StringImpl* b)
+{
+    if (a->hash() != b->hash())
+        return false;
+    return StringHash::equal(a, b);
+}
+
+bool threadSafeMatch(const QualifiedName& a, const QualifiedName& b)
+{
+    return threadSafeEqual(a.localName().impl(), b.localName().impl());
+}
+
+bool threadSafeMatch(const String& localName, const QualifiedName& qName)
+{
+    return threadSafeEqual(localName.impl(), qName.localName().impl());
+}
+
+}

trunk/Source/WebCore/html/parser/HTMLParserIdioms.h

@@ -32 +32 @@
 
 class Decimal;
+class QualifiedName;
 
 // Space characters as defined by the HTML specification.
@@ -86 +87 @@
 }
 
+bool threadSafeMatch(const QualifiedName&, const QualifiedName&);
+bool threadSafeMatch(const String&, const QualifiedName&);
+
 }
 

trunk/Source/WebCore/html/parser/XSSAuditor.cpp

@@ -487 +487 @@
         const HTMLToken::Attribute& attribute = token.attributes().at(indexOfAttribute);
         if (isContainedInRequest(decodedSnippetForAttribute(token, attribute, treatment))) {
-            if (attributeName == srcAttr && isLikelySafeResource(String(attribute.m_value.data(), attribute.m_value.size())))
+            if (threadSafeMatch(attributeName, srcAttr) && isLikelySafeResource(String(attribute.m_value.data(), attribute.m_value.size())))
                 return false;
-            if (attributeName == http_equivAttr && !isDangerousHTTPEquiv(String(attribute.m_value.data(), attribute.m_value.size())))
+            if (threadSafeMatch(attributeName, http_equivAttr) && !isDangerousHTTPEquiv(String(attribute.m_value.data(), attribute.m_value.size())))
                 return false;
             token.eraseValueOfAttribute(indexOfAttribute);