Attachment 368692 Details for Bug 181950 – Patch

[patch] Patch

bug-181950-20190501123341.patch (text/plain), 10.61 KB, created by Jiewen Tan on 2019-05-01 12:33:42 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Jiewen Tan

Created: 2019-05-01 12:33:42 PDT

Size: 10.61 KB

patch

obsolete

>Subversion Revision: 244745
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index dcb5cf3c13c0cd7f1ebdc6ff7ca01e51debf9199..88e76f958e38bed41ccb55f2e1e2a47d14266c8b 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2019-05-01  Jiewen Tan  <jiewen_tan@apple.com>
>+
>+        Move Document::domainIsRegisterable to SecurityOrigin::isMatchingRegistrableDomainSuffix
>+        https://bugs.webkit.org/show_bug.cgi?id=181950
>+        <rdar://problem/43357371>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch moves Document::domainIsRegisterable to SecurityOrigin::isMatchingRegistrableDomainSuffix
>+        to be more aligned with the HTML standard:
>+        https://html.spec.whatwg.org/multipage/origin.html#is-a-registrable-domain-suffix-of-or-is-equal-to.
>+        Besides that, it also removes redundant codes within the original method that is also done in
>+        OriginAccessEntry::matchesOrigin.
>+
>+        Covered by new API tests.
>+
>+        * dom/Document.cpp:
>+        (WebCore::Document::setDomain):
>+        (WebCore::Document::domainIsRegisterable const): Deleted.
>+        * dom/Document.h:
>+        * page/SecurityOrigin.cpp:
>+        (WebCore::SecurityOrigin::isMatchingRegistrableDomainSuffix const):
>+        * page/SecurityOrigin.h:
>+
> 2019-04-29  Chris Dumez  <cdumez@apple.com>
> 
>         User-facing strings should use curly quotes instead of straight
>diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
>index 1794bed75a5bd9600331d2520f26b0ec70cd0f21..8c65a59de7a952bca1d39f7eb641876051e339bd 100644
>--- a/Source/WebCore/dom/Document.cpp
>+++ b/Source/WebCore/dom/Document.cpp
>@@ -135,7 +135,6 @@
> #include "NodeIterator.h"
> #include "NodeRareData.h"
> #include "NodeWithIndex.h"
>-#include "OriginAccessEntry.h"
> #include "OverflowEvent.h"
> #include "PageConsoleClient.h"
> #include "PageGroup.h"
>@@ -4857,49 +4856,6 @@ String Document::domain() const
>     return securityOrigin().domain();
> }
> 
>-bool Document::domainIsRegisterable(const String& newDomain) const
>-{
>-    if (newDomain.isEmpty())
>-        return false;
>-
>-    const String& effectiveDomain = domain();
>-
>-    // If the new domain is the same as the old domain, return true so that
>-    // we still call securityOrigin().setDomainForDOM. This will change the
>-    // security check behavior. For example, if a page loaded on port 8000
>-    // assigns its current domain using document.domain, the page will
>-    // allow other pages loaded on different ports in the same domain that
>-    // have also assigned to access this page.
>-    if (equalIgnoringASCIICase(effectiveDomain, newDomain))
>-        return true;
>-
>-    // e.g. newDomain = webkit.org (10) and domain() = www.webkit.org (14)
>-    unsigned oldLength = effectiveDomain.length();
>-    unsigned newLength = newDomain.length();
>-    if (newLength >= oldLength)
>-        return false;
>-
>-    auto ipAddressSetting = settings().treatIPAddressAsDomain() ? OriginAccessEntry::TreatIPAddressAsDomain : OriginAccessEntry::TreatIPAddressAsIPAddress;
>-    OriginAccessEntry accessEntry { securityOrigin().protocol(), newDomain, OriginAccessEntry::AllowSubdomains, ipAddressSetting };
>-    if (!accessEntry.matchesOrigin(securityOrigin()))
>-        return false;
>-
>-    if (effectiveDomain[oldLength - newLength - 1] != '.')
>-        return false;
>-    if (StringView { effectiveDomain }.substring(oldLength - newLength) != newDomain)
>-        return false;
>-
>-    auto potentialPublicSuffix = newDomain;
>-    if (potentialPublicSuffix.startsWith('.'))
>-        potentialPublicSuffix.remove(0, 1);
>-
>-#if ENABLE(PUBLIC_SUFFIX_LIST)
>-    return !isPublicSuffix(potentialPublicSuffix);
>-#else
>-    return true;
>-#endif
>-}
>-
> ExceptionOr<void> Document::setDomain(const String& newDomain)
> {
>     if (!frame())
>@@ -4917,7 +4873,7 @@ ExceptionOr<void> Document::setDomain(const String& newDomain)
>     if (effectiveDomain.isEmpty())
>         return Exception { SecurityError, "The document has a null effectiveDomain." };
> 
>-    if (!domainIsRegisterable(newDomain))
>+    if (!securityOrigin().isMatchingRegistrableDomainSuffix(newDomain, settings().treatIPAddressAsDomain()))
>         return Exception { SecurityError, "Attempted to use a non-registrable domain." };
> 
>     securityOrigin().setDomainFromDOM(newDomain);
>diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
>index 1417c2e126e10936d9289f4d49de5eb24624cac4..5a5ffc62d539eebe3e331f1b1ec82e6656e0231d 100644
>--- a/Source/WebCore/dom/Document.h
>+++ b/Source/WebCore/dom/Document.h
>@@ -1643,8 +1643,6 @@ private:
> 
>     void platformSuspendOrStopActiveDOMObjects();
> 
>-    bool domainIsRegisterable(const String&) const;
>-
>     void enableTemporaryTimeUserGesture();
> 
>     bool isBodyPotentiallyScrollable(HTMLBodyElement&);
>diff --git a/Source/WebCore/page/SecurityOrigin.cpp b/Source/WebCore/page/SecurityOrigin.cpp
>index ddcc7d2dbde929b1152b65a7a6d4286bb74a45bf..7b7cf2c676b2d4fc87c7b05bb1e4ac47628c6f65 100644
>--- a/Source/WebCore/page/SecurityOrigin.cpp
>+++ b/Source/WebCore/page/SecurityOrigin.cpp
>@@ -30,6 +30,7 @@
> #include "SecurityOrigin.h"
> 
> #include "BlobURL.h"
>+#include "OriginAccessEntry.h"
> #include "SchemeRegistry.h"
> #include "SecurityPolicy.h"
> #include "TextEncoding.h"
>@@ -432,6 +433,27 @@ bool SecurityOrigin::isSameOriginAs(const SecurityOrigin& other) const
>     return isSameSchemeHostPort(other);
> }
> 
>+bool SecurityOrigin::isMatchingRegistrableDomainSuffix(const String& domainSuffix, bool treatIPAddressAsDomain) const
>+{
>+    if (domainSuffix.isEmpty())
>+        return false;
>+
>+    auto ipAddressSetting = treatIPAddressAsDomain ? OriginAccessEntry::TreatIPAddressAsDomain : OriginAccessEntry::TreatIPAddressAsIPAddress;
>+    OriginAccessEntry accessEntry { protocol(), domainSuffix, OriginAccessEntry::AllowSubdomains, ipAddressSetting };
>+    if (!accessEntry.matchesOrigin(*this))
>+        return false;
>+
>+    // Always return true if it is an exact match.
>+    if (domainSuffix.length() == host().length())
>+        return true;
>+
>+#if ENABLE(PUBLIC_SUFFIX_LIST)
>+    return !isPublicSuffix(domainSuffix);
>+#else
>+    return true;
>+#endif
>+}
>+
> void SecurityOrigin::grantLoadLocalResources()
> {
>     // Granting privileges to some, but not all, documents in a SecurityOrigin
>diff --git a/Source/WebCore/page/SecurityOrigin.h b/Source/WebCore/page/SecurityOrigin.h
>index 9a95b459cb53b1b332175669850d8a23e34d3a05..31b6b3d523809ea71da03dbb0a804d2521732d15 100644
>--- a/Source/WebCore/page/SecurityOrigin.h
>+++ b/Source/WebCore/page/SecurityOrigin.h
>@@ -204,6 +204,10 @@ public:
>     // https://html.spec.whatwg.org/multipage/browsers.html#same-origin
>     WEBCORE_EXPORT bool isSameOriginAs(const SecurityOrigin&) const;
> 
>+    // This method implements the "is a registrable domain suffix of or is equal to" algorithm from the HTML Standard:
>+    // https://html.spec.whatwg.org/multipage/origin.html#is-a-registrable-domain-suffix-of-or-is-equal-to
>+    WEBCORE_EXPORT bool isMatchingRegistrableDomainSuffix(const String&, bool treatIPAddressAsDomain = false) const;
>+
>     bool isPotentiallyTrustworthy() const { return m_isPotentiallyTrustworthy; }
>     void setIsPotentiallyTrustworthy(bool value) { m_isPotentiallyTrustworthy = value; }
> 
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index c89195bfb8a245ac4ff9b1bf7da6376b41b35181..193da1232f39124324516614e3aa8f2d776051f7 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-05-01  Jiewen Tan  <jiewen_tan@apple.com>
>+
>+        Move Document::domainIsRegisterable to SecurityOrigin::isMatchingRegistrableDomainSuffix
>+        https://bugs.webkit.org/show_bug.cgi?id=181950
>+        <rdar://problem/43357371>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/WebCore/SecurityOrigin.cpp:
>+        (TestWebKitAPI::TEST_F):
>+
> 2019-04-26  Stephanie Lewis  <slewis@apple.com>
> 
>         run-benchmarks should have an intial prep and restore env call for tasks that are too expensive to do for every iteration
>diff --git a/Tools/TestWebKitAPI/Tests/WebCore/SecurityOrigin.cpp b/Tools/TestWebKitAPI/Tests/WebCore/SecurityOrigin.cpp
>index a41ab4bca371d45de8cb458685bc974e30f406f3..bf88878e28ce02c03bc516c1bdc0462ed115631e 100644
>--- a/Tools/TestWebKitAPI/Tests/WebCore/SecurityOrigin.cpp
>+++ b/Tools/TestWebKitAPI/Tests/WebCore/SecurityOrigin.cpp
>@@ -183,4 +183,44 @@ TEST_F(SecurityOriginTest, IsPotentiallyTrustworthy)
>     EXPECT_FALSE(SecurityOrigin::createFromString("dummy:a")->isPotentiallyTrustworthy());
> }
> 
>+TEST_F(SecurityOriginTest, IsRegistrableDomainSuffix)
>+{
>+    auto exampleOrigin = SecurityOrigin::create(URL(URL(), "http://www.example.com"));
>+    EXPECT_TRUE(exampleOrigin->isMatchingRegistrableDomainSuffix("example.com"));
>+    EXPECT_TRUE(exampleOrigin->isMatchingRegistrableDomainSuffix("www.example.com"));
>+#if !ENABLE(PUBLIC_SUFFIX_LIST)
>+    EXPECT_TRUE(exampleOrigin->isMatchingRegistrableDomainSuffix("com"));
>+#endif
>+    EXPECT_FALSE(exampleOrigin->isMatchingRegistrableDomainSuffix(""));
>+    EXPECT_FALSE(exampleOrigin->isMatchingRegistrableDomainSuffix("."));
>+    EXPECT_FALSE(exampleOrigin->isMatchingRegistrableDomainSuffix(".example.com"));
>+    EXPECT_FALSE(exampleOrigin->isMatchingRegistrableDomainSuffix(".www.example.com"));
>+    EXPECT_FALSE(exampleOrigin->isMatchingRegistrableDomainSuffix("example.com."));
>+#if ENABLE(PUBLIC_SUFFIX_LIST)
>+    EXPECT_FALSE(exampleOrigin->isMatchingRegistrableDomainSuffix("com"));
>+#endif
>+
>+    auto exampleDotOrigin = SecurityOrigin::create(URL(URL(), "http://www.example.com."));
>+    EXPECT_TRUE(exampleDotOrigin->isMatchingRegistrableDomainSuffix("example.com."));
>+    EXPECT_TRUE(exampleDotOrigin->isMatchingRegistrableDomainSuffix("www.example.com."));
>+#if !ENABLE(PUBLIC_SUFFIX_LIST)
>+    EXPECT_TRUE(exampleOrigin->isMatchingRegistrableDomainSuffix("com."));
>+#endif
>+    EXPECT_FALSE(exampleDotOrigin->isMatchingRegistrableDomainSuffix(""));
>+    EXPECT_FALSE(exampleDotOrigin->isMatchingRegistrableDomainSuffix("."));
>+    EXPECT_FALSE(exampleDotOrigin->isMatchingRegistrableDomainSuffix(".example.com."));
>+    EXPECT_FALSE(exampleDotOrigin->isMatchingRegistrableDomainSuffix(".www.example.com."));
>+    EXPECT_FALSE(exampleDotOrigin->isMatchingRegistrableDomainSuffix("example.com"));
>+#if ENABLE(PUBLIC_SUFFIX_LIST)
>+    EXPECT_FALSE(exampleDotOrigin->isMatchingRegistrableDomainSuffix("com"));
>+#endif
>+
>+    auto ipOrigin = SecurityOrigin::create(URL(URL(), "http://127.0.0.1"));
>+    EXPECT_TRUE(ipOrigin->isMatchingRegistrableDomainSuffix("127.0.0.1", true));
>+    EXPECT_FALSE(ipOrigin->isMatchingRegistrableDomainSuffix("127.0.0.2", true));
>+
>+    auto comOrigin = SecurityOrigin::create(URL(URL(), "http://com"));
>+    EXPECT_TRUE(comOrigin->isMatchingRegistrableDomainSuffix("com"));
>+}
>+
> } // namespace TestWebKitAPI

Actions: View | Formatted Diff | Diff

Attachments on bug 181950: 368655 | 368692