⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 287628 in webkit


Ignore:
Timestamp:
Jan 5, 2022, 10:21:08 AM (5 years ago)
Author:
Russell Epstein
Message:

Cherry-pick r287039. rdar://problem/85015428

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:

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@287039 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-612-branch
Files:
15 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-612-branch/Source/WebKit/ChangeLog

    r287619 r287628  
     12022-01-05  Russell Epstein  <repstein@apple.com>
     2
     3        Cherry-pick r287039. rdar://problem/85015428
     4
     5    Move FTP disabling from NetworkLoad::start to NetworkDataTask::NetworkDataTask
     6    https://bugs.webkit.org/show_bug.cgi?id=234280
     7    Source/WebKit:
     8   
     9    <rdar://85015428>
     10   
     11    Reviewed by Brady Eidson.
     12   
     13    A NetworkLoad synchronously failing has caused several issues.
     14    This issue is when a WKDownload is prevented in this way, we don't have a WKDownload in the UI process yet,
     15    and we haven't assigned it an identifier in the network process yet either, so we dereference null and send
     16    unreceived messages in many places.  To fix this issue, I move the FTP check to the NetworkDataTask constructor
     17    like we do with blocked port checks and invalid URL checks.  I also found that there is a race condition with
     18    CFNetwork also failing to load an empty FTP URL, so we need to prevent us from asking CFNetwork to start a load
     19    for which we have scheduled a failure.
     20   
     21    Covered by an API test which used to hit all the horrible failures and now passes reliably.
     22   
     23    * NetworkProcess/NetworkCORSPreflightChecker.cpp:
     24    (WebKit::NetworkCORSPreflightChecker::wasBlockedByDisabledFTP):
     25    * NetworkProcess/NetworkCORSPreflightChecker.h:
     26    * NetworkProcess/NetworkDataTask.cpp:
     27    (WebKit::NetworkDataTask::NetworkDataTask):
     28    (WebKit::NetworkDataTask::scheduleFailure):
     29    * NetworkProcess/NetworkDataTask.h:
     30    * NetworkProcess/NetworkLoad.cpp:
     31    (WebKit::NetworkLoad::start):
     32    (WebKit::NetworkLoad::wasBlockedByDisabledFTP):
     33    * NetworkProcess/NetworkLoad.h:
     34    * NetworkProcess/PingLoad.cpp:
     35    (WebKit::PingLoad::wasBlockedByDisabledFTP):
     36    * NetworkProcess/PingLoad.h:
     37    * NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
     38    (WebKit::NetworkDataTaskCocoa::resume):
     39    * Shared/WebErrors.cpp:
     40    (WebKit::ftpDisabledError):
     41    * Shared/WebErrors.h:
     42   
     43    Tools:
     44   
     45    Reviewed by Brady Eidson.
     46   
     47    * TestWebKitAPI/Tests/WebKitCocoa/Download.mm:
     48   
     49    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@287039 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     50
     51    2021-12-14  Alex Christensen  <achristensen@webkit.org>
     52
     53            Move FTP disabling from NetworkLoad::start to NetworkDataTask::NetworkDataTask
     54            https://bugs.webkit.org/show_bug.cgi?id=234280
     55            <rdar://85015428>
     56
     57            Reviewed by Brady Eidson.
     58
     59            A NetworkLoad synchronously failing has caused several issues.
     60            This issue is when a WKDownload is prevented in this way, we don't have a WKDownload in the UI process yet,
     61            and we haven't assigned it an identifier in the network process yet either, so we dereference null and send
     62            unreceived messages in many places.  To fix this issue, I move the FTP check to the NetworkDataTask constructor
     63            like we do with blocked port checks and invalid URL checks.  I also found that there is a race condition with
     64            CFNetwork also failing to load an empty FTP URL, so we need to prevent us from asking CFNetwork to start a load
     65            for which we have scheduled a failure.
     66
     67            Covered by an API test which used to hit all the horrible failures and now passes reliably.
     68
     69            * NetworkProcess/NetworkCORSPreflightChecker.cpp:
     70            (WebKit::NetworkCORSPreflightChecker::wasBlockedByDisabledFTP):
     71            * NetworkProcess/NetworkCORSPreflightChecker.h:
     72            * NetworkProcess/NetworkDataTask.cpp:
     73            (WebKit::NetworkDataTask::NetworkDataTask):
     74            (WebKit::NetworkDataTask::scheduleFailure):
     75            * NetworkProcess/NetworkDataTask.h:
     76            * NetworkProcess/NetworkLoad.cpp:
     77            (WebKit::NetworkLoad::start):
     78            (WebKit::NetworkLoad::wasBlockedByDisabledFTP):
     79            * NetworkProcess/NetworkLoad.h:
     80            * NetworkProcess/PingLoad.cpp:
     81            (WebKit::PingLoad::wasBlockedByDisabledFTP):
     82            * NetworkProcess/PingLoad.h:
     83            * NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
     84            (WebKit::NetworkDataTaskCocoa::resume):
     85            * Shared/WebErrors.cpp:
     86            (WebKit::ftpDisabledError):
     87            * Shared/WebErrors.h:
     88
    1892022-01-05  Russell Epstein  <repstein@apple.com>
    290

  • branches/safari-612-branch/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp

    r280425 r287628  
    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{

  • branches/safari-612-branch/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.h

    r258052 r287628  
    7878    void cannotShowURL() final;
    7979    void wasBlockedByRestrictions() final;
     80    void wasBlockedByDisabledFTP() final;
    8081
    8182    Parameters m_parameters;

  • branches/safari-612-branch/Source/WebKit/NetworkProcess/NetworkDataTask.cpp

    r284400 r287628  
    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 = makeWeakPtr(*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    });

  • branches/safari-612-branch/Source/WebKit/NetworkProcess/NetworkDataTask.h

    r284400 r287628  
    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

  • branches/safari-612-branch/Source/WebKit/NetworkProcess/NetworkLoad.cpp

    r284400 r287628  
    6161    if (!m_task)
    6262        return;
    63 
    64     if (!m_networkProcess->ftpEnabled() && m_parameters.request.url().protocolIsInFTPFamily()) {
    65         m_task->clearClient();
    66         m_task = nullptr;
    67         WebCore::NetworkLoadMetrics emptyMetrics;
    68         didCompleteWithError(ResourceError { errorDomainWebKitInternal, 0, url(), "FTP URLs are disabled"_s, ResourceError::Type::AccessControl }, emptyMetrics);
    69         return;
    70     }
    71 
    7263    m_task->resume();
    7364}
    … …  
    297288}
    298289
     290void NetworkLoad::wasBlockedByDisabledFTP()
     291{
     292    m_client.get().didFailLoading(ftpDisabledError(m_currentRequest));
     293}
     294
    299295void NetworkLoad::didNegotiateModernTLS(const URL& url)
    300296{

  • branches/safari-612-branch/Source/WebKit/NetworkProcess/NetworkLoad.h

    r284400 r287628  
    8787    void cannotShowURL() final;
    8888    void wasBlockedByRestrictions() final;
     89    void wasBlockedByDisabledFTP() final;
    8990    void didNegotiateModernTLS(const URL&) final;
    9091

  • branches/safari-612-branch/Source/WebKit/NetworkProcess/PingLoad.cpp

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

  • branches/safari-612-branch/Source/WebKit/NetworkProcess/PingLoad.h

    r255846 r287628  
    6161    void cannotShowURL() final;
    6262    void wasBlockedByRestrictions() final;
     63    void wasBlockedByDisabledFTP() final;
    6364    void timeoutTimerFired();
    6465

  • branches/safari-612-branch/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm

    r286236 r287628  
    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                }

  • branches/safari-612-branch/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp

    r279872 r287628  
    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    }

  • branches/safari-612-branch/Source/WebKit/Shared/WebErrors.cpp

    r264021 r287628  
    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{

  • branches/safari-612-branch/Source/WebKit/Shared/WebErrors.h

    r264021 r287628  
    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)

  • branches/safari-612-branch/Tools/ChangeLog

    r287618 r287628  
     12022-01-05  Russell Epstein  <repstein@apple.com>
     2
     3        Cherry-pick r287039. rdar://problem/85015428
     4
     5    Move FTP disabling from NetworkLoad::start to NetworkDataTask::NetworkDataTask
     6    https://bugs.webkit.org/show_bug.cgi?id=234280
     7    Source/WebKit:
     8   
     9    <rdar://85015428>
     10   
     11    Reviewed by Brady Eidson.
     12   
     13    A NetworkLoad synchronously failing has caused several issues.
     14    This issue is when a WKDownload is prevented in this way, we don't have a WKDownload in the UI process yet,
     15    and we haven't assigned it an identifier in the network process yet either, so we dereference null and send
     16    unreceived messages in many places.  To fix this issue, I move the FTP check to the NetworkDataTask constructor
     17    like we do with blocked port checks and invalid URL checks.  I also found that there is a race condition with
     18    CFNetwork also failing to load an empty FTP URL, so we need to prevent us from asking CFNetwork to start a load
     19    for which we have scheduled a failure.
     20   
     21    Covered by an API test which used to hit all the horrible failures and now passes reliably.
     22   
     23    * NetworkProcess/NetworkCORSPreflightChecker.cpp:
     24    (WebKit::NetworkCORSPreflightChecker::wasBlockedByDisabledFTP):
     25    * NetworkProcess/NetworkCORSPreflightChecker.h:
     26    * NetworkProcess/NetworkDataTask.cpp:
     27    (WebKit::NetworkDataTask::NetworkDataTask):
     28    (WebKit::NetworkDataTask::scheduleFailure):
     29    * NetworkProcess/NetworkDataTask.h:
     30    * NetworkProcess/NetworkLoad.cpp:
     31    (WebKit::NetworkLoad::start):
     32    (WebKit::NetworkLoad::wasBlockedByDisabledFTP):
     33    * NetworkProcess/NetworkLoad.h:
     34    * NetworkProcess/PingLoad.cpp:
     35    (WebKit::PingLoad::wasBlockedByDisabledFTP):
     36    * NetworkProcess/PingLoad.h:
     37    * NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
     38    (WebKit::NetworkDataTaskCocoa::resume):
     39    * Shared/WebErrors.cpp:
     40    (WebKit::ftpDisabledError):
     41    * Shared/WebErrors.h:
     42   
     43    Tools:
     44   
     45    Reviewed by Brady Eidson.
     46   
     47    * TestWebKitAPI/Tests/WebKitCocoa/Download.mm:
     48   
     49    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@287039 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     50
     51    2021-12-14  Alex Christensen  <achristensen@webkit.org>
     52
     53            Move FTP disabling from NetworkLoad::start to NetworkDataTask::NetworkDataTask
     54            https://bugs.webkit.org/show_bug.cgi?id=234280
     55
     56            Reviewed by Brady Eidson.
     57
     58            * TestWebKitAPI/Tests/WebKitCocoa/Download.mm:
     59
    1602022-01-05  Russell Epstein  <repstein@apple.com>
    261

  • branches/safari-612-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm

    r283789 r287628  
    21582158        DownloadCallback::DidFailWithError,
    21592159    });
     2160
     2161    failed = false;
     2162    [webView startDownloadUsingRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"ftp:///"]] completionHandler:^(WKDownload *download) {
     2163        download.delegate = delegate.get();
     2164        delegate.get().didFailWithError = ^(WKDownload *download, NSError *error, NSData *resumeData) {
     2165            EXPECT_WK_STREQ(error.domain, WebKitErrorDomain);
     2166            EXPECT_EQ(error.code, 101);
     2167            failed = true;
     2168        };
     2169    }];
     2170    Util::run(&failed);
     2171
     2172    checkCallbackRecord(delegate.get(), {
     2173        DownloadCallback::DidFailWithError,
     2174    });
    21602175}
    21612176
Note: See TracChangeset for help on using the changeset viewer.