Attachment 349578 Details for Bug 189554 – Patch

[patch] Patch

bug-189554-20180912134726.patch (text/plain), 6.31 KB, created by Alex Christensen on 2018-09-12 13:47:27 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Christensen

Created: 2018-09-12 13:47:27 PDT

Size: 6.31 KB

patch

obsolete

>Index: Source/WTF/ChangeLog
>===================================================================
>--- Source/WTF/ChangeLog	(revision 235943)
>+++ Source/WTF/ChangeLog	(working copy)
>@@ -1,3 +1,14 @@
>+2018-09-12  Alex Christensen  <achristensen@webkit.org>
>+
>+        URL::string should return a null String if parsing failed
>+        https://bugs.webkit.org/show_bug.cgi?id=189554
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * wtf/text/WTFString.cpp:
>+        (WTF::nullString):
>+        * wtf/text/WTFString.h:
>+
> 2018-09-12  Guillaume Emont  <guijemont@igalia.com>
> 
>         Add IGNORE_WARNING_.* macros
>Index: Source/WTF/wtf/text/WTFString.cpp
>===================================================================
>--- Source/WTF/wtf/text/WTFString.cpp	(revision 235942)
>+++ Source/WTF/wtf/text/WTFString.cpp	(working copy)
>@@ -1212,6 +1212,12 @@ const String& emptyString()
>     return emptyString;
> }
> 
>+const String& nullString()
>+{
>+    static NeverDestroyed<String> nullString;
>+    return nullString;
>+}
>+
> } // namespace WTF
> 
> #ifndef NDEBUG
>Index: Source/WTF/wtf/text/WTFString.h
>===================================================================
>--- Source/WTF/wtf/text/WTFString.h	(revision 235942)
>+++ Source/WTF/wtf/text/WTFString.h	(working copy)
>@@ -424,6 +424,7 @@ template<typename CharacterType> void ap
> 
> // Shared global empty string.
> WTF_EXPORT_PRIVATE const String& emptyString();
>+WTF_EXPORT_PRIVATE const String& nullString();
> 
> template<typename> struct DefaultHash;
> template<> struct DefaultHash<String> { using Hash = StringHash; };
>@@ -652,6 +653,7 @@ using WTF::equal;
> using WTF::find;
> using WTF::isAllSpecialCharacters;
> using WTF::isSpaceOrNewline;
>+using WTF::nullString;
> using WTF::reverseFind;
> 
> #include <wtf/text/AtomicString.h>
>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 235942)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,23 @@
>+2018-09-12  Alex Christensen  <achristensen@webkit.org>
>+
>+        URL::string should return a null String if parsing failed
>+        https://bugs.webkit.org/show_bug.cgi?id=189554
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        No intentional change in behavior, but if we are misusing isNull/isEmpty/isValid, this could prevent
>+        security issues where we are using invalid URL strings as valid URLs.
>+        We need to keep the invalid String around, though, for the href of anchor elements, so I made a new accessor for that.
>+
>+        * html/URLUtils.h:
>+        (WebCore::URLUtils<T>::toString const):
>+        * platform/URL.h:
>+        (WebCore::URL::isNull const):
>+        (WebCore::URL::isEmpty const):
>+        (WebCore::URL::isValid const):
>+        (WebCore::URL::string const):
>+        (WebCore::URL::stringMaybeInvalid const):
>+
> 2018-09-12  Fujii Hironori  <Hironori.Fujii@sony.com>
> 
>         [Win][Clang] error: non-constant-expression cannot be narrowed from type 'int' to 'SHORT'
>Index: Source/WebCore/html/URLUtils.h
>===================================================================
>--- Source/WebCore/html/URLUtils.h	(revision 235942)
>+++ Source/WebCore/html/URLUtils.h	(working copy)
>@@ -71,7 +71,7 @@ public:
> template <typename T>
> String URLUtils<T>::toString() const
> {
>-    return href().string();
>+    return href().stringMaybeInvalid();
> }
> 
> template <typename T>
>Index: Source/WebCore/platform/URL.h
>===================================================================
>--- Source/WebCore/platform/URL.h	(revision 235942)
>+++ Source/WebCore/platform/URL.h	(working copy)
>@@ -88,9 +88,9 @@ public:
>     // no other reason to ever prefer isolatedCopy() over plain old assignment.
>     WEBCORE_EXPORT URL isolatedCopy() const;
> 
>-    bool isNull() const;
>-    bool isEmpty() const;
>-    bool isValid() const;
>+    bool isNull() const { return !m_isValid; }
>+    bool isEmpty() const { return !m_isValid; }
>+    bool isValid() const { return m_isValid; }
> 
>     // Returns true if you can set the host and port for the URL.
>     // Non-hierarchical URLs don't have a host and port.
>@@ -99,7 +99,8 @@ public:
>     bool canSetPathname() const { return isHierarchical(); }
>     bool isHierarchical() const;
> 
>-    const String& string() const { return m_string; }
>+    const String& string() const { return m_isValid ? m_string : nullString(); }
>+    const String& stringMaybeInvalid() const { return m_string; }
> 
>     WEBCORE_EXPORT String stringCenterEllipsizedToLength(unsigned length = 1024) const;
> 
>@@ -346,24 +347,6 @@ inline bool operator!=(const String& a, 
>     return a != b.string();
> }
> 
>-// Inline versions of some non-GoogleURL functions so we can get inlining
>-// without having to have a lot of ugly ifdefs in the class definition.
>-
>-inline bool URL::isNull() const
>-{
>-    return m_string.isNull();
>-}
>-
>-inline bool URL::isEmpty() const
>-{
>-    return m_string.isEmpty();
>-}
>-
>-inline bool URL::isValid() const
>-{
>-    return m_isValid;
>-}
>-
> inline bool URL::hasPath() const
> {
>     return m_pathEnd != m_hostEnd + m_portLength;
>Index: Source/WebCore/platform/URLHash.h
>===================================================================
>--- Source/WebCore/platform/URLHash.h	(revision 235942)
>+++ Source/WebCore/platform/URLHash.h	(working copy)
>@@ -23,8 +23,7 @@
>  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
>  */
> 
>-#ifndef URLHash_h
>-#define URLHash_h
>+#pragma once
> 
> #include "URL.h"
> #include <wtf/text/StringHash.h>
>@@ -32,19 +31,19 @@
> 
> namespace WebCore {
> 
>-    struct URLHash {
>-        static unsigned hash(const URL& key)
>-        {
>-            return key.string().impl()->hash();
>-        }
>-
>-        static bool equal(const URL& a, const URL& b)
>-        {
>-            return StringHash::equal(a.string(), b.string());
>-        }
>+struct URLHash {
>+    static unsigned hash(const URL& key)
>+    {
>+        return key.stringMaybeInvalid().impl()->hash();
>+    }
>+
>+    static bool equal(const URL& a, const URL& b)
>+    {
>+        return StringHash::equal(a.stringMaybeInvalid(), b.stringMaybeInvalid());
>+    }
> 
>-        static const bool safeToCompareToEmptyOrDeleted = false;
>-    };
>+    static const bool safeToCompareToEmptyOrDeleted = false;
>+};
> 
> } // namespace WebCore
> 
>@@ -58,5 +57,3 @@ template<> struct DefaultHash<WebCore::U
> template<> struct HashTraits<WebCore::URL> : SimpleClassHashTraits<WebCore::URL> { };
> 
> } // namespace WTF
>-
>-#endif // URLHash_h

Flags:

achristensen: review-
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 189554: 349563 | 349564 | 349565 | 349567 | 349568 | 349572 | 349578 | 349584 | 349585