Changeset 287039 in webkit


Ignore:
Timestamp:
Dec 14, 2021, 12:06:12 PM (3 years ago)
Author:
achristensen@apple.com
Message:

Move FTP disabling from NetworkLoad::start to NetworkDataTask::NetworkDataTask
​https://bugs.webkit.org/show_bug.cgi?id=234280
Source/WebKit:

<rdar://85015428>

Reviewed by Brady Eidson.

A NetworkLoad synchronously failing has caused several issues.
This issue is when a WKDownload is prevented in this way, we don't have a WKDownload in the UI process yet,
and we haven't assigned it an identifier in the network process yet either, so we dereference null and send
unreceived messages in many places. To fix this issue, I move the FTP check to the NetworkDataTask constructor
like we do with blocked port checks and invalid URL checks. I also found that there is a race condition with
CFNetwork also failing to load an empty FTP URL, so we need to prevent us from asking CFNetwork to start a load
for which we have scheduled a failure.

Covered by an API test which used to hit all the horrible failures and now passes reliably.

  • NetworkProcess/NetworkCORSPreflightChecker.cpp:

(WebKit::NetworkCORSPreflightChecker::wasBlockedByDisabledFTP):

  • NetworkProcess/NetworkCORSPreflightChecker.h:
  • NetworkProcess/NetworkDataTask.cpp:

(WebKit::NetworkDataTask::NetworkDataTask):
(WebKit::NetworkDataTask::scheduleFailure):

  • NetworkProcess/NetworkDataTask.h:
  • NetworkProcess/NetworkLoad.cpp:

(WebKit::NetworkLoad::start):
(WebKit::NetworkLoad::wasBlockedByDisabledFTP):

  • NetworkProcess/NetworkLoad.h:
  • NetworkProcess/PingLoad.cpp:

(WebKit::PingLoad::wasBlockedByDisabledFTP):

  • NetworkProcess/PingLoad.h:
  • NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:

(WebKit::NetworkDataTaskCocoa::resume):

  • Shared/WebErrors.cpp:

(WebKit::ftpDisabledError):

  • Shared/WebErrors.h:

Tools:

Reviewed by Brady Eidson.

  • TestWebKitAPI/Tests/WebKitCocoa/Download.mm:
Location:
trunk
Files:
15 edited

Legend:

Unmodified
Added
Removed

  • TabularUnified trunk/Source/WebKit/ChangeLog

    r287035 r287039  
     12021-12-14  Alex Christensen  <achristensen@webkit.org>
     2
     3        Move FTP disabling from NetworkLoad::start to NetworkDataTask::NetworkDataTask
     4        https://bugs.webkit.org/show_bug.cgi?id=234280
     5        <rdar://85015428>
     6
     7        Reviewed by Brady Eidson.
     8
     9        A NetworkLoad synchronously failing has caused several issues.
     10        This issue is when a WKDownload is prevented in this way, we don't have a WKDownload in the UI process yet,
     11        and we haven't assigned it an identifier in the network process yet either, so we dereference null and send
     12        unreceived messages in many places.  To fix this issue, I move the FTP check to the NetworkDataTask constructor
     13        like we do with blocked port checks and invalid URL checks.  I also found that there is a race condition with
     14        CFNetwork also failing to load an empty FTP URL, so we need to prevent us from asking CFNetwork to start a load
     15        for which we have scheduled a failure.
     16
     17        Covered by an API test which used to hit all the horrible failures and now passes reliably.
     18
     19        * NetworkProcess/NetworkCORSPreflightChecker.cpp:
     20        (WebKit::NetworkCORSPreflightChecker::wasBlockedByDisabledFTP):
     21        * NetworkProcess/NetworkCORSPreflightChecker.h:
     22        * NetworkProcess/NetworkDataTask.cpp:
     23        (WebKit::NetworkDataTask::NetworkDataTask):
     24        (WebKit::NetworkDataTask::scheduleFailure):
     25        * NetworkProcess/NetworkDataTask.h:
     26        * NetworkProcess/NetworkLoad.cpp:
     27        (WebKit::NetworkLoad::start):
     28        (WebKit::NetworkLoad::wasBlockedByDisabledFTP):
     29        * NetworkProcess/NetworkLoad.h:
     30        * NetworkProcess/PingLoad.cpp:
     31        (WebKit::PingLoad::wasBlockedByDisabledFTP):
     32        * NetworkProcess/PingLoad.h:
     33        * NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
     34        (WebKit::NetworkDataTaskCocoa::resume):
     35        * Shared/WebErrors.cpp:
     36        (WebKit::ftpDisabledError):
     37        * Shared/WebErrors.h:
     38
    1392021-12-14  Tim Horton  <timothy_horton@apple.com>
    240

  • TabularUnified trunk/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp

    r287021 r287039  
    3333#include "NetworkProcess.h"
    3434#include "NetworkResourceLoader.h"
     35#include "WebErrors.h"
    3536#include <WebCore/CrossOriginAccessControl.h>
    3637#include <WebCore/SecurityOrigin.h>
    … …  
    173174}
    174175
     176void NetworkCORSPreflightChecker::wasBlockedByDisabledFTP()
     177{
     178    m_completionCallback(ftpDisabledError(m_parameters.originalRequest));
     179}
     180
    175181NetworkTransactionInformation NetworkCORSPreflightChecker::takeInformation()
    176182{

  • TabularUnified trunk/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.h

    r287021 r287039  
    7878    void cannotShowURL() final;
    7979    void wasBlockedByRestrictions() final;
     80    void wasBlockedByDisabledFTP() final;
    8081
    8182    Parameters m_parameters;

  • TabularUnified trunk/Source/WebKit/NetworkProcess/NetworkDataTask.cpp

    r284301 r287039  
    7777
    7878    if (!requestWithCredentials.url().isValid()) {
    79         scheduleFailure(InvalidURLFailure);
     79        scheduleFailure(FailureType::InvalidURL);
    8080        return;
    8181    }
    8282
    8383    if (!portAllowed(requestWithCredentials.url())) {
    84         scheduleFailure(BlockedFailure);
     84        scheduleFailure(FailureType::Blocked);
     85        return;
     86    }
     87
     88    if (!session.networkProcess().ftpEnabled()
     89        && requestWithCredentials.url().protocolIsInFTPFamily()) {
     90        scheduleFailure(FailureType::FTPDisabled);
    8591        return;
    8692    }
    … …  
    95101void NetworkDataTask::scheduleFailure(FailureType type)
    96102{
     103    m_failureScheduled = true;
    97104    RunLoop::main().dispatch([this, weakThis = WeakPtr { *this }, type] {
    98105        if (!weakThis || !m_client)
    … …  
    100107
    101108        switch (type) {
    102         case BlockedFailure:
     109        case FailureType::Blocked:
    103110            m_client->wasBlocked();
    104111            return;
    105         case InvalidURLFailure:
     112        case FailureType::InvalidURL:
    106113            m_client->cannotShowURL();
    107114            return;
    108         case RestrictedURLFailure:
     115        case FailureType::RestrictedURL:
    109116            m_client->wasBlockedByRestrictions();
    110117            return;
     118        case FailureType::FTPDisabled:
     119            m_client->wasBlockedByDisabledFTP();
    111120        }
    112121    });

  • TabularUnified trunk/Source/WebKit/NetworkProcess/NetworkDataTask.h

    r287021 r287039  
    7070    virtual void cannotShowURL() = 0;
    7171    virtual void wasBlockedByRestrictions() = 0;
     72    virtual void wasBlockedByDisabledFTP() = 0;
    7273
    7374    virtual bool shouldCaptureExtraNetworkLoadMetrics() const { return false; }
    … …  
    146147    NetworkDataTask(NetworkSession&, NetworkDataTaskClient&, const WebCore::ResourceRequest&, WebCore::StoredCredentialsPolicy, bool shouldClearReferrerOnHTTPSToHTTPRedirect, bool dataTaskIsForMainFrameNavigation);
    147148
    148     enum FailureType {
    149         BlockedFailure,
    150         InvalidURLFailure,
    151         RestrictedURLFailure
     149    enum class FailureType : uint8_t {
     150        Blocked,
     151        InvalidURL,
     152        RestrictedURL,
     153        FTPDisabled
    152154    };
    153155    void scheduleFailure(FailureType);
    … …  
    173175    String m_suggestedFilename;
    174176    bool m_dataTaskIsForMainFrameNavigation { false };
     177    bool m_failureScheduled { false };
    175178};
    176179

  • TabularUnified trunk/Source/WebKit/NetworkProcess/NetworkLoad.cpp

    r287021 r287039  
    6868    if (!m_task)
    6969        return;
    70 
    71     if (!m_networkProcess->ftpEnabled() && m_parameters.request.url().protocolIsInFTPFamily()) {
    72         m_task->clearClient();
    73         m_task = nullptr;
    74         WebCore::NetworkLoadMetrics emptyMetrics;
    75         didCompleteWithError(ResourceError { errorDomainWebKitInternal, 0, url(), "FTP URLs are disabled"_s, ResourceError::Type::AccessControl }, emptyMetrics);
    76         return;
    77     }
    78 
    7970    m_task->resume();
    8071}
    … …  
    304295}
    305296
     297void NetworkLoad::wasBlockedByDisabledFTP()
     298{
     299    m_client.get().didFailLoading(ftpDisabledError(m_currentRequest));
     300}
     301
    306302void NetworkLoad::didNegotiateModernTLS(const URL& url)
    307303{

  • TabularUnified trunk/Source/WebKit/NetworkProcess/NetworkLoad.h

    r287021 r287039  
    8888    void cannotShowURL() final;
    8989    void wasBlockedByRestrictions() final;
     90    void wasBlockedByDisabledFTP() final;
    9091    void didNegotiateModernTLS(const URL&) final;
    9192

  • TabularUnified trunk/Source/WebKit/NetworkProcess/PingLoad.cpp

    r287021 r287039  
    211211}
    212212
     213void PingLoad::wasBlockedByDisabledFTP()
     214{
     215    PING_RELEASE_LOG("wasBlockedByDisabledFTP");
     216    didFinish(ftpDisabledError(ResourceRequest(currentURL())));
     217}
     218
    213219void PingLoad::timeoutTimerFired()
    214220{

  • TabularUnified trunk/Source/WebKit/NetworkProcess/PingLoad.h

    r287021 r287039  
    6161    void cannotShowURL() final;
    6262    void wasBlockedByRestrictions() final;
     63    void wasBlockedByDisabledFTP() final;
    6364    void timeoutTimerFired();
    6465

  • TabularUnified trunk/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm

    r287021 r287039  
    640640    WTFEmitSignpost(m_task.get(), "DataTask", "resume");
    641641
     642    if (m_failureScheduled)
     643        return;
     644
    642645    auto& cocoaSession = static_cast<NetworkSessionCocoa&>(*m_session);
    643646    if (cocoaSession.deviceManagementRestrictionsEnabled() && m_isForMainResourceNavigationForAnyFrame) {
    … …  
    645648            callOnMainRunLoop([this, protectedThis = WTFMove(protectedThis), isBlocked] {
    646649                if (isBlocked) {
    647                     scheduleFailure(RestrictedURLFailure);
     650                    scheduleFailure(FailureType::RestrictedURL);
    648651                    return;
    649652                }

  • TabularUnified trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp

    r287021 r287039  
    152152
    153153    if (!m_currentRequest.url().protocolIsInHTTPFamily()) {
    154         scheduleFailure(InvalidURLFailure);
     154        scheduleFailure(FailureType::InvalidURL);
    155155        return;
    156156    }
    … …  
    160160    m_soupMessage = m_currentRequest.createSoupMessage(m_session->blobRegistry());
    161161    if (!m_soupMessage) {
    162         scheduleFailure(InvalidURLFailure);
     162        scheduleFailure(FailureType::InvalidURL);
    163163        return;
    164164    }

  • TabularUnified trunk/Source/WebKit/Shared/WebErrors.cpp

    r264021 r287039  
    6363}
    6464
     65ResourceError ftpDisabledError(const ResourceRequest& request)
     66{
     67    return ResourceError(errorDomainWebKitInternal, 0, request.url(), "FTP URLs are disabled"_s, ResourceError::Type::AccessControl);
     68}
     69
    6570ResourceError failedCustomProtocolSyncLoad(const ResourceRequest& request)
    6671{

  • TabularUnified trunk/Source/WebKit/Shared/WebErrors.h

    r264021 r287039  
    4242WebCore::ResourceError wasBlockedByRestrictionsError(const WebCore::ResourceRequest&);
    4343WebCore::ResourceError interruptedForPolicyChangeError(const WebCore::ResourceRequest&);
     44WebCore::ResourceError ftpDisabledError(const WebCore::ResourceRequest&);
    4445WebCore::ResourceError failedCustomProtocolSyncLoad(const WebCore::ResourceRequest&);
    4546#if ENABLE(CONTENT_FILTERING)

  • TabularUnified trunk/Tools/ChangeLog

    r287030 r287039  
     12021-12-14  Alex Christensen  <achristensen@webkit.org>
     2
     3        Move FTP disabling from NetworkLoad::start to NetworkDataTask::NetworkDataTask
     4        https://bugs.webkit.org/show_bug.cgi?id=234280
     5
     6        Reviewed by Brady Eidson.
     7
     8        * TestWebKitAPI/Tests/WebKitCocoa/Download.mm:
     9
    1102021-12-11  Dean Jackson  <dino@apple.com>
    211

  • TabularUnified trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm

    r285547 r287039  
    21552155        DownloadCallback::DidFailWithError,
    21562156    });
     2157
     2158    failed = false;
     2159    [webView startDownloadUsingRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"ftp:///"]] completionHandler:^(WKDownload *download) {
     2160        download.delegate = delegate.get();
     2161        delegate.get().didFailWithError = ^(WKDownload *download, NSError *error, NSData *resumeData) {
     2162            EXPECT_WK_STREQ(error.domain, WebKitErrorDomain);
     2163            EXPECT_EQ(error.code, 101);
     2164            failed = true;
     2165        };
     2166    }];
     2167    Util::run(&failed);
     2168
     2169    checkCallbackRecord(delegate.get(), {
     2170        DownloadCallback::DidFailWithError,
     2171    });
    21572172}
    21582173
Note: See TracChangeset for help on using the changeset viewer.