[webkit-changes] [206744] trunk

Mon, 03 Oct 2016 13:27:07 -0700

Title: [206744] trunk
Revision
206744
Author
[email protected]
Date
2016-10-03 13:27:33 -0700 (Mon, 03 Oct 2016)

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

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (206743 => 206744)


--- trunk/LayoutTests/ChangeLog	2016-10-03 20:26:00 UTC (rev 206743)
+++ trunk/LayoutTests/ChangeLog	2016-10-03 20:27:33 UTC (rev 206744)
@@ -1,5 +1,16 @@
 2016-10-03  Andy Estes  <[email protected]>
 
+        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  <[email protected]>
+
         ASSERTION FAILED: result in WebCore::CSSParser::parseURI
         https://bugs.webkit.org/show_bug.cgi?id=141638
         <rdar://problem/27709952>

Added: trunk/LayoutTests/fast/css/cursor-with-invalid-url-expected.txt (0 => 206744)


--- trunk/LayoutTests/fast/css/cursor-with-invalid-url-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/css/cursor-with-invalid-url-expected.txt	2016-10-03 20:27:33 UTC (rev 206744)
@@ -0,0 +1 @@
+Test that a cursor with an invalid CSS URL does not trigger an assertion in a Debug build.

Added: trunk/LayoutTests/fast/css/cursor-with-invalid-url.html (0 => 206744)


--- trunk/LayoutTests/fast/css/cursor-with-invalid-url.html	                        (rev 0)
+++ trunk/LayoutTests/fast/css/cursor-with-invalid-url.html	2016-10-03 20:27:33 UTC (rev 206744)
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script>
+        if (window.testRunner)
+            testRunner.dumpAsText();
+    </script>
+    <style>
+        body {
+            cursor: url(scheme://host:80\ff);
+        }
+    </style>
+</head>
+<body>
+Test that a cursor with an invalid CSS URL does not trigger an assertion in a Debug build.
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (206743 => 206744)


--- trunk/Source/WebCore/ChangeLog	2016-10-03 20:26:00 UTC (rev 206743)
+++ trunk/Source/WebCore/ChangeLog	2016-10-03 20:27:33 UTC (rev 206744)
@@ -1,3 +1,44 @@
+2016-10-03  Andy Estes  <[email protected]>
+
+        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  <[email protected]>
 
         Log an error to stderr when FrameView::layout() fails to clean all the renderers.

Modified: trunk/Source/WebCore/css/CSSCursorImageValue.cpp (206743 => 206744)


--- trunk/Source/WebCore/css/CSSCursorImageValue.cpp	2016-10-03 20:26:00 UTC (rev 206743)
+++ trunk/Source/WebCore/css/CSSCursorImageValue.cpp	2016-10-03 20:27:33 UTC (rev 206744)
@@ -44,7 +44,7 @@
     , m_hotSpot(hotSpot)
 {
     if (is<CSSImageValue>(m_imageValue.get()))
-        m_originalURL = { ParsedURLString, downcast<CSSImageValue>(m_imageValue.get()).url() };
+        m_originalURL = downcast<CSSImageValue>(m_imageValue.get()).url();
 }
 
 CSSCursorImageValue::~CSSCursorImageValue()
@@ -107,7 +107,7 @@
 
     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()));
     }
 
     return { downcast<CSSImageValue>(m_imageValue.get()).loadImage(loader, options), 1 };

Modified: trunk/Source/WebCore/css/CSSImageValue.cpp (206743 => 206744)


--- trunk/Source/WebCore/css/CSSImageValue.cpp	2016-10-03 20:26:00 UTC (rev 206743)
+++ trunk/Source/WebCore/css/CSSImageValue.cpp	2016-10-03 20:27:33 UTC (rev 206744)
@@ -35,9 +35,9 @@
 
 namespace WebCore {
 
-CSSImageValue::CSSImageValue(const String& url)
+CSSImageValue::CSSImageValue(URL&& url)
     : CSSValue(ImageClass)
-    , m_url(url)
+    , m_url(WTFMove(url))
     , m_accessedImage(false)
 {
 }
@@ -65,7 +65,7 @@
     if (!m_accessedImage) {
         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);
         else

Modified: trunk/Source/WebCore/css/CSSImageValue.h (206743 => 206744)


--- trunk/Source/WebCore/css/CSSImageValue.h	2016-10-03 20:26:00 UTC (rev 206743)
+++ trunk/Source/WebCore/css/CSSImageValue.h	2016-10-03 20:27:33 UTC (rev 206744)
@@ -18,24 +18,22 @@
  * Boston, MA 02110-1301, USA.
  */
 
-#ifndef CSSImageValue_h
-#define CSSImageValue_h
+#pragma once
 
 #include "CSSValue.h"
 #include "CachedResourceHandle.h"
-#include <wtf/RefPtr.h>
+#include <wtf/Ref.h>
 
 namespace WebCore {
 
 class CachedImage;
 class CachedResourceLoader;
-class Element;
 class RenderElement;
 struct ResourceLoaderOptions;
 
 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();
 
@@ -43,7 +41,7 @@
     CachedImage* loadImage(CachedResourceLoader&, const ResourceLoaderOptions&);
     CachedImage* cachedImage() const { return m_cachedImage.get(); }
 
-    const String& url() const { return m_url; }
+    const URL& url() const { return m_url; }
 
     String customCSSText() const;
 
@@ -58,10 +56,10 @@
     void setInitiator(const AtomicString& name) { m_initiatorName = name; }
 
 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;
     AtomicString m_initiatorName;
@@ -70,5 +68,3 @@
 } // namespace WebCore
 
 SPECIALIZE_TYPE_TRAITS_CSS_VALUE(CSSImageValue, isImageValue())
-
-#endif // CSSImageValue_h

Modified: trunk/Source/WebCore/html/HTMLBodyElement.cpp (206743 => 206744)


--- trunk/Source/WebCore/html/HTMLBodyElement.cpp	2016-10-03 20:26:00 UTC (rev 206743)
+++ trunk/Source/WebCore/html/HTMLBodyElement.cpp	2016-10-03 20:27:33 UTC (rev 206744)
@@ -82,7 +82,7 @@
     if (name == backgroundAttr) {
         String url = ""
         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)));
         }

Modified: trunk/Source/WebCore/html/HTMLTableElement.cpp (206743 => 206744)


--- trunk/Source/WebCore/html/HTMLTableElement.cpp	2016-10-03 20:26:00 UTC (rev 206743)
+++ trunk/Source/WebCore/html/HTMLTableElement.cpp	2016-10-03 20:27:33 UTC (rev 206744)
@@ -332,7 +332,7 @@
     else if (name == backgroundAttr) {
         String url = ""
         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())
             addPropertyToPresentationAttributeStyle(style, CSSPropertyVerticalAlign, value);

Modified: trunk/Source/WebCore/html/HTMLTablePartElement.cpp (206743 => 206744)


--- trunk/Source/WebCore/html/HTMLTablePartElement.cpp	2016-10-03 20:26:00 UTC (rev 206743)
+++ trunk/Source/WebCore/html/HTMLTablePartElement.cpp	2016-10-03 20:27:33 UTC (rev 206744)
@@ -52,7 +52,7 @@
     else if (name == backgroundAttr) {
         String url = ""
         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"))
             addPropertyToPresentationAttributeStyle(style, CSSPropertyVerticalAlign, CSSValueTop);

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to