Attachment 281358 Details for Bug 158728 – Beefing up tests and addressing other comments

[patch] Beefing up tests and addressing other comments

bug-158728-20160615160910.patch (text/plain), 34.09 KB, created by youenn fablet on 2016-06-15 07:07:44 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2016-06-15 07:07:44 PDT

Size: 34.09 KB

patch

obsolete

>Subversion Revision: 202085
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 7be018f703d72d27f5401d228a927450100634fb..f301352dd95932d8ea9b946192ef16f9b89f92fa 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,57 @@
>+2016-06-15  Youenn Fablet  <youenn.fablet@crf.canon.fr>
>+
>+        Clean CSS stylesheets should be accessible from JavaScript
>+        https://bugs.webkit.org/show_bug.cgi?id=158728
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Introducing computation of the resource response type as per fetch specification.
>+        This computation is done in SubresourceLoader at init time and for each redirection.
>+        If cross-origin, SubresourceLoader will set the response type to Cors if fetch mode is Cors
>+        or to Opaque if fetch mode is NoCors.
>+
>+        Making use of the resource response type to evaluate whether to make the CSS stylesheets visible
>+        for stylesheet link elements and for stylesheet processing instructions.
>+        Ideally, the origin flag should be a boolean.
>+        To keep behavior consistent in cors-check-unaware cases, the flag can be set to undefined in which case the
>+        behavior remains the same (checking of the stylesheet URL).
>+
>+        Covered by updated tests.
>+
>+        * css/CSSStyleSheet.cpp:
>+        (WebCore::CSSStyleSheet::create): Makes origin flag undefined if boolean parameter is not passed.
>+        Otherwise origin flag is set according given boolean value.
>+        (WebCore::CSSStyleSheet::createInline): Makes origin flag set to true.
>+        (WebCore::CSSStyleSheet::CSSStyleSheet): Adding origin flag initialization.
>+        (WebCore::CSSStyleSheet::canAccessRules): Returning according the origin flag if set.
>+        Returning as before in case the flag is undefined.
>+        * css/CSSStyleSheet.h: Adding origin clean flag.
>+        * html/HTMLLinkElement.cpp:
>+        (WebCore::HTMLLinkElement::initializeStyleSheet): Helper routine used in setCSSStyleSheet.
>+        Sets origin clean flag if the resource is clean.
>+        (WebCore::HTMLLinkElement::setCSSStyleSheet): Making use of initializeStyleSheet.
>+        * html/HTMLLinkElement.h:
>+        * loader/SubresourceLoader.cpp:
>+        (WebCore::SubresourceLoader::init): Getting m_origin to compute the resource response type.
>+        (WebCore::SubresourceLoader::willSendRequestInternal):
>+        (WebCore::SubresourceLoader::didReceiveResponse):
>+        (WebCore::SubresourceLoader::checkRedirectionCrossOriginAccessControl): Marking resource as cross-origin
>+        if redirection is cross-origin.
>+        * loader/SubresourceLoader.h:
>+        * loader/cache/CachedResource.cpp:
>+        (WebCore::CachedResource::load): Computing resource origin from the request or the document loader itself.
>+        (WebCore::CachedResource::setCrossOrigin): Set response type as opaque or cors depending on fetch mode.
>+        (WebCore::CachedResource::isCrossOrigin): Returns true if response type is opaque or cors.
>+        (WebCore::CachedResource::isClean): Returns true if response type is not opaque and loading is ok.
>+        (WebCore::CachedResource::setResourceToRevalidate):
>+        * loader/cache/CachedResource.h:
>+        (WebCore::CachedResource::setOpaqueRedirect):
>+        (WebCore::CachedResource::origin):
>+        * platform/network/ResourceRequestBase.cpp:
>+        (WebCore::ResourceRequestBase::hasHTTPOrigin): Added to detect whether code calling loaders set the origin of the request.
>+        (WebCore::ResourceRequestBase::clearHTTPOrigin):
>+        * platform/network/ResourceRequestBase.h:
>+
> 2016-06-14  Myles C. Maxfield  <mmaxfield@apple.com>
> 
>         Addressing post-review comments after r201971
>diff --git a/Source/WebCore/css/CSSStyleSheet.cpp b/Source/WebCore/css/CSSStyleSheet.cpp
>index 8f6a30ada023c501de596175e47e68653f38625a..1c2700f2f3a59c63a419f1f3170f118688273fd7 100644
>--- a/Source/WebCore/css/CSSStyleSheet.cpp
>+++ b/Source/WebCore/css/CSSStyleSheet.cpp
>@@ -78,19 +78,24 @@ static bool isAcceptableCSSStyleSheetParent(Node* parentNode)
> #endif
> 
> Ref<CSSStyleSheet> CSSStyleSheet::create(Ref<StyleSheetContents>&& sheet, CSSImportRule* ownerRule)
>-{ 
>+{
>     return adoptRef(*new CSSStyleSheet(WTFMove(sheet), ownerRule));
> }
> 
> Ref<CSSStyleSheet> CSSStyleSheet::create(Ref<StyleSheetContents>&& sheet, Node* ownerNode)
>-{ 
>-    return adoptRef(*new CSSStyleSheet(WTFMove(sheet), ownerNode, TextPosition::minimumPosition(), false));
>+{
>+    return adoptRef(*new CSSStyleSheet(WTFMove(sheet), ownerNode, TextPosition::minimumPosition(), false, OriginCleanFlag::Undefined));
>+}
>+
>+Ref<CSSStyleSheet> CSSStyleSheet::create(Ref<StyleSheetContents>&& sheet, Node* ownerNode, bool isCleanOrigin)
>+{
>+    return adoptRef(*new CSSStyleSheet(WTFMove(sheet), ownerNode, TextPosition::minimumPosition(), false, isCleanOrigin ? OriginCleanFlag::True : OriginCleanFlag::False));
> }
> 
> Ref<CSSStyleSheet> CSSStyleSheet::createInline(Node& ownerNode, const URL& baseURL, const TextPosition& startPosition, const String& encoding)
> {
>     CSSParserContext parserContext(ownerNode.document(), baseURL, encoding);
>-    return adoptRef(*new CSSStyleSheet(StyleSheetContents::create(baseURL.string(), parserContext), &ownerNode, startPosition, true));
>+    return adoptRef(*new CSSStyleSheet(StyleSheetContents::create(baseURL.string(), parserContext), &ownerNode, startPosition, true, OriginCleanFlag::True));
> }
> 
> CSSStyleSheet::CSSStyleSheet(Ref<StyleSheetContents>&& contents, CSSImportRule* ownerRule)
>@@ -105,11 +110,12 @@ CSSStyleSheet::CSSStyleSheet(Ref<StyleSheetContents>&& contents, CSSImportRule*
>     m_contents->registerClient(this);
> }
> 
>-CSSStyleSheet::CSSStyleSheet(Ref<StyleSheetContents>&& contents, Node* ownerNode, const TextPosition& startPosition, bool isInlineStylesheet)
>+CSSStyleSheet::CSSStyleSheet(Ref<StyleSheetContents>&& contents, Node* ownerNode, const TextPosition& startPosition, bool isInlineStylesheet, OriginCleanFlag flag)
>     : m_contents(WTFMove(contents))
>     , m_isInlineStylesheet(isInlineStylesheet)
>     , m_isDisabled(false)
>     , m_mutatedRules(false)
>+    , m_isOriginClean(flag)
>     , m_ownerNode(ownerNode)
>     , m_ownerRule(0)
>     , m_startPosition(startPosition)
>@@ -258,17 +264,16 @@ CSSRule* CSSStyleSheet::item(unsigned index)
> 
> bool CSSStyleSheet::canAccessRules() const
> {
>-    if (m_isInlineStylesheet)
>-        return true;
>+    if (m_isOriginClean != OriginCleanFlag::Undefined)
>+        return m_isOriginClean == OriginCleanFlag::True;
>+
>     URL baseURL = m_contents->baseURL();
>     if (baseURL.isEmpty())
>         return true;
>     Document* document = ownerDocument();
>     if (!document)
>         return true;
>-    if (document->securityOrigin()->canRequest(baseURL))
>-        return true;
>-    return false;
>+    return document->securityOrigin()->canRequest(baseURL);
> }
> 
> RefPtr<CSSRuleList> CSSStyleSheet::rules()
>diff --git a/Source/WebCore/css/CSSStyleSheet.h b/Source/WebCore/css/CSSStyleSheet.h
>index c0e67e1f5b59d4b537a8bf53adf00883878993fb..31c50bd8fc4e13dbc987fac2395f97509a56ad44 100644
>--- a/Source/WebCore/css/CSSStyleSheet.h
>+++ b/Source/WebCore/css/CSSStyleSheet.h
>@@ -51,6 +51,7 @@ class CSSStyleSheet final : public StyleSheet {
> public:
>     static Ref<CSSStyleSheet> create(Ref<StyleSheetContents>&&, CSSImportRule* ownerRule = 0);
>     static Ref<CSSStyleSheet> create(Ref<StyleSheetContents>&&, Node* ownerNode);
>+    static Ref<CSSStyleSheet> create(Ref<StyleSheetContents>&&, Node* ownerNode, bool /* isClean */);
>     static Ref<CSSStyleSheet> createInline(Node&, const URL&, const TextPosition& startPosition = TextPosition::minimumPosition(), const String& encoding = String());
> 
>     virtual ~CSSStyleSheet();
>@@ -123,18 +124,25 @@ public:
>     void detachFromDocument() { m_ownerNode = nullptr; }
> 
> private:
>+    enum class OriginCleanFlag {
>+        True,
>+        False,
>+        Undefined
>+    };
>+
>     CSSStyleSheet(Ref<StyleSheetContents>&&, CSSImportRule* ownerRule);
>-    CSSStyleSheet(Ref<StyleSheetContents>&&, Node* ownerNode, const TextPosition& startPosition, bool isInlineStylesheet);
>+    CSSStyleSheet(Ref<StyleSheetContents>&&, Node* ownerNode, const TextPosition& startPosition, bool isInlineStylesheet, OriginCleanFlag);
> 
>     bool isCSSStyleSheet() const override { return true; }
>     String type() const override { return ASCIILiteral("text/css"); }
> 
>     bool canAccessRules() const;
>-    
>+
>     Ref<StyleSheetContents> m_contents;
>     bool m_isInlineStylesheet;
>     bool m_isDisabled;
>     bool m_mutatedRules;
>+    OriginCleanFlag m_isOriginClean { OriginCleanFlag::Undefined };
>     String m_title;
>     RefPtr<MediaQuerySet> m_mediaQueries;
> 
>diff --git a/Source/WebCore/dom/ProcessingInstruction.cpp b/Source/WebCore/dom/ProcessingInstruction.cpp
>index 28033159facafccd3fb98f5b9f9e4e2a3105183a..fe8f4912c87d8a6838a9c9c152e9d7a1068e0a10 100644
>--- a/Source/WebCore/dom/ProcessingInstruction.cpp
>+++ b/Source/WebCore/dom/ProcessingInstruction.cpp
>@@ -190,10 +190,10 @@ void ProcessingInstruction::setCSSStyleSheet(const String& href, const URL& base
>     ASSERT(m_isCSS);
>     CSSParserContext parserContext(document(), baseURL, charset);
> 
>-    auto cssSheet = CSSStyleSheet::create(StyleSheetContents::create(href, parserContext), this);
>-    cssSheet.get().setDisabled(m_alternate);
>-    cssSheet.get().setTitle(m_title);
>-    cssSheet.get().setMediaQueries(MediaQuerySet::create(m_media));
>+    auto cssSheet = CSSStyleSheet::create(StyleSheetContents::create(href, parserContext), this, sheet->isClean());
>+    cssSheet->setDisabled(m_alternate);
>+    cssSheet->setTitle(m_title);
>+    cssSheet->setMediaQueries(MediaQuerySet::create(m_media));
> 
>     m_sheet = WTFMove(cssSheet);
> 
>diff --git a/Source/WebCore/html/HTMLLinkElement.cpp b/Source/WebCore/html/HTMLLinkElement.cpp
>index 83424c0283683f5191e562dba442f2d64ea8a08a..8fb51982af36223236174bc96373d9d6b97533e5 100644
>--- a/Source/WebCore/html/HTMLLinkElement.cpp
>+++ b/Source/WebCore/html/HTMLLinkElement.cpp
>@@ -331,6 +331,13 @@ void HTMLLinkElement::finishParsingChildren()
>     HTMLElement::finishParsingChildren();
> }
> 
>+void HTMLLinkElement::initializeStyleSheet(Ref<StyleSheetContents>&& styleSheet, const CachedCSSStyleSheet& cachedStyleSheet)
>+{
>+    m_sheet = CSSStyleSheet::create(WTFMove(styleSheet), this, cachedStyleSheet.isClean());
>+    m_sheet->setMediaQueries(MediaQuerySet::createAllowingDescriptionSyntax(m_media));
>+    m_sheet->setTitle(title());
>+}
>+
> void HTMLLinkElement::setCSSStyleSheet(const String& href, const URL& baseURL, const String& charset, const CachedCSSStyleSheet* cachedStyleSheet)
> {
>     if (!inDocument()) {
>@@ -350,10 +357,7 @@ void HTMLLinkElement::setCSSStyleSheet(const String& href, const URL& baseURL, c
>     if (RefPtr<StyleSheetContents> restoredSheet = const_cast<CachedCSSStyleSheet*>(cachedStyleSheet)->restoreParsedStyleSheet(parserContext, cachePolicy)) {
>         ASSERT(restoredSheet->isCacheable());
>         ASSERT(!restoredSheet->isLoading());
>-
>-        m_sheet = CSSStyleSheet::create(restoredSheet.releaseNonNull(), this);
>-        m_sheet->setMediaQueries(MediaQuerySet::createAllowingDescriptionSyntax(m_media));
>-        m_sheet->setTitle(title());
>+        initializeStyleSheet(restoredSheet.releaseNonNull(), *cachedStyleSheet);
> 
>         m_loading = false;
>         sheetLoaded();
>@@ -362,9 +366,7 @@ void HTMLLinkElement::setCSSStyleSheet(const String& href, const URL& baseURL, c
>     }
> 
>     auto styleSheet = StyleSheetContents::create(href, parserContext);
>-    m_sheet = CSSStyleSheet::create(styleSheet.copyRef(), this);
>-    m_sheet->setMediaQueries(MediaQuerySet::createAllowingDescriptionSyntax(m_media));
>-    m_sheet->setTitle(title());
>+    initializeStyleSheet(styleSheet.copyRef(), *cachedStyleSheet);
> 
>     styleSheet.get().parseAuthorStyleSheet(cachedStyleSheet, document().securityOrigin());
> 
>diff --git a/Source/WebCore/html/HTMLLinkElement.h b/Source/WebCore/html/HTMLLinkElement.h
>index 1b1b1cec8de47e16ad4dc9c9539490222d2c842f..229cf96812dad7e9f5ad606c4bb99b77c674c43d 100644
>--- a/Source/WebCore/html/HTMLLinkElement.h
>+++ b/Source/WebCore/html/HTMLLinkElement.h
>@@ -86,6 +86,8 @@ private:
>     InsertionNotificationRequest insertedInto(ContainerNode&) override;
>     void removedFrom(ContainerNode&) override;
> 
>+    void initializeStyleSheet(Ref<StyleSheetContents>&&, const CachedCSSStyleSheet&);
>+
>     // from CachedResourceClient
>     void setCSSStyleSheet(const String& href, const URL& baseURL, const String& charset, const CachedCSSStyleSheet* sheet) override;
>     bool sheetLoaded() override;
>diff --git a/Source/WebCore/loader/SubresourceLoader.cpp b/Source/WebCore/loader/SubresourceLoader.cpp
>index 386ec073b7ddcf7c16bcd5c4ebc4404d340f4682..237943644c991c402697a9293cbe2900d5ea956b 100644
>--- a/Source/WebCore/loader/SubresourceLoader.cpp
>+++ b/Source/WebCore/loader/SubresourceLoader.cpp
>@@ -6,13 +6,13 @@
>  * are met:
>  *
>  * 1.  Redistributions of source code must retain the above copyright
>- *     notice, this list of conditions and the following disclaimer. 
>+ *     notice, this list of conditions and the following disclaimer.
>  * 2.  Redistributions in binary form must reproduce the above copyright
>  *     notice, this list of conditions and the following disclaimer in the
>- *     documentation and/or other materials provided with the distribution. 
>+ *     documentation and/or other materials provided with the distribution.
>  * 3.  Neither the name of Apple Inc. ("Apple") nor the names of
>  *     its contributors may be used to endorse or promote products derived
>- *     from this software without specific prior written permission. 
>+ *     from this software without specific prior written permission.
>  *
>  * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
>  * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
>@@ -151,8 +151,11 @@ bool SubresourceLoader::init(const ResourceRequest& request)
>     // FIXME: https://bugs.webkit.org/show_bug.cgi?id=155633.
>     // SubresourceLoader could use the document origin as a default and set PotentiallyCrossOriginEnabled requests accordingly.
>     // This would simplify resource loader users as they would only need to set the policy to PotentiallyCrossOriginEnabled.
>-    if (options().requestOriginPolicy() == PotentiallyCrossOriginEnabled)
>-        m_origin = SecurityOrigin::createFromString(request.httpOrigin());
>+
>+    m_origin = m_resource->origin();
>+    // https://fetch.spec.whatwg.org/#main-fetch, step 11, data URL here is considered not cross origin.
>+    if (!request.url().protocolIsData() && m_origin && !m_origin->canRequest(request.url()))
>+        m_resource->setCrossOrigin();
> 
>     return true;
> }
>@@ -202,7 +205,7 @@ void SubresourceLoader::willSendRequestInternal(ResourceRequest& newRequest, con
>             return;
>         }
> 
>-        if (options().requestOriginPolicy() == PotentiallyCrossOriginEnabled && !checkCrossOriginAccessControl(request(), redirectResponse, newRequest)) {
>+        if (!checkRedirectionCrossOriginAccessControl(request(), redirectResponse, newRequest)) {
>             cancel();
>             return;
>         }
>@@ -295,8 +298,8 @@ void SubresourceLoader::didReceiveResponse(const ResourceResponse& response)
>         // The resource data will change as the next part is loaded, so we need to make a copy.
>         m_resource->finishLoading(buffer->copy().ptr());
>         clearResourceData();
>-        // Since a subresource loader does not load multipart sections progressively, data was delivered to the loader all at once.        
>-        // After the first multipart section is complete, signal to delegates that this load is "finished" 
>+        // Since a subresource loader does not load multipart sections progressively, data was delivered to the loader all at once.
>+        // After the first multipart section is complete, signal to delegates that this load is "finished"
>         m_documentLoader->subresourceLoaderFinishedLoadingOnePart(this);
>         didFinishLoadingOnePart(0);
>     }
>@@ -396,11 +399,15 @@ static void logResourceLoaded(Frame* frame, CachedResource::Type type)
>     frame->page()->diagnosticLoggingClient().logDiagnosticMessageWithValue(DiagnosticLoggingKeys::resourceKey(), DiagnosticLoggingKeys::loadedKey(), resourceType, ShouldSample::Yes);
> }
> 
>-bool SubresourceLoader::checkCrossOriginAccessControl(const ResourceRequest& previousRequest, const ResourceResponse& redirectResponse, ResourceRequest& newRequest)
>+bool SubresourceLoader::checkRedirectionCrossOriginAccessControl(const ResourceRequest& previousRequest, const ResourceResponse& redirectResponse, ResourceRequest& newRequest)
> {
>-    if (m_origin->canRequest(newRequest.url()))
>+    if (!(m_origin && m_origin->canRequest(newRequest.url())))
>+        m_resource->setCrossOrigin();
>+
>+    if (options().requestOriginPolicy() != PotentiallyCrossOriginEnabled || !m_resource->isCrossOrigin())
>         return true;
> 
>+    ASSERT(m_origin);
>     String errorDescription;
>     bool responsePassesCORS = m_origin->canRequest(previousRequest.url())
>         || passesAccessControlCheck(redirectResponse, options().allowCredentials(), m_origin.get(), errorDescription);
>diff --git a/Source/WebCore/loader/SubresourceLoader.h b/Source/WebCore/loader/SubresourceLoader.h
>index 5ec7311d09cf1d30dc255045cf4b9b580ce9fde8..31c4b3ea056686400ab5c20d91412e9b25fc99dc 100644
>--- a/Source/WebCore/loader/SubresourceLoader.h
>+++ b/Source/WebCore/loader/SubresourceLoader.h
>@@ -6,13 +6,13 @@
>  * are met:
>  *
>  * 1.  Redistributions of source code must retain the above copyright
>- *     notice, this list of conditions and the following disclaimer. 
>+ *     notice, this list of conditions and the following disclaimer.
>  * 2.  Redistributions in binary form must reproduce the above copyright
>  *     notice, this list of conditions and the following disclaimer in the
>- *     documentation and/or other materials provided with the distribution. 
>+ *     documentation and/or other materials provided with the distribution.
>  * 3.  Neither the name of Apple Inc. ("Apple") nor the names of
>  *     its contributors may be used to endorse or promote products derived
>- *     from this software without specific prior written permission. 
>+ *     from this software without specific prior written permission.
>  *
>  * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
>  * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
>@@ -91,7 +91,7 @@ private:
> #endif
> 
>     bool checkForHTTPStatusCodeError();
>-    bool checkCrossOriginAccessControl(const ResourceRequest&, const ResourceResponse&, ResourceRequest& newRequest);
>+    bool checkRedirectionCrossOriginAccessControl(const ResourceRequest&, const ResourceResponse&, ResourceRequest& newRequest);
> 
>     void didReceiveDataOrBuffer(const char*, int, RefPtr<SharedBuffer>&&, long long encodedDataLength, DataPayloadType);
> 
>diff --git a/Source/WebCore/loader/cache/CachedResource.cpp b/Source/WebCore/loader/cache/CachedResource.cpp
>index 5573c4deecb77e1551058ba6b08939aa8a4fb271..9e0ee7195857dd7b198a5b767d530020ef0749dd 100644
>--- a/Source/WebCore/loader/cache/CachedResource.cpp
>+++ b/Source/WebCore/loader/cache/CachedResource.cpp
>@@ -275,8 +275,14 @@ void CachedResource::load(CachedResourceLoader& cachedResourceLoader, const Reso
> #endif
>     m_resourceRequest.setPriority(loadPriority());
> 
>-    if (type() != MainResource)
>+    if (type() != MainResource) {
>+        if (m_resourceRequest.hasHTTPOrigin() || m_options.requestOriginPolicy() == PotentiallyCrossOriginEnabled)
>+            m_origin = SecurityOrigin::createFromString(m_resourceRequest.httpOrigin());
>+        else
>+            m_origin = cachedResourceLoader.document()->securityOrigin();
>+        ASSERT(m_origin);
>         addAdditionalRequestHeaders(cachedResourceLoader);
>+    }
> 
>     // FIXME: It's unfortunate that the cache layer and below get to know anything about fragment identifiers.
>     // We should look into removing the expectation of that knowledge from the platform network stacks.
>@@ -362,6 +368,25 @@ bool CachedResource::passesSameOriginPolicyCheck(SecurityOrigin& securityOrigin)
>     return passesAccessControlCheck(securityOrigin);
> }
> 
>+void CachedResource::setCrossOrigin()
>+{
>+    ASSERT(m_options.requestOriginPolicy() != RestrictToSameOrigin);
>+    m_responseType = (m_options.requestOriginPolicy() == PotentiallyCrossOriginEnabled) ? ResourceResponseBase::Type::Cors : ResourceResponseBase::Type::Opaque;
>+}
>+
>+bool CachedResource::isCrossOrigin() const
>+{
>+    ASSERT(m_options.requestOriginPolicy() != RestrictToSameOrigin);
>+    return m_responseType == ResourceResponseBase::Type::Cors || m_responseType == ResourceResponseBase::Type::Opaque;
>+}
>+
>+bool CachedResource::isClean() const
>+{
>+    // https://html.spec.whatwg.org/multipage/infrastructure.html#cors-same-origin
>+    ASSERT(m_responseType != ResourceResponseBase::Type::Opaqueredirect && m_responseType != ResourceResponseBase::Type::Error);
>+    return !loadFailedOrCanceled() && m_responseType != ResourceResponseBase::Type::Opaque;
>+}
>+
> bool CachedResource::isExpired() const
> {
>     if (m_response.isNull())
>diff --git a/Source/WebCore/loader/cache/CachedResource.h b/Source/WebCore/loader/cache/CachedResource.h
>index d40e9580481e807acb35174158664aa626899465..c3648f4aa0796b8f5965942433a835d5af3a9c75 100644
>--- a/Source/WebCore/loader/cache/CachedResource.h
>+++ b/Source/WebCore/loader/cache/CachedResource.h
>@@ -200,11 +200,17 @@ public:
>     virtual void responseReceived(const ResourceResponse&);
>     virtual bool shouldCacheResponse(const ResourceResponse&) { return true; }
>     void setResponse(const ResourceResponse&);
>-    void setOpaqueRedirect() { m_responseType = ResourceResponseBase::Type::Opaqueredirect; }
>     const ResourceResponse& response() const { return m_response; }
>     // This is the same as response() except after HTTP redirect to data: URL.
>     const ResourceResponse& responseForSameOriginPolicyChecks() const;
> 
>+    void setOpaqueRedirect() { m_responseType = ResourceResponseBase::Type::Opaqueredirect; }
>+    void setCrossOrigin();
>+    bool isCrossOrigin() const;
>+    bool isClean() const;
>+
>+    SecurityOrigin* origin() const { return m_origin.get(); }
>+
>     bool canDelete() const { return !hasClients() && !m_loader && !m_preloadCount && !m_handleCount && !m_resourceToRevalidate && !m_proxyResource; }
>     bool hasOneHandle() const { return m_handleCount == 1; }
> 
>@@ -313,6 +319,7 @@ private:
>     String m_fragmentIdentifierForRequest;
> 
>     ResourceError m_error;
>+    RefPtr<SecurityOrigin> m_origin;
> 
>     double m_lastDecodedAccessTime; // Used as a "thrash guard" in the cache
>     double m_loadFinishTime;
>diff --git a/Source/WebCore/platform/network/ResourceRequestBase.cpp b/Source/WebCore/platform/network/ResourceRequestBase.cpp
>index a48be4f9862c3d52bc5d65f8ef572a41a4b97cbf..174963e188776433d3dc99d80f45db99c1333c1f 100644
>--- a/Source/WebCore/platform/network/ResourceRequestBase.cpp
>+++ b/Source/WebCore/platform/network/ResourceRequestBase.cpp
>@@ -313,9 +313,14 @@ void ResourceRequestBase::setHTTPOrigin(const String& httpOrigin)
>     setHTTPHeaderField(HTTPHeaderName::Origin, httpOrigin);
> }
> 
>+bool ResourceRequestBase::hasHTTPOrigin() const
>+{
>+    return m_httpHeaderFields.contains(HTTPHeaderName::Origin);
>+}
>+
> void ResourceRequestBase::clearHTTPOrigin()
> {
>-    updateResourceRequest(); 
>+    updateResourceRequest();
> 
>     m_httpHeaderFields.remove(HTTPHeaderName::Origin);
> 
>diff --git a/Source/WebCore/platform/network/ResourceRequestBase.h b/Source/WebCore/platform/network/ResourceRequestBase.h
>index 6e0bbe020b9e8798db700eb4d563f4a7086a9590..d3ac391efa4a59fa6ab1a8c1a316766f02aa875f 100644
>--- a/Source/WebCore/platform/network/ResourceRequestBase.h
>+++ b/Source/WebCore/platform/network/ResourceRequestBase.h
>@@ -22,7 +22,7 @@
>  * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>  * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  */
> 
> #ifndef ResourceRequestBase_h
>@@ -100,8 +100,9 @@ public:
>     WEBCORE_EXPORT String httpReferrer() const;
>     WEBCORE_EXPORT void setHTTPReferrer(const String&);
>     WEBCORE_EXPORT void clearHTTPReferrer();
>-    
>+
>     String httpOrigin() const;
>+    bool hasHTTPOrigin() const;
>     void setHTTPOrigin(const String&);
>     WEBCORE_EXPORT void clearHTTPOrigin();
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index d01c13eb394cabf27eaf6717ea7e6d20178cb4a4..99da5cad354f61219e76963b68f7a312e3fdf7c0 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2016-06-15  Youenn Fablet  <youenn.fablet@crf.canon.fr>
>+
>+        Clean CSS stylesheets should be accessible from JavaScript
>+        https://bugs.webkit.org/show_bug.cgi?id=158728
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * http/tests/security/cannot-read-cssrules-redirect-expected.txt:
>+        * http/tests/security/cannot-read-cssrules-redirect.html: Adding more tests related to CORS.
>+        * http/tests/security/cross-origin-css-9.html: Activating JS visibility checks.
>+
> 2016-06-14  Myles C. Maxfield  <mmaxfield@apple.com>
> 
>         Honor bidi unicode codepoints
>diff --git a/LayoutTests/http/tests/security/cannot-read-cssrules-expected.txt b/LayoutTests/http/tests/security/cannot-read-cssrules-expected.txt
>index 7a78babc2a5d8447f04b594bb22d0e2ae4296435..b915122f7396898c1cefeadf98b56db3673e307d 100644
>--- a/LayoutTests/http/tests/security/cannot-read-cssrules-expected.txt
>+++ b/LayoutTests/http/tests/security/cannot-read-cssrules-expected.txt
>@@ -1,10 +1,22 @@
> This test whether a script can read the rules from a cross-origin style sheet. For more information on why we block this, please see https://bugs.webkit.org/show_bug.cgi?id=20527.
> 
> Test begins.
>-== Cross-Origin ==
>+== Cross-Origin resource, no-cors mode ==
> cssRules: null
> rules: null
>-== Same-Origin ==
>+== Same-Origin resource, no-cors mode ==
> cssRules: [object CSSRuleList]
>+cssRules length: 1
> rules: [object CSSRuleList]
>+rules length: 1
>+== Cross-Origin resource, cors mode, failing cors check ==
>+cssRules: [object CSSRuleList]
>+cssRules length: 1
>+rules: [object CSSRuleList]
>+rules length: 1
>+== Cross-Origin resource, cors mode, successful cors check ==
>+cssRules: [object CSSRuleList]
>+cssRules length: 1
>+rules: [object CSSRuleList]
>+rules length: 1
> Test ends.
>diff --git a/LayoutTests/http/tests/security/cannot-read-cssrules-redirect-expected.txt b/LayoutTests/http/tests/security/cannot-read-cssrules-redirect-expected.txt
>index 062646fd42d9feb8d3ce3efe80a7845d7e6a2396..146f6d0433de31a302620448d2ba2ba10ae90f5b 100644
>--- a/LayoutTests/http/tests/security/cannot-read-cssrules-redirect-expected.txt
>+++ b/LayoutTests/http/tests/security/cannot-read-cssrules-redirect-expected.txt
>@@ -1,10 +1,24 @@
>+CONSOLE MESSAGE: Cross-origin redirection denied by Cross-Origin Resource Sharing policy: Origin http://127.0.0.1:8000 is not allowed by Access-Control-Allow-Origin.
> This test whether a script can read the rules from a cross-origin style sheet in the presence of redirects. For more information on why we block this, please see https://bugs.webkit.org/show_bug.cgi?id=20527.
> 
> Test begins.
>-== Same-Origin to Cross-Origin ==
>+== Same-Origin to Cross-Origin, no-cors mode ==
> cssRules: null
> rules: null
>-== Cross-Origin to Same-Origin ==
>+== Cross-Origin to Same-Origin, no-cors mode ==
>+cssRules: null
>+rules: null
>+== Same-Origin to Same-Origin, cors mode ==
>+cssRules: [object CSSRuleList]
>+cssRules length: 1
>+rules: [object CSSRuleList]
>+rules length: 1
>+== Cross-Origin to Same-Origin, cors mode, successful cors check ==
> cssRules: [object CSSRuleList]
>+cssRules length: 1
> rules: [object CSSRuleList]
>+rules length: 1
>+== Cross-Origin to Same-Origin, cors mode, failing cors check ==
>+cssRules: null
>+rules: null
> Test ends.
>diff --git a/LayoutTests/http/tests/security/cannot-read-cssrules-redirect.html b/LayoutTests/http/tests/security/cannot-read-cssrules-redirect.html
>index a45a3850b295c335c5ef03203bd679fdd1744f72..cfef9b56962ca758b947ddee3af486504f16ab0d 100644
>--- a/LayoutTests/http/tests/security/cannot-read-cssrules-redirect.html
>+++ b/LayoutTests/http/tests/security/cannot-read-cssrules-redirect.html
>@@ -5,6 +5,12 @@
>       href="/resources/redirect.php?url=http://localhost:8000/security/resources/cssStyle.css">
> <link rel="stylesheet"
>       href="http://localhost:8000/resources/redirect.php?url=http://127.0.0.1:8000/security/resources/cssStyle.css">
>+<link rel="stylesheet"
>+      href="/resources/redirect.php?url=http://127.0.0.1:8000/security/resources/cssStyle.css" crossorigin="">
>+<link rel="stylesheet"
>+      href="http://localhost:8000/security/resources/redirect-allow-star.php?url=http://127.0.0.1:8000/security/resources/xorigincss1-allow-star.php" crossorigin="">
>+<link rel="stylesheet"
>+      href="http://localhost:8000/resources/redirect.php?url=http://127.0.0.1:8000/security/resources/xorigincss1.css" crossorigin="">
> <script>
> if (window.testRunner)
>     testRunner.dumpAsText();
>@@ -15,16 +21,38 @@ function log(msg) {
>     document.getElementById("console").appendChild(div);
> }
> 
>+function checkCSSRules(sheet)
>+{
>+    try {
>+        var cssRules = sheet.cssRules;
>+        log("cssRules: " + cssRules);
>+        if (cssRules)
>+            log("cssRules length: " + cssRules.length);
>+    } catch(e) {
>+        log("exception thrown for cssRules: " + e);
>+    }
>+    try {
>+        var rules = sheet.rules;
>+        log("rules: " + rules);
>+        if (rules)
>+            log("rules length: " + rules.length);
>+    } catch(e) {
>+        log("exception thrown for rules: " + e);
>+    }
>+}
>+
> window.onload = function() {
>     log("Test begins.");
>-    log("== Same-Origin to Cross-Origin ==");
>-    var sheet1 = document.styleSheets[0];
>-    log("cssRules: " + sheet1.cssRules);
>-    log("rules: " + sheet1.rules);
>-    log("== Cross-Origin to Same-Origin ==");
>-    var sheet2 = document.styleSheets[1];
>-    log("cssRules: " + sheet2.cssRules);
>-    log("rules: " + sheet2.rules);
>+    log("== Same-Origin to Cross-Origin, no-cors mode ==");
>+    checkCSSRules(document.styleSheets[0]);
>+    log("== Cross-Origin to Same-Origin, no-cors mode ==");
>+    checkCSSRules(document.styleSheets[1]);
>+    log("== Same-Origin to Same-Origin, cors mode ==");
>+    checkCSSRules(document.styleSheets[2]);
>+    log("== Cross-Origin to Same-Origin, cors mode, successful cors check ==");
>+    checkCSSRules(document.styleSheets[3]);
>+    log("== Cross-Origin to Same-Origin, cors mode, failing cors check ==");
>+    checkCSSRules(document.styleSheets[4]);
>     log("Test ends.");
> }
> </script>
>diff --git a/LayoutTests/http/tests/security/cannot-read-cssrules.html b/LayoutTests/http/tests/security/cannot-read-cssrules.html
>index 626092bd43d985f180d4fcfca8987d70d61040b5..63fd6ce180968e43810bdef8378aede03f6e51bb 100644
>--- a/LayoutTests/http/tests/security/cannot-read-cssrules.html
>+++ b/LayoutTests/http/tests/security/cannot-read-cssrules.html
>@@ -4,6 +4,10 @@
> <link rel="stylesheet"
>       href="http://localhost:8000/security/resources/cssStyle.css">
> <link rel="stylesheet" href="resources/cssStyle.css">
>+<link rel="stylesheet"
>+      href="http://localhost:8000/security/resources/cssStyle.css" crossorigin="">
>+<link rel="stylesheet"
>+      href="http://localhost:8000/security/resources/xorigincss1-allow-star.php" crossorigin="">
> <script>
> if (window.testRunner)
>     testRunner.dumpAsText();
>@@ -14,16 +18,36 @@ function log(msg) {
>     document.getElementById("console").appendChild(div);
> }
> 
>+function checkCSSRules(sheet)
>+{
>+    try {
>+        var cssRules = sheet.cssRules;
>+        log("cssRules: " + cssRules);
>+        if (cssRules)
>+            log("cssRules length: " + cssRules.length);
>+    } catch(e) {
>+        log("exception thrown for cssRules: " + e);
>+    }
>+    try {
>+        var rules = sheet.rules;
>+        log("rules: " + rules);
>+        if (rules)
>+            log("rules length: " + rules.length);
>+    } catch(e) {
>+        log("exception thrown for rules: " + e);
>+    }
>+}
>+
> window.onload = function() {
>     log("Test begins.");
>-    log("== Cross-Origin ==");
>-    var sheet1 = document.styleSheets[0];
>-    log("cssRules: " + sheet1.cssRules);
>-    log("rules: " + sheet1.rules);
>-    log("== Same-Origin ==");
>-    var sheet2 = document.styleSheets[1];
>-    log("cssRules: " + sheet2.cssRules);
>-    log("rules: " + sheet2.rules);
>+    log("== Cross-Origin resource, no-cors mode ==");
>+    checkCSSRules(document.styleSheets[0]);
>+    log("== Same-Origin resource, no-cors mode ==");
>+    checkCSSRules(document.styleSheets[1]);
>+    log("== Cross-Origin resource, cors mode, failing cors check ==");
>+    checkCSSRules(document.styleSheets[2]);
>+    log("== Cross-Origin resource, cors mode, successful cors check ==");
>+    checkCSSRules(document.styleSheets[3]);
>     log("Test ends.");
> }
> </script>
>diff --git a/LayoutTests/http/tests/security/cross-origin-css-9.html b/LayoutTests/http/tests/security/cross-origin-css-9.html
>index 61b250248aba91b48ff28b41ef4faee4477a7524..5b9061efb916d2a87eb8593731c13c27504746ff 100644
>--- a/LayoutTests/http/tests/security/cross-origin-css-9.html
>+++ b/LayoutTests/http/tests/security/cross-origin-css-9.html
>@@ -21,8 +21,7 @@ function checkCSSLoading(id, expectToIncludeOrigin, expectVisibility)
> {
>     expectedColor = expectToIncludeOrigin ? "rgb(255, 255, 0)" : "rgb(0, 0, 255)";
>     assert_equals(getBackgroundColorForId(id), expectedColor);
>-    // FIXME: cssRules should be made visible according resource tainted status, not only based on the resource URL being cross origin.
>-    // assert_equals(document.getElementById("link-" + id).sheet.cssRules !== null, expectVisibility);
>+    assert_equals(document.getElementById("link-" + id).sheet.cssRules !== null, expectVisibility);
> }
> 
> var onloadTest = async_test(" Testing that link element to load stylesheets correctly handle crossorigin mode.");
>diff --git a/LayoutTests/http/tests/security/resources/xorigincss1-allow-star.php b/LayoutTests/http/tests/security/resources/xorigincss1-allow-star.php
>new file mode 100644
>index 0000000000000000000000000000000000000000..4479844148f409d2047bc207bbe2e3f5a4460378
>--- /dev/null
>+++ b/LayoutTests/http/tests/security/resources/xorigincss1-allow-star.php
>@@ -0,0 +1,11 @@
>+<?php
>+header("Access-Control-Allow-Origin: *");
>+
>+$name = 'xorigincss1.css';
>+$fp = fopen($name, 'rb');
>+header("Content-Type: text/css");
>+header("Content-Length: " . filesize($name));
>+
>+fpassthru($fp);
>+exit;
>+?>

Actions: View | Formatted Diff | Diff

Attachments on bug 158728: 281251 | 281358 | 281836 | 281908 | 285101 | 287949 | 287953