Changeset 206717 in webkit


Ignore:
Timestamp:
Oct 2, 2016 7:25:08 AM (7 years ago)
Author:
commit-queue@webkit.org
Message:

Unreviewed, rolling out r206716.
https://bugs.webkit.org/show_bug.cgi?id=162858

It is breaking Mac CMake Debug build (Requested by youenn on
#webkit).

Reverted changeset:

"[Fetch API] Forbid redirection to non-HTTP(s) URL in non-
navigation mode."
https://bugs.webkit.org/show_bug.cgi?id=162785
http://trac.webkit.org/changeset/206716

Location:
trunk
Files:
9 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/imported/w3c/ChangeLog

    r206716 r206717  
     12016-10-02  Commit Queue  <commit-queue@webkit.org>
     2
     3        Unreviewed, rolling out r206716.
     4        https://bugs.webkit.org/show_bug.cgi?id=162858
     5
     6        It is breaking Mac CMake Debug build (Requested by youenn on
     7        #webkit).
     8
     9        Reverted changeset:
     10
     11        "[Fetch API] Forbid redirection to non-HTTP(s) URL in non-
     12        navigation mode."
     13        https://bugs.webkit.org/show_bug.cgi?id=162785
     14        http://trac.webkit.org/changeset/206716
     15
    1162016-10-02  Youenn Fablet  <youenn@apple.com>
    217
  • trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/redirect/redirect-to-dataurl-expected.txt

    r206716 r206717  
    55
    66PASS Testing data URL loading after same-origin redirection (cors mode)
    7 PASS Testing data URL loading after same-origin redirection (no-cors mode)
     7FAIL Testing data URL loading after same-origin redirection (no-cors mode) assert_unreached: Should have rejected. Reached unreachable code
    88PASS Testing data URL loading after same-origin redirection (same-origin mode)
    99PASS Testing data URL loading after cross-origin redirection (cors mode)
    10 PASS Testing data URL loading after cross-origin redirection (no-cors mode)
     10FAIL Testing data URL loading after cross-origin redirection (no-cors mode) assert_unreached: Should have rejected. Reached unreachable code
    1111
  • trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/redirect/redirect-to-dataurl-worker-expected.txt

    r206716 r206717  
    55
    66PASS Testing data URL loading after same-origin redirection (cors mode)
    7 PASS Testing data URL loading after same-origin redirection (no-cors mode)
     7FAIL Testing data URL loading after same-origin redirection (no-cors mode) assert_unreached: Should have rejected. Reached unreachable code
    88PASS Testing data URL loading after same-origin redirection (same-origin mode)
    99PASS Testing data URL loading after cross-origin redirection (cors mode)
    10 PASS Testing data URL loading after cross-origin redirection (no-cors mode)
     10FAIL Testing data URL loading after cross-origin redirection (no-cors mode) assert_unreached: Should have rejected. Reached unreachable code
    1111
  • trunk/Source/WebCore/ChangeLog

    r206716 r206717  
     12016-10-02  Commit Queue  <commit-queue@webkit.org>
     2
     3        Unreviewed, rolling out r206716.
     4        https://bugs.webkit.org/show_bug.cgi?id=162858
     5
     6        It is breaking Mac CMake Debug build (Requested by youenn on
     7        #webkit).
     8
     9        Reverted changeset:
     10
     11        "[Fetch API] Forbid redirection to non-HTTP(s) URL in non-
     12        navigation mode."
     13        https://bugs.webkit.org/show_bug.cgi?id=162785
     14        http://trac.webkit.org/changeset/206716
     15
    1162016-10-02  Youenn Fablet  <youenn@apple.com>
    217
  • trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj

    r206716 r206717  
    57925792                CE2849891CA3614600B4A57F /* ContentSecurityPolicyDirectiveNames.cpp in Sources */ = {isa = PBXBuildFile; fileRef = CE2849881CA3614600B4A57F /* ContentSecurityPolicyDirectiveNames.cpp */; };
    57935793                CE6DADF91C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.cpp in Sources */ = {isa = PBXBuildFile; fileRef = CE6DADF71C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.cpp */; };
    5794                 CE6DADFA1C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.h in Headers */ = {isa = PBXBuildFile; fileRef = CE6DADF81C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.h */; settings = {ATTRIBUTES = (Private, ); }; };
     5794                CE6DADFA1C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.h in Headers */ = {isa = PBXBuildFile; fileRef = CE6DADF81C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.h */; };
    57955795                CE799F971C6A46BC0097B518 /* ContentSecurityPolicySourceList.cpp in Sources */ = {isa = PBXBuildFile; fileRef = CE799F951C6A46BC0097B518 /* ContentSecurityPolicySourceList.cpp */; };
    57965796                CE799F981C6A46BC0097B518 /* ContentSecurityPolicySourceList.h in Headers */ = {isa = PBXBuildFile; fileRef = CE799F961C6A46BC0097B518 /* ContentSecurityPolicySourceList.h */; };
  • trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp

    r206716 r206717  
    217217}
    218218
    219 static inline void reportRedirectionWithBadScheme(ThreadableLoaderClient& client, const URL& url)
    220 {
    221     client.didFail(ResourceError(errorDomainWebKitInternal, 0, url, "Redirection to URL with a scheme that is not HTTP(S).", ResourceError::Type::AccessControl));
    222 }
    223 
    224219void DocumentThreadableLoader::redirectReceived(CachedResource* resource, ResourceRequest& request, const ResourceResponse& redirectResponse)
    225220{
     
    228223
    229224    Ref<DocumentThreadableLoader> protectedThis(*this);
    230 
    231     // FIXME: We restrict this check to Fetch API for the moment, as this might disrupt WorkerScriptLoader.
    232     // Reassess this check based on https://github.com/whatwg/fetch/issues/393 discussions.
    233     // We should also disable that check in navigation mode.
    234     if (!request.url().protocolIsInHTTPFamily() && m_options.initiator == cachedResourceRequestInitiators().fetch) {
    235         reportRedirectionWithBadScheme(*m_client, request.url());
    236         clearResource();
    237         return;
    238     }
    239 
    240225    if (!isAllowedByContentSecurityPolicy(request.url(), redirectResponse.isNull() ? ContentSecurityPolicy::RedirectResponseReceived::No : ContentSecurityPolicy::RedirectResponseReceived::Yes)) {
    241226        reportContentSecurityPolicyError(*m_client, redirectResponse.url());
  • trunk/Source/WebCore/loader/SubresourceLoader.cpp

    r206716 r206717  
    201201        }
    202202
    203         if (!m_documentLoader->cachedResourceLoader().canRequestAfterRedirection(m_resource->type(), newRequest.url(), options())) {
     203        if (!m_documentLoader->cachedResourceLoader().canRequest(m_resource->type(), newRequest.url(), options(), false /* forPreload */, true /* didReceiveRedirectResponse */)) {
    204204            cancel();
    205205            return;
  • trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp

    r206716 r206717  
    184184                document->contentSecurityPolicy()->upgradeInsecureRequestIfNeeded(request.mutableResourceRequest(), ContentSecurityPolicy::InsecureRequestType::Load);
    185185            URL requestURL = request.resourceRequest().url();
    186             if (requestURL.isValid() && canRequest(CachedResource::ImageResource, requestURL, request))
     186            if (requestURL.isValid() && canRequest(CachedResource::ImageResource, requestURL, request.options(), request.forPreload()))
    187187                PingLoader::loadImage(*frame, requestURL);
    188188            return nullptr;
     
    334334bool CachedResourceLoader::checkInsecureContent(CachedResource::Type type, const URL& url) const
    335335{
    336 
    337     if (!canRequestInContentDispositionAttachmentSandbox(type, url))
    338         return false;
    339 
    340336    switch (type) {
    341337    case CachedResource::Script:
     
    384380}
    385381
    386 bool CachedResourceLoader::allowedByContentSecurityPolicy(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options, ContentSecurityPolicy::RedirectResponseReceived redirectResponseReceived)
     382static inline bool isSameOriginDataURL(const URL& url, const ResourceLoaderOptions& options, bool didReceiveRedirectResponse)
     383{
     384    return !didReceiveRedirectResponse && url.protocolIsData() && options.sameOriginDataURLFlag == SameOriginDataURLFlag::Set;
     385}
     386
     387bool CachedResourceLoader::allowedByContentSecurityPolicy(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options, bool didReceiveRedirectResponse)
    387388{
    388389    if (options.contentSecurityPolicyImposition == ContentSecurityPolicyImposition::SkipPolicyCheck)
     
    391392    ASSERT(m_document);
    392393    ASSERT(m_document->contentSecurityPolicy());
     394
     395    auto redirectResponseReceived = didReceiveRedirectResponse ? ContentSecurityPolicy::RedirectResponseReceived::Yes : ContentSecurityPolicy::RedirectResponseReceived::No;
    393396
    394397    switch (type) {
     
    428431        ASSERT_NOT_REACHED();
    429432    }
    430 
    431433    return true;
    432434}
    433435
    434 static inline bool isSameOriginDataURL(const URL& url, const ResourceLoaderOptions& options)
    435 {
    436     // FIXME: Remove same-origin data URL flag since it was removed from fetch spec (https://github.com/whatwg/fetch/issues/381).
    437     return url.protocolIsData() && options.sameOriginDataURLFlag == SameOriginDataURLFlag::Set;
    438 }
    439 
    440 bool CachedResourceLoader::canRequest(CachedResource::Type type, const URL& url, const CachedResourceRequest& request)
    441 {
    442     auto& options = request.options();
    443 
     436bool CachedResourceLoader::canRequest(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options, bool forPreload, bool didReceiveRedirectResponse)
     437{
    444438    if (document() && !document()->securityOrigin()->canDisplay(url)) {
    445         if (!request.forPreload())
     439        if (!forPreload)
    446440            FrameLoader::reportLocalLoadFailed(frame(), url.stringCenterEllipsizedToLength());
    447441        LOG(ResourceLoading, "CachedResourceLoader::requestResource URL was not allowed by SecurityOrigin::canDisplay");
     
    449443    }
    450444
    451     if (options.mode == FetchOptions::Mode::SameOrigin && !m_document->securityOrigin()->canRequest(url) && !isSameOriginDataURL(url, options)) {
     445    // FIXME: Remove same-origin data URL flag since it was removed from fetch spec (see https://github.com/whatwg/fetch/issues/381).
     446    if (options.mode == FetchOptions::Mode::SameOrigin && !isSameOriginDataURL(url, options, didReceiveRedirectResponse) && !m_document->securityOrigin()->canRequest(url)) {
    452447        printAccessDeniedMessage(url);
    453448        return false;
    454449    }
    455450
    456     if (!allowedByContentSecurityPolicy(type, url, options, ContentSecurityPolicy::RedirectResponseReceived::No))
     451    if (!allowedByContentSecurityPolicy(type, url, options, didReceiveRedirectResponse))
    457452        return false;
    458453
     
    463458    }
    464459
    465     // Last of all, check for insecure content. We do this last so that when folks block insecure content with a CSP policy, they don't get a warning.
     460    if (!canRequestInContentDispositionAttachmentSandbox(type, url))
     461        return false;
     462
     463    // Last of all, check for insecure content. We do this last so that when
     464    // folks block insecure content with a CSP policy, they don't get a warning.
    466465    // They'll still get a warning in the console about CSP blocking the load.
    467466
    468     // FIXME: Should we consider whether the request is for preload here?
     467    // FIXME: Should we consider forPreload here?
    469468    if (!checkInsecureContent(type, url))
    470469        return false;
     
    473472}
    474473
    475 // FIXME: Should we find a way to know whether the redirection is for a preload request like we do for CachedResourceLoader::canRequest?
    476 bool CachedResourceLoader::canRequestAfterRedirection(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options)
    477 {
    478     if (document() && !document()->securityOrigin()->canDisplay(url)) {
    479         FrameLoader::reportLocalLoadFailed(frame(), url.stringCenterEllipsizedToLength());
    480         LOG(ResourceLoading, "CachedResourceLoader::requestResource URL was not allowed by SecurityOrigin::canDisplay");
    481         return false;
    482     }
    483 
    484     // FIXME: According to https://fetch.spec.whatwg.org/#http-redirect-fetch, we should check that the URL is HTTP(s) except if in navigation mode.
    485     // But we currently allow at least data URLs to be loaded.
    486 
    487     if (options.mode == FetchOptions::Mode::SameOrigin && !m_document->securityOrigin()->canRequest(url)) {
    488         printAccessDeniedMessage(url);
    489         return false;
    490     }
    491 
    492     if (!allowedByContentSecurityPolicy(type, url, options, ContentSecurityPolicy::RedirectResponseReceived::Yes))
    493         return false;
    494 
    495     // Last of all, check for insecure content. We do this last so that when folks block insecure content with a CSP policy, they don't get a warning.
    496     // They'll still get a warning in the console about CSP blocking the load.
    497     if (!checkInsecureContent(type, url))
    498         return false;
    499 
    500     return true;
    501 }
    502 
    503474bool CachedResourceLoader::canRequestInContentDispositionAttachmentSandbox(CachedResource::Type type, const URL& url) const
    504475{
    505476    Document* document;
    506 
     477   
    507478    // FIXME: Do we want to expand this to all resource types that the mixed content checker would consider active content?
    508479    switch (type) {
     
    654625    prepareFetch(type, request);
    655626
    656     // We are passing url as well as request, as request url may contain a fragment identifier.
    657     if (!canRequest(type, url, request)) {
     627    if (!canRequest(type, url, request.options(), request.forPreload())) {
    658628        RELEASE_LOG_IF_ALLOWED("requestResource: Not allowed to request resource (frame = %p)", frame());
    659629        return nullptr;
  • trunk/Source/WebCore/loader/cache/CachedResourceLoader.h

    r206716 r206717  
    3131#include "CachedResourceHandle.h"
    3232#include "CachedResourceRequest.h"
    33 #include "ContentSecurityPolicy.h"
    3433#include "ResourceLoadPriority.h"
    3534#include "ResourceTimingInformation.h"
     
    124123
    125124    WEBCORE_EXPORT void garbageCollectDocumentResources();
    126 
     125   
    127126    void incrementRequestCount(const CachedResource&);
    128127    void decrementRequestCount(const CachedResource&);
     
    137136    void printPreloadStats();
    138137
    139     bool canRequest(CachedResource::Type, const URL&, const CachedResourceRequest&);
    140     bool canRequestAfterRedirection(CachedResource::Type, const URL&, const ResourceLoaderOptions&);
     138    bool canRequest(CachedResource::Type, const URL&, const ResourceLoaderOptions&, bool forPreload = false, bool didReceiveRedirectResponse = false);
    141139
    142140    static const ResourceLoaderOptions& defaultCachedResourceOptions();
     
    167165    bool shouldContinueAfterNotifyingLoadedFromMemoryCache(const CachedResourceRequest&, CachedResource*);
    168166    bool checkInsecureContent(CachedResource::Type, const URL&) const;
    169     bool allowedByContentSecurityPolicy(CachedResource::Type, const URL&, const ResourceLoaderOptions&, ContentSecurityPolicy::RedirectResponseReceived);
     167    bool allowedByContentSecurityPolicy(CachedResource::Type, const URL&, const ResourceLoaderOptions&, bool);
    170168
    171169    void performPostLoadActions();
Note: See TracChangeset for help on using the changeset viewer.