Begin making SecurityOrigin immutable
authorandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 May 2013 11:59:15 +0000 (11:59 +0000)
committerandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 May 2013 11:59:15 +0000 (11:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=115898

Reviewed by Andreas Kling.

Replace SecurityOrigin::setDomainFromDOM and SecurityOrigin::grantUniversalAccess with
member functions that return new SecurityOrigin objects.

* dom/Document.cpp:
(WebCore::Document::setDomain):
Update the security origin to one returned by copyWithDomainSetFromDOM.

(WebCore::Document::initSecurityContext):
Set the security origin to one returned by copyWithUniversalAccessGranted().

* page/SecurityOrigin.cpp:
(WebCore::SecurityOrigin::SecurityOrigin):
Add a new constructor that takes all the member variables as parameters. This is a little unwieldy at the moment,
but all the boolean parameters could be replaced by a bitmask of flags.

(WebCore::SecurityOrigin::isolatedCopy):
Call the new constructor.

(WebCore::SecurityOrigin::copyWithDomainSetFromDOM):
Return a new security origin with m_domainWasSetInDOM set to true and the domain updated.

(WebCore::SecurityOrigin::copyWithUniversalAccessGranted):
Return a new security origin with m_universalAccess set to true.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@149866 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/page/SecurityOrigin.cpp
Source/WebCore/page/SecurityOrigin.h

index 4871916..a7496ce 100644 (file)
@@ -1,5 +1,36 @@
 2013-05-10  Anders Carlsson  <andersca@apple.com>
 
+        Begin making SecurityOrigin immutable
+        https://bugs.webkit.org/show_bug.cgi?id=115898
+
+        Reviewed by Andreas Kling.
+
+        Replace SecurityOrigin::setDomainFromDOM and SecurityOrigin::grantUniversalAccess with
+        member functions that return new SecurityOrigin objects.
+
+        * dom/Document.cpp:
+        (WebCore::Document::setDomain):
+        Update the security origin to one returned by copyWithDomainSetFromDOM.
+    
+        (WebCore::Document::initSecurityContext):
+        Set the security origin to one returned by copyWithUniversalAccessGranted().
+
+        * page/SecurityOrigin.cpp:
+        (WebCore::SecurityOrigin::SecurityOrigin):
+        Add a new constructor that takes all the member variables as parameters. This is a little unwieldy at the moment,
+        but all the boolean parameters could be replaced by a bitmask of flags.
+
+        (WebCore::SecurityOrigin::isolatedCopy):
+        Call the new constructor.
+
+        (WebCore::SecurityOrigin::copyWithDomainSetFromDOM):
+        Return a new security origin with m_domainWasSetInDOM set to true and the domain updated.
+
+        (WebCore::SecurityOrigin::copyWithUniversalAccessGranted):
+        Return a new security origin with m_universalAccess set to true.
+
+2013-05-10  Anders Carlsson  <andersca@apple.com>
+
         Remove ScriptController::updateSecurityOrigin
         https://bugs.webkit.org/show_bug.cgi?id=115895
 
index c07ce96..ab3100f 100644 (file)
@@ -3804,13 +3804,13 @@ void Document::setDomain(const String& newDomain, ExceptionCode& ec)
     // FIXME: We should add logging indicating why a domain was not allowed.
 
     // If the new domain is the same as the old domain, still call
-    // securityOrigin()->setDomainForDOM. This will change the
+    // securityOrigin()->copyWithDomainSetFromDOM. 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 (equalIgnoringCase(domain(), newDomain)) {
-        securityOrigin()->setDomainFromDOM(newDomain);
+        setSecurityOrigin(securityOrigin()->copyWithDomainSetFromDOM(newDomain));
         return;
     }
 
@@ -3837,7 +3837,7 @@ void Document::setDomain(const String& newDomain, ExceptionCode& ec)
         return;
     }
 
-    securityOrigin()->setDomainFromDOM(newDomain);
+    setSecurityOrigin(securityOrigin()->copyWithDomainSetFromDOM(newDomain));
 }
 
 // http://www.whatwg.org/specs/web-apps/current-work/#dom-document-lastmodified
@@ -4597,11 +4597,11 @@ void Document::initSecurityContext()
         if (!settings->webSecurityEnabled()) {
             // Web security is turned off. We should let this document access every other document. This is used primary by testing
             // harnesses for web sites.
-            securityOrigin()->grantUniversalAccess();
+            setSecurityOrigin(securityOrigin()->copyWithUniversalAccessGranted());
         } else if (securityOrigin()->isLocal()) {
             if (settings->allowUniversalAccessFromFileURLs() || m_frame->loader()->client()->shouldForceUniversalAccessFromLocalURL(m_url)) {
                 // Some clients want local URLs to have universal access, but that setting is dangerous for other clients.
-                securityOrigin()->grantUniversalAccess();
+                setSecurityOrigin(securityOrigin()->copyWithUniversalAccessGranted());
             } else if (!settings->allowFileAccessFromFileURLs()) {
                 // Some clients want local URLs to have even tighter restrictions by default, and not be able to access other local files.
                 // FIXME 81578: The naming of this is confusing. Files with restricted access to other local files
index 47ea1f5..09e5229 100644 (file)
@@ -156,19 +156,19 @@ SecurityOrigin::SecurityOrigin()
 {
 }
 
-SecurityOrigin::SecurityOrigin(const SecurityOrigin* other)
-    : m_protocol(other->m_protocol.isolatedCopy())
-    , m_host(other->m_host.isolatedCopy())
-    , m_domain(other->m_domain.isolatedCopy())
-    , m_filePath(other->m_filePath.isolatedCopy())
-    , m_port(other->m_port)
-    , m_isUnique(other->m_isUnique)
-    , m_universalAccess(other->m_universalAccess)
-    , m_domainWasSetInDOM(other->m_domainWasSetInDOM)
-    , m_canLoadLocalResources(other->m_canLoadLocalResources)
-    , m_storageBlockingPolicy(other->m_storageBlockingPolicy)
-    , m_enforceFilePathSeparation(other->m_enforceFilePathSeparation)
-    , m_needsDatabaseIdentifierQuirkForFiles(other->m_needsDatabaseIdentifierQuirkForFiles)
+SecurityOrigin::SecurityOrigin(const String& protocol, const String& host, const String& domain, const String& filePath, unsigned short port, bool isUnique, bool universalAccess, bool domainWasSetInDOM, bool canLoadLocalResources, StorageBlockingPolicy storageBlockingPolicy, bool enforceFilePathSeparation, bool needsDatabaseIdentifierQuirkForFiles)
+    : m_protocol(protocol)
+    , m_host(host)
+    , m_domain(domain)
+    , m_filePath(filePath)
+    , m_port(port)
+    , m_isUnique(isUnique)
+    , m_universalAccess(universalAccess)
+    , m_domainWasSetInDOM(domainWasSetInDOM)
+    , m_canLoadLocalResources(canLoadLocalResources)
+    , m_storageBlockingPolicy(storageBlockingPolicy)
+    , m_enforceFilePathSeparation(enforceFilePathSeparation)
+    , m_needsDatabaseIdentifierQuirkForFiles(needsDatabaseIdentifierQuirkForFiles)
 {
 }
 
@@ -207,13 +207,16 @@ PassRefPtr<SecurityOrigin> SecurityOrigin::createUnique()
 
 PassRefPtr<SecurityOrigin> SecurityOrigin::isolatedCopy() const
 {
-    return adoptRef(new SecurityOrigin(this));
+    return adoptRef(new SecurityOrigin(m_protocol.isolatedCopy(), m_host.isolatedCopy(), m_domain.isolatedCopy(), m_filePath.isolatedCopy(), m_port, m_isUnique, m_universalAccess, m_domainWasSetInDOM, m_canLoadLocalResources, m_storageBlockingPolicy, m_enforceFilePathSeparation, m_needsDatabaseIdentifierQuirkForFiles));
 }
 
-void SecurityOrigin::setDomainFromDOM(const String& newDomain)
+PassRefPtr<SecurityOrigin> SecurityOrigin::copyWithDomainSetFromDOM(const String& newDomain) const
 {
-    m_domainWasSetInDOM = true;
-    m_domain = newDomain.lower();
+    String domain = newDomain.lower();
+    if (m_domainWasSetInDOM && m_domain == domain)
+        return const_cast<SecurityOrigin*>(this);
+
+    return adoptRef(new SecurityOrigin(m_protocol, m_host, domain, m_filePath, m_port, m_isUnique, m_universalAccess, true, m_canLoadLocalResources, m_storageBlockingPolicy, m_enforceFilePathSeparation, m_needsDatabaseIdentifierQuirkForFiles));
 }
 
 bool SecurityOrigin::isSecure(const KURL& url)
@@ -434,9 +437,12 @@ void SecurityOrigin::grantLoadLocalResources()
     m_canLoadLocalResources = true;
 }
 
-void SecurityOrigin::grantUniversalAccess()
+PassRefPtr<SecurityOrigin> SecurityOrigin::copyWithUniversalAccessGranted() const
 {
-    m_universalAccess = true;
+    if (m_universalAccess)
+        return const_cast<SecurityOrigin*>(this);
+
+    return adoptRef(new SecurityOrigin(m_protocol, m_host, m_domain, m_filePath, m_port, m_isUnique, true, m_domainWasSetInDOM, m_canLoadLocalResources, m_storageBlockingPolicy, m_enforceFilePathSeparation, m_needsDatabaseIdentifierQuirkForFiles));
 }
 
 #if ENABLE(CACHE_PARTITIONING)
index 3fcd87c..cfd2ab1 100644 (file)
@@ -74,10 +74,10 @@ public:
     // when marshalling a SecurityOrigin to another thread.
     PassRefPtr<SecurityOrigin> isolatedCopy() const;
 
-    // Set the domain property of this security origin to newDomain. This
+    // Create a new security origin with the domain property set to newDomain. This
     // function does not check whether newDomain is a suffix of the current
     // domain. The caller is responsible for validating newDomain.
-    void setDomainFromDOM(const String& newDomain);
+    PassRefPtr<SecurityOrigin> copyWithDomainSetFromDOM(const String& newDomain) const;
     bool domainWasSetInDOM() const { return m_domainWasSetInDOM; }
 
     String protocol() const { return m_protocol; }
@@ -137,7 +137,7 @@ public:
     // Explicitly grant the ability to access very other SecurityOrigin.
     //
     // WARNING: This is an extremely powerful ability. Use with caution!
-    void grantUniversalAccess();
+    PassRefPtr<SecurityOrigin> copyWithUniversalAccessGranted() const;
 
     void setStorageBlockingPolicy(StorageBlockingPolicy policy) { m_storageBlockingPolicy = policy; }
 
@@ -215,6 +215,7 @@ private:
     SecurityOrigin();
     explicit SecurityOrigin(const KURL&);
     explicit SecurityOrigin(const SecurityOrigin*);
+    SecurityOrigin(const String& protocol, const String& host, const String& domain, const String& filePath, unsigned short port, bool isUnique, bool universalAccess, bool domainWasSetInDOM, bool canLoadLocalResources, StorageBlockingPolicy, bool enforceFilePathSeparation, bool needsDatabaseIdentifierQuirkForFiles);
 
     // FIXME: Rename this function to something more semantic.
     bool passesFileCheck(const SecurityOrigin*) const;