Changeset 206744 in webkit

Timestamp:

Oct 3, 2016 1:27:33 PM (8 years ago)

Author:

aestes@apple.com

Message:

ASSERTION FAILED: url.containsOnlyASCII() in WebCore::checkEncodedString() when parsing an invalid CSS cursor URL
​https://bugs.webkit.org/show_bug.cgi?id=162763
<rdar://problem/28572758>

Reviewed by Youenn Fablet.

Source/WebCore:

CSSCursorImageValue copies the URL of its underlying CSSImageValue by using the
ParsedURLString URL constructor on the String returned by CSSImageValue::url(). While
CSSImageValues were always being constructed from a URL implicitly converted to a String,
nothing ensured that the URL was valid. For invalid URLs, URL::string() returns the string
it was constructed with, which might still represent a relative URL or contain non-ASCII
characters, violating the preconditions of the ParsedURLString URL constructor and causing
an assertion to fail in Debug builds.

Fix this by having CSSImageValue store its image URL using a WebCore::URL rather than a
String. CSSCursorImageValue can then copy this URL instead of attempting to re-parse a
possibly-invalid URL string.

Test: fast/css/cursor-with-invalid-url.html

• css/CSSCursorImageValue.cpp:

(WebCore::CSSCursorImageValue::CSSCursorImageValue): Copied m_imageValue.url() into
m_originalURL instead of using the ParsedURLString URL constructor, since
CSSImageValue::url() now returns a WebCore::URL.
(WebCore::CSSCursorImageValue::loadImage): Created a URL from cursorElement->href() by
calling Document::completeURL().

• css/CSSImageValue.cpp:

(WebCore::CSSImageValue::CSSImageValue): Changed to take a URL&& instead of a const String&.
(WebCore::CSSImageValue::loadImage): Stopped calling Document::completeURL(), since m_url is
now a WebCore::URL.

• css/CSSImageValue.h: Changed url() to return a const URL&, and changed m_url to be a URL.
• html/HTMLBodyElement.cpp:

(WebCore::HTMLBodyElement::collectStyleForPresentationAttribute): Removed a call to
URL::string().

• html/HTMLTableElement.cpp:

(WebCore::HTMLTableElement::collectStyleForPresentationAttribute): Ditto.

• html/HTMLTablePartElement.cpp:

(WebCore::HTMLTablePartElement::collectStyleForPresentationAttribute): Ditto.

LayoutTests:

• fast/css/cursor-with-invalid-url.html: Added.
• fast/css/cursor-with-invalid-url-expected.txt: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css/cursor-with-invalid-url-expected.txt (added)
• LayoutTests/fast/css/cursor-with-invalid-url.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/CSSCursorImageValue.cpp (modified) (2 diffs)
• Source/WebCore/css/CSSImageValue.cpp (modified) (2 diffs)
• Source/WebCore/css/CSSImageValue.h (modified) (6 diffs)
• Source/WebCore/html/HTMLBodyElement.cpp (modified) (1 diff)
• Source/WebCore/html/HTMLTableElement.cpp (modified) (1 diff)
• Source/WebCore/html/HTMLTablePartElement.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-10-03  Andy Estes  <aestes@apple.com>
+
+        ASSERTION FAILED: url.containsOnlyASCII() in WebCore::checkEncodedString() when parsing an invalid CSS cursor URL
+        https://bugs.webkit.org/show_bug.cgi?id=162763
+        <rdar://problem/28572758>
+
+        Reviewed by Youenn Fablet.
+
+        * fast/css/cursor-with-invalid-url.html: Added.
+        * fast/css/cursor-with-invalid-url-expected.txt: Added.
+
 2016-10-03  Andy Estes  <aestes@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-10-03  Andy Estes  <aestes@apple.com>
+
+        ASSERTION FAILED: url.containsOnlyASCII() in WebCore::checkEncodedString() when parsing an invalid CSS cursor URL
+        https://bugs.webkit.org/show_bug.cgi?id=162763
+        <rdar://problem/28572758>
+
+        Reviewed by Youenn Fablet.
+
+        CSSCursorImageValue copies the URL of its underlying CSSImageValue by using the
+        ParsedURLString URL constructor on the String returned by CSSImageValue::url(). While
+        CSSImageValues were always being constructed from a URL implicitly converted to a String,
+        nothing ensured that the URL was valid. For invalid URLs, URL::string() returns the string
+        it was constructed with, which might still represent a relative URL or contain non-ASCII
+        characters, violating the preconditions of the ParsedURLString URL constructor and causing
+        an assertion to fail in Debug builds.
+
+        Fix this by having CSSImageValue store its image URL using a WebCore::URL rather than a
+        String. CSSCursorImageValue can then copy this URL instead of attempting to re-parse a
+        possibly-invalid URL string.
+
+        Test: fast/css/cursor-with-invalid-url.html
+
+        * css/CSSCursorImageValue.cpp:
+        (WebCore::CSSCursorImageValue::CSSCursorImageValue): Copied m_imageValue.url() into
+        m_originalURL instead of using the ParsedURLString URL constructor, since
+        CSSImageValue::url() now returns a WebCore::URL.
+        (WebCore::CSSCursorImageValue::loadImage): Created a URL from cursorElement->href() by
+        calling Document::completeURL().
+        * css/CSSImageValue.cpp:
+        (WebCore::CSSImageValue::CSSImageValue): Changed to take a URL&& instead of a const String&.
+        (WebCore::CSSImageValue::loadImage): Stopped calling Document::completeURL(), since m_url is
+        now a WebCore::URL.
+        * css/CSSImageValue.h: Changed url() to return a const URL&, and changed m_url to be a URL.
+        * html/HTMLBodyElement.cpp:
+        (WebCore::HTMLBodyElement::collectStyleForPresentationAttribute): Removed a call to
+        URL::string().
+        * html/HTMLTableElement.cpp:
+        (WebCore::HTMLTableElement::collectStyleForPresentationAttribute): Ditto.
+        * html/HTMLTablePartElement.cpp:
+        (WebCore::HTMLTablePartElement::collectStyleForPresentationAttribute): Ditto.
+
 2016-10-03  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/css/CSSCursorImageValue.cpp

@@ -45 +45 @@
 {
     if (is<CSSImageValue>(m_imageValue.get()))
-        m_originalURL = { ParsedURLString, downcast<CSSImageValue>(m_imageValue.get()).url() };
+        m_originalURL = downcast<CSSImageValue>(m_imageValue.get()).url();
 }
 
@@ -108 +108 @@
     if (auto* cursorElement = updateCursorElement(*loader.document())) {
         if (cursorElement->href() != downcast<CSSImageValue>(m_imageValue.get()).url())
-            m_imageValue = CSSImageValue::create(cursorElement->href());
+            m_imageValue = CSSImageValue::create(loader.document()->completeURL(cursorElement->href()));
     }
 

trunk/Source/WebCore/css/CSSImageValue.cpp

@@ -36 +36 @@
 namespace WebCore {
 
-CSSImageValue::CSSImageValue(const String& url)
+CSSImageValue::CSSImageValue(URL&& url)
     : CSSValue(ImageClass)
-    , m_url(url)
+    , m_url(WTFMove(url))
     , m_accessedImage(false)
 {
@@ -66 +66 @@
         m_accessedImage = true;
 
-        CachedResourceRequest request(ResourceRequest(loader.document()->completeURL(m_url)), options);
+        CachedResourceRequest request(ResourceRequest(m_url), options);
         if (m_initiatorName.isEmpty())
             request.setInitiator(cachedResourceRequestInitiators().css);

trunk/Source/WebCore/css/CSSImageValue.h

@@ -19 +19 @@
  */
 
-#ifndef CSSImageValue_h
-#define CSSImageValue_h
+#pragma once
 
 #include "CSSValue.h"
 #include "CachedResourceHandle.h"
-#include <wtf/RefPtr.h>
+#include <wtf/Ref.h>
 
 namespace WebCore {
@@ -30 +29 @@
 class CachedImage;
 class CachedResourceLoader;
-class Element;
 class RenderElement;
 struct ResourceLoaderOptions;
@@ -36 +34 @@
 class CSSImageValue final : public CSSValue {
 public:
-    static Ref<CSSImageValue> create(const String& url) { return adoptRef(*new CSSImageValue(url)); }
+    static Ref<CSSImageValue> create(URL&& url) { return adoptRef(*new CSSImageValue(WTFMove(url))); }
     static Ref<CSSImageValue> create(CachedImage& image) { return adoptRef(*new CSSImageValue(image)); }
     ~CSSImageValue();
@@ -44 +42 @@
     CachedImage* cachedImage() const { return m_cachedImage.get(); }
 
-    const String& url() const { return m_url; }
+    const URL& url() const { return m_url; }
 
     String customCSSText() const;
@@ -59 +57 @@
 
 private:
-    explicit CSSImageValue(const String& url);
+    explicit CSSImageValue(URL&&);
     explicit CSSImageValue(CachedImage&);
 
-    String m_url;
+    URL m_url;
     CachedResourceHandle<CachedImage> m_cachedImage;
     bool m_accessedImage;
@@ -71 +69 @@
 
 SPECIALIZE_TYPE_TRAITS_CSS_VALUE(CSSImageValue, isImageValue())
-
-#endif // CSSImageValue_h

trunk/Source/WebCore/html/HTMLBodyElement.cpp

@@ -83 +83 @@
         String url = stripLeadingAndTrailingHTMLSpaces(value);
         if (!url.isEmpty()) {
-            auto imageValue = CSSImageValue::create(document().completeURL(url).string());
+            auto imageValue = CSSImageValue::create(document().completeURL(url));
             imageValue.get().setInitiator(localName());
             style.setProperty(CSSProperty(CSSPropertyBackgroundImage, WTFMove(imageValue)));

trunk/Source/WebCore/html/HTMLTableElement.cpp

@@ -333 +333 @@
         String url = stripLeadingAndTrailingHTMLSpaces(value);
         if (!url.isEmpty())
-            style.setProperty(CSSProperty(CSSPropertyBackgroundImage, CSSImageValue::create(document().completeURL(url).string())));
+            style.setProperty(CSSProperty(CSSPropertyBackgroundImage, CSSImageValue::create(document().completeURL(url))));
     } else if (name == valignAttr) {
         if (!value.isEmpty())

trunk/Source/WebCore/html/HTMLTablePartElement.cpp

@@ -53 +53 @@
         String url = stripLeadingAndTrailingHTMLSpaces(value);
         if (!url.isEmpty())
-            style.setProperty(CSSProperty(CSSPropertyBackgroundImage, CSSImageValue::create(document().completeURL(url).string())));
+            style.setProperty(CSSProperty(CSSPropertyBackgroundImage, CSSImageValue::create(document().completeURL(url))));
     } else if (name == valignAttr) {
         if (equalLettersIgnoringASCIICase(value, "top"))