Changeset 202542 in webkit


Ignore:
Timestamp:
Jun 27, 2016 11:09:05 PM (8 years ago)
Author:
commit-queue@webkit.org
Message:

Remove didFailAccessControlCheck ThreadableLoaderClient callback
​https://bugs.webkit.org/show_bug.cgi?id=159149

Patch by Youenn Fablet <​youenn@apple.com> on 2016-06-27
Reviewed by Daniel Bates.

LayoutTests/imported/w3c:

  • web-platform-tests/XMLHttpRequest/send-authentication-cors-setrequestheader-no-cred-expected.txt:

Source/WebCore:

Adding an AccessControl ResourceError type.
Replacing didFailAccessControlCheck callback by a direct call to didFail with an error of type AccessControl.

Making CrossOriginPreflightChecker always return an AccessControl error. Previously some errors created below
were passed directly to threadable loader clients.

When doing preflight on unauthorized web sites, WTR/DRT will trigger a cancellation error which was translating into an abort event in XMLHttpRequest.
This patch is changing the error type to AccessControl, which translates into an error event in XMLHttpReauest.

This change of behavior is seen in imported/w3c/web-platform-tests/XMLHttpRequest/send-authentication-cors-setrequestheader-no-cred.htm.
No other observable change of behavior should be expected.

  • inspector/InspectorNetworkAgent.cpp: Computing error message in didFail according the error type.
  • loader/CrossOriginPreflightChecker.cpp:

(WebCore::CrossOriginPreflightChecker::validatePreflightResponse): Setting preflightFailure error type to AccessControl.
(WebCore::CrossOriginPreflightChecker::notifyFinished): Ditto.
(WebCore::CrossOriginPreflightChecker::doPreflight): Ditto.

  • loader/DocumentThreadableLoader.cpp:

(WebCore::DocumentThreadableLoader::makeSimpleCrossOriginAccessRequest): Replacing didFailAccessControlCheck
callback by a direct call to didFail with an error of type AccessControl.
(WebCore::reportContentSecurityPolicyError): Ditto.
(WebCore::reportCrossOriginResourceSharingError): Ditto.
(WebCore::DocumentThreadableLoader::didReceiveResponse): Ditto.
(WebCore::DocumentThreadableLoader::preflightFailure): Calling didFail directly.

  • loader/ThreadableLoaderClient.h: Removing didFailAccessControlCheck.
  • loader/ThreadableLoaderClientWrapper.h: Ditto.
  • loader/WorkerThreadableLoader.cpp: Ditto.
  • loader/WorkerThreadableLoader.h: Ditto.
  • page/EventSource.cpp:

(WebCore::EventSource::didFail): Removing didFailAccessControlCheck and putting handling code in didFail.

  • page/EventSource.h:
  • platform/network/ResourceErrorBase.cpp:

(WebCore::ResourceErrorBase::setType): Softening the assertion to cover the case of migration to AccessControl.

  • platform/network/ResourceErrorBase.h: Adding AccessControl error type.

(WebCore::ResourceErrorBase::isAccessControl):

Location:
trunk
Files:
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r202539 r202542  
     12016-06-27  Youenn Fablet  <youenn@apple.com>
     2
     3        Remove didFailAccessControlCheck ThreadableLoaderClient callback
     4        https://bugs.webkit.org/show_bug.cgi?id=159149
     5
     6        Reviewed by Daniel Bates.
     7
     8        * web-platform-tests/XMLHttpRequest/send-authentication-cors-setrequestheader-no-cred-expected.txt:
     9
    1102016-06-27  Chris Dumez  <cdumez@apple.com>
    211

  • trunk/LayoutTests/imported/w3c/web-platform-tests/XMLHttpRequest/send-authentication-cors-setrequestheader-no-cred-expected.txt

    r200309 r202542  
    33
    44FAIL CORS request with setRequestHeader auth to URL accepting Authorization header assert_true: responseText should contain the right user and password expected true got false
    5 FAIL CORS request with setRequestHeader auth to URL NOT accepting Authorization header assert_true: The error event should fire expected true got false
     5PASS CORS request with setRequestHeader auth to URL NOT accepting Authorization header
    66

  • trunk/Source/WebCore/ChangeLog

    r202539 r202542  
     12016-06-27  Youenn Fablet  <youenn@apple.com>
     2
     3        Remove didFailAccessControlCheck ThreadableLoaderClient callback
     4        https://bugs.webkit.org/show_bug.cgi?id=159149
     5
     6        Reviewed by Daniel Bates.
     7
     8        Adding an AccessControl ResourceError type.
     9        Replacing didFailAccessControlCheck callback by a direct call to didFail with an error of type AccessControl.
     10
     11        Making CrossOriginPreflightChecker always return an AccessControl error. Previously some errors created below
     12        were passed directly to threadable loader clients.
     13
     14        When doing preflight on unauthorized web sites, WTR/DRT will trigger a cancellation error which was translating into an abort event in XMLHttpRequest.
     15        This patch is changing the error type to AccessControl, which translates into an error event in XMLHttpReauest.
     16
     17        This change of behavior is seen in imported/w3c/web-platform-tests/XMLHttpRequest/send-authentication-cors-setrequestheader-no-cred.htm.
     18        No other observable change of behavior should be expected.
     19
     20        * inspector/InspectorNetworkAgent.cpp: Computing error message in didFail according the error type.
     21        * loader/CrossOriginPreflightChecker.cpp:
     22        (WebCore::CrossOriginPreflightChecker::validatePreflightResponse): Setting preflightFailure error type to AccessControl.
     23        (WebCore::CrossOriginPreflightChecker::notifyFinished): Ditto.
     24        (WebCore::CrossOriginPreflightChecker::doPreflight): Ditto.
     25        * loader/DocumentThreadableLoader.cpp:
     26        (WebCore::DocumentThreadableLoader::makeSimpleCrossOriginAccessRequest): Replacing didFailAccessControlCheck
     27        callback by a direct call to didFail with an error of type AccessControl.
     28        (WebCore::reportContentSecurityPolicyError): Ditto.
     29        (WebCore::reportCrossOriginResourceSharingError): Ditto.
     30        (WebCore::DocumentThreadableLoader::didReceiveResponse): Ditto.
     31        (WebCore::DocumentThreadableLoader::preflightFailure): Calling didFail directly.
     32        * loader/ThreadableLoaderClient.h: Removing didFailAccessControlCheck.
     33        * loader/ThreadableLoaderClientWrapper.h: Ditto.
     34        * loader/WorkerThreadableLoader.cpp: Ditto.
     35        * loader/WorkerThreadableLoader.h: Ditto.
     36        * page/EventSource.cpp:
     37        (WebCore::EventSource::didFail): Removing didFailAccessControlCheck and putting handling code in didFail.
     38        * page/EventSource.h:
     39        * platform/network/ResourceErrorBase.cpp:
     40        (WebCore::ResourceErrorBase::setType): Softening the assertion to cover the case of migration to AccessControl.
     41        * platform/network/ResourceErrorBase.h: Adding AccessControl error type.
     42        (WebCore::ResourceErrorBase::isAccessControl):
     43
    1442016-06-27  Chris Dumez  <cdumez@apple.com>
    245

  • trunk/Source/WebCore/inspector/InspectorNetworkAgent.cpp

    r202480 r202542  
    123123    }
    124124
    125     void didFail(const ResourceError&) override
     125    void didFail(const ResourceError& error) override
    126126    {
    127         m_callback->sendFailure(ASCIILiteral("Loading resource for inspector failed"));
    128         dispose();
    129     }
    130 
    131     void didFailAccessControlCheck(const ResourceError&) final
    132     {
    133         m_callback->sendFailure(ASCIILiteral("Loading resource for inspector failed access control check"));
     127        m_callback->sendFailure(error.isAccessControl() ? ASCIILiteral("Loading resource for inspector failed access control check") : ASCIILiteral("Loading resource for inspector failed"));
    134128        dispose();
    135129    }

  • trunk/Source/WebCore/loader/CrossOriginPreflightChecker.cpp

    r202336 r202542  
    6565
    6666    if (!response.isSuccessful()) {
    67         loader.preflightFailure(identifier, ResourceError(errorDomainWebKitInternal, 0, request.url(), ASCIILiteral("Preflight response is not successful")));
     67        loader.preflightFailure(identifier, ResourceError(errorDomainWebKitInternal, 0, request.url(), ASCIILiteral("Preflight response is not successful"), ResourceError::Type::AccessControl));
    6868        return;
    6969    }
    … …  
    7171    String description;
    7272    if (!passesAccessControlCheck(response, loader.options().allowCredentials(), loader.securityOrigin(), description)) {
    73         loader.preflightFailure(identifier, ResourceError(errorDomainWebKitInternal, 0, request.url(), description));
     73        loader.preflightFailure(identifier, ResourceError(errorDomainWebKitInternal, 0, request.url(), description, ResourceError::Type::AccessControl));
    7474        return;
    7575    }
    … …  
    7979        || !result->allowsCrossOriginMethod(request.httpMethod(), description)
    8080        || !result->allowsCrossOriginHeaders(request.httpHeaderFields(), description)) {
    81         loader.preflightFailure(identifier, ResourceError(errorDomainWebKitInternal, 0, request.url(), description));
     81        loader.preflightFailure(identifier, ResourceError(errorDomainWebKitInternal, 0, request.url(), description, ResourceError::Type::AccessControl));
    8282        return;
    8383    }
    … …  
    9191    ASSERT_UNUSED(resource, resource == m_resource);
    9292    if (m_resource->loadFailedOrCanceled()) {
    93         m_loader.preflightFailure(m_resource->identifier(), m_resource->resourceError());
     93        ResourceError preflightError = m_resource->resourceError();
     94        preflightError.setType(ResourceError::Type::AccessControl);
     95        m_loader.preflightFailure(m_resource->identifier(), preflightError);
    9496        return;
    9597    }
    … …  
    132134
    133135    if (!error.isNull() && response.httpStatusCode() <= 0) {
     136        error.setType(ResourceError::Type::AccessControl);
    134137        loader.preflightFailure(identifier, error);
    135138        return;

  • trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp

    r202480 r202542  
    134134    // Cross-origin requests are only allowed for HTTP and registered schemes. We would catch this when checking response headers later, but there is no reason to send a request that's guaranteed to be denied.
    135135    if (!SchemeRegistry::shouldTreatURLSchemeAsCORSEnabled(request.url().protocol())) {
    136         m_client->didFailAccessControlCheck(ResourceError(errorDomainWebKitInternal, 0, request.url(), "Cross origin requests are only supported for HTTP."));
     136        m_client->didFail(ResourceError(errorDomainWebKitInternal, 0, request.url(), "Cross origin requests are only supported for HTTP.", ResourceError::Type::AccessControl));
    137137        return;
    138138    }
    … …  
    195195static inline void reportContentSecurityPolicyError(ThreadableLoaderClient& client, const URL& url)
    196196{
    197     client.didFailAccessControlCheck(ResourceError(errorDomainWebKitInternal, 0, url, "Cross-origin redirection denied by Content Security Policy."));
     197    client.didFail(ResourceError(errorDomainWebKitInternal, 0, url, "Cross-origin redirection denied by Content Security Policy.", ResourceError::Type::AccessControl));
    198198}
    199199
    200200static inline void reportCrossOriginResourceSharingError(ThreadableLoaderClient& client, const URL& url)
    201201{
    202     client.didFailAccessControlCheck(ResourceError(errorDomainWebKitInternal, 0, url, "Cross-origin redirection denied by Cross-Origin Resource Sharing policy."));
     202    client.didFail(ResourceError(errorDomainWebKitInternal, 0, url, "Cross-origin redirection denied by Cross-Origin Resource Sharing policy.", ResourceError::Type::AccessControl));
    203203}
    204204
    … …  
    280280    if (!m_sameOriginRequest && m_options.crossOriginRequestPolicy == UseAccessControl) {
    281281        if (!passesAccessControlCheck(response, m_options.allowCredentials(), securityOrigin(), accessControlErrorDescription)) {
    282             m_client->didFailAccessControlCheck(ResourceError(errorDomainWebKitInternal, 0, response.url(), accessControlErrorDescription));
     282            m_client->didFail(ResourceError(errorDomainWebKitInternal, 0, response.url(), accessControlErrorDescription, ResourceError::Type::AccessControl));
    283283            return;
    284284        }
    … …  
    337337void DocumentThreadableLoader::preflightFailure(unsigned long identifier, const ResourceError& error)
    338338{
     339    ASSERT(error.isAccessControl());
    339340    m_preflightChecker = Nullopt;
    340341
    … …  
    342343
    343344    ASSERT(m_client);
    344     m_client->didFailAccessControlCheck(error);
     345    m_client->didFail(error);
    345346}
    346347

  • trunk/Source/WebCore/loader/ThreadableLoaderClient.h

    r202480 r202542  
    4747        virtual void didFinishLoading(unsigned long /*identifier*/, double /*finishTime*/) { }
    4848        virtual void didFail(const ResourceError&) { }
    49         virtual void didFailAccessControlCheck(const ResourceError& error) { didFail(error); }
    5049
    5150    protected:

  • trunk/Source/WebCore/loader/ThreadableLoaderClientWrapper.h

    r202480 r202542  
    8989    }
    9090
    91     void didFailAccessControlCheck(const ResourceError& error)
    92     {
    93         m_done = true;
    94         if (m_client)
    95             m_client->didFailAccessControlCheck(error);
    96     }
    97 
    9891    void didReceiveAuthenticationCancellation(unsigned long identifier, const ResourceResponse& response)
    9992    {

  • trunk/Source/WebCore/loader/WorkerThreadableLoader.cpp

    r202480 r202542  
    198198}
    199199
    200 void WorkerThreadableLoader::MainThreadBridge::didFailAccessControlCheck(const ResourceError& error)
    201 {
    202     m_loadingFinished = true;
    203     m_loaderProxy.postTaskForModeToWorkerGlobalScope([workerClientWrapper = Ref<ThreadableLoaderClientWrapper>(*m_workerClientWrapper), error = error.isolatedCopy()] (ScriptExecutionContext& context) mutable {
    204         ASSERT_UNUSED(context, context.isWorkerGlobalScope());
    205         workerClientWrapper->didFailAccessControlCheck(error);
    206     }, m_taskMode);
    207 }
    208 
    209200} // namespace WebCore

  • trunk/Source/WebCore/loader/WorkerThreadableLoader.h

    r202480 r202542  
    105105            void didFinishLoading(unsigned long identifier, double finishTime) override;
    106106            void didFail(const ResourceError&) override;
    107             void didFailAccessControlCheck(const ResourceError&) override;
    108107
    109108            // Only to be used on the main thread.

  • trunk/Source/WebCore/page/EventSource.cpp

    r202480 r202542  
    244244{
    245245    ASSERT(m_state != CLOSED);
     246
     247    if (error.isAccessControl()) {
     248        String message = makeString("EventSource cannot load ", error.failingURL().string(), ". ", error.localizedDescription());
     249        scriptExecutionContext()->addConsoleMessage(MessageSource::JS, MessageLevel::Error, message);
     250
     251        abortConnectionAttempt();
     252        return;
     253    }
     254
    246255    ASSERT(m_requestInFlight);
    247256
    … …  
    252261
    253262    networkRequestEnded();
    254 }
    255 
    256 void EventSource::didFailAccessControlCheck(const ResourceError& error)
    257 {
    258     String message = makeString("EventSource cannot load ", error.failingURL().string(), ". ", error.localizedDescription());
    259     scriptExecutionContext()->addConsoleMessage(MessageSource::JS, MessageLevel::Error, message);
    260 
    261     abortConnectionAttempt();
    262263}
    263264

  • trunk/Source/WebCore/page/EventSource.h

    r202480 r202542  
    8282    void didFinishLoading(unsigned long, double) final;
    8383    void didFail(const ResourceError&) final;
    84     void didFailAccessControlCheck(const ResourceError&) final;
    8584
    8685    void stop() final;

  • trunk/Source/WebCore/platform/network/ResourceErrorBase.cpp

    r201856 r202542  
    6060void ResourceErrorBase::setType(Type type)
    6161{
    62     ASSERT(m_type == Type::General || m_type == Type::Null);
     62    // setType should only be used to specialize the error type.
     63    ASSERT(m_type == Type::General || m_type == Type::Null || (m_type == Type::Cancellation && type == Type::AccessControl));
    6364    m_type = type;
    6465}

  • trunk/Source/WebCore/platform/network/ResourceErrorBase.h

    r201856 r202542  
    4848        Null,
    4949        General,
     50        AccessControl,
    5051        Cancellation,
    5152        Timeout
    … …  
    5354
    5455    bool isNull() const { return m_type == Type::Null; }
     56    bool isAccessControl() const { return m_type == Type::AccessControl; }
    5557    bool isCancellation() const { return m_type == Type::Cancellation; }
    5658    bool isTimeout() const { return m_type == Type::Timeout; }
Note: See TracChangeset for help on using the changeset viewer.