Diff
Modified: trunk/Source/WebKit/ChangeLog (287038 => 287039)
--- trunk/Source/WebKit/ChangeLog 2021-12-14 19:59:26 UTC (rev 287038)
+++ trunk/Source/WebKit/ChangeLog 2021-12-14 20:06:12 UTC (rev 287039)
@@ -1,3 +1,41 @@
+2021-12-14 Alex Christensen <achristen...@webkit.org>
+
+ Move FTP disabling from NetworkLoad::start to NetworkDataTask::NetworkDataTask
+ https://bugs.webkit.org/show_bug.cgi?id=234280
+ <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:
+
2021-12-14 Tim Horton <timothy_hor...@apple.com>
Fix the build
Modified: trunk/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp (287038 => 287039)
--- trunk/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp 2021-12-14 19:59:26 UTC (rev 287038)
+++ trunk/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp 2021-12-14 20:06:12 UTC (rev 287039)
@@ -32,6 +32,7 @@
#include "NetworkLoadParameters.h"
#include "NetworkProcess.h"
#include "NetworkResourceLoader.h"
+#include "WebErrors.h"
#include <WebCore/CrossOriginAccessControl.h>
#include <WebCore/SecurityOrigin.h>
@@ -172,6 +173,11 @@
m_completionCallback(ResourceError { errorDomainWebKitInternal, 0, m_parameters.originalRequest.url(), "Preflight response was blocked"_s, ResourceError::Type::AccessControl });
}
+void NetworkCORSPreflightChecker::wasBlockedByDisabledFTP()
+{
+ m_completionCallback(ftpDisabledError(m_parameters.originalRequest));
+}
+
NetworkTransactionInformation NetworkCORSPreflightChecker::takeInformation()
{
ASSERT(m_shouldCaptureExtraNetworkLoadMetrics);
Modified: trunk/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.h (287038 => 287039)
--- trunk/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.h 2021-12-14 19:59:26 UTC (rev 287038)
+++ trunk/Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.h 2021-12-14 20:06:12 UTC (rev 287039)
@@ -77,6 +77,7 @@
void wasBlocked() final;
void cannotShowURL() final;
void wasBlockedByRestrictions() final;
+ void wasBlockedByDisabledFTP() final;
Parameters m_parameters;
Ref<NetworkProcess> m_networkProcess;
Modified: trunk/Source/WebKit/NetworkProcess/NetworkDataTask.cpp (287038 => 287039)
--- trunk/Source/WebKit/NetworkProcess/NetworkDataTask.cpp 2021-12-14 19:59:26 UTC (rev 287038)
+++ trunk/Source/WebKit/NetworkProcess/NetworkDataTask.cpp 2021-12-14 20:06:12 UTC (rev 287039)
@@ -76,14 +76,20 @@
ASSERT(RunLoop::isMain());
if (!requestWithCredentials.url().isValid()) {
- scheduleFailure(InvalidURLFailure);
+ scheduleFailure(FailureType::InvalidURL);
return;
}
if (!portAllowed(requestWithCredentials.url())) {
- scheduleFailure(BlockedFailure);
+ scheduleFailure(FailureType::Blocked);
return;
}
+
+ if (!session.networkProcess().ftpEnabled()
+ && requestWithCredentials.url().protocolIsInFTPFamily()) {
+ scheduleFailure(FailureType::FTPDisabled);
+ return;
+ }
}
NetworkDataTask::~NetworkDataTask()
@@ -94,20 +100,23 @@
void NetworkDataTask::scheduleFailure(FailureType type)
{
+ m_failureScheduled = true;
RunLoop::main().dispatch([this, weakThis = WeakPtr { *this }, type] {
if (!weakThis || !m_client)
return;
switch (type) {
- case BlockedFailure:
+ case FailureType::Blocked:
m_client->wasBlocked();
return;
- case InvalidURLFailure:
+ case FailureType::InvalidURL:
m_client->cannotShowURL();
return;
- case RestrictedURLFailure:
+ case FailureType::RestrictedURL:
m_client->wasBlockedByRestrictions();
return;
+ case FailureType::FTPDisabled:
+ m_client->wasBlockedByDisabledFTP();
}
});
}
Modified: trunk/Source/WebKit/NetworkProcess/NetworkDataTask.h (287038 => 287039)
--- trunk/Source/WebKit/NetworkProcess/NetworkDataTask.h 2021-12-14 19:59:26 UTC (rev 287038)
+++ trunk/Source/WebKit/NetworkProcess/NetworkDataTask.h 2021-12-14 20:06:12 UTC (rev 287039)
@@ -69,6 +69,7 @@
virtual void wasBlocked() = 0;
virtual void cannotShowURL() = 0;
virtual void wasBlockedByRestrictions() = 0;
+ virtual void wasBlockedByDisabledFTP() = 0;
virtual bool shouldCaptureExtraNetworkLoadMetrics() const { return false; }
@@ -145,10 +146,11 @@
protected:
NetworkDataTask(NetworkSession&, NetworkDataTaskClient&, const WebCore::ResourceRequest&, WebCore::StoredCredentialsPolicy, bool shouldClearReferrerOnHTTPSToHTTPRedirect, bool dataTaskIsForMainFrameNavigation);
- enum FailureType {
- BlockedFailure,
- InvalidURLFailure,
- RestrictedURLFailure
+ enum class FailureType : uint8_t {
+ Blocked,
+ InvalidURL,
+ RestrictedURL,
+ FTPDisabled
};
void scheduleFailure(FailureType);
@@ -172,6 +174,7 @@
bool m_shouldClearReferrerOnHTTPSToHTTPRedirect { true };
String m_suggestedFilename;
bool m_dataTaskIsForMainFrameNavigation { false };
+ bool m_failureScheduled { false };
};
} // namespace WebKit
Modified: trunk/Source/WebKit/NetworkProcess/NetworkLoad.cpp (287038 => 287039)
--- trunk/Source/WebKit/NetworkProcess/NetworkLoad.cpp 2021-12-14 19:59:26 UTC (rev 287038)
+++ trunk/Source/WebKit/NetworkProcess/NetworkLoad.cpp 2021-12-14 20:06:12 UTC (rev 287039)
@@ -67,15 +67,6 @@
{
if (!m_task)
return;
-
- if (!m_networkProcess->ftpEnabled() && m_parameters.request.url().protocolIsInFTPFamily()) {
- m_task->clearClient();
- m_task = nullptr;
- WebCore::NetworkLoadMetrics emptyMetrics;
- didCompleteWithError(ResourceError { errorDomainWebKitInternal, 0, url(), "FTP URLs are disabled"_s, ResourceError::Type::AccessControl }, emptyMetrics);
- return;
- }
-
m_task->resume();
}
@@ -303,6 +294,11 @@
m_client.get().didFailLoading(wasBlockedByRestrictionsError(m_currentRequest));
}
+void NetworkLoad::wasBlockedByDisabledFTP()
+{
+ m_client.get().didFailLoading(ftpDisabledError(m_currentRequest));
+}
+
void NetworkLoad::didNegotiateModernTLS(const URL& url)
{
if (m_parameters.webPageProxyID)
Modified: trunk/Source/WebKit/NetworkProcess/NetworkLoad.h (287038 => 287039)
--- trunk/Source/WebKit/NetworkProcess/NetworkLoad.h 2021-12-14 19:59:26 UTC (rev 287038)
+++ trunk/Source/WebKit/NetworkProcess/NetworkLoad.h 2021-12-14 20:06:12 UTC (rev 287039)
@@ -87,6 +87,7 @@
void wasBlocked() final;
void cannotShowURL() final;
void wasBlockedByRestrictions() final;
+ void wasBlockedByDisabledFTP() final;
void didNegotiateModernTLS(const URL&) final;
void notifyDidReceiveResponse(WebCore::ResourceResponse&&, NegotiatedLegacyTLS, ResponseCompletionHandler&&);
Modified: trunk/Source/WebKit/NetworkProcess/PingLoad.cpp (287038 => 287039)
--- trunk/Source/WebKit/NetworkProcess/PingLoad.cpp 2021-12-14 19:59:26 UTC (rev 287038)
+++ trunk/Source/WebKit/NetworkProcess/PingLoad.cpp 2021-12-14 20:06:12 UTC (rev 287039)
@@ -210,6 +210,12 @@
didFinish(wasBlockedByRestrictionsError(ResourceRequest { currentURL() }));
}
+void PingLoad::wasBlockedByDisabledFTP()
+{
+ PING_RELEASE_LOG("wasBlockedByDisabledFTP");
+ didFinish(ftpDisabledError(ResourceRequest(currentURL())));
+}
+
void PingLoad::timeoutTimerFired()
{
PING_RELEASE_LOG("timeoutTimerFired");
Modified: trunk/Source/WebKit/NetworkProcess/PingLoad.h (287038 => 287039)
--- trunk/Source/WebKit/NetworkProcess/PingLoad.h 2021-12-14 19:59:26 UTC (rev 287038)
+++ trunk/Source/WebKit/NetworkProcess/PingLoad.h 2021-12-14 20:06:12 UTC (rev 287039)
@@ -60,6 +60,7 @@
void wasBlocked() final;
void cannotShowURL() final;
void wasBlockedByRestrictions() final;
+ void wasBlockedByDisabledFTP() final;
void timeoutTimerFired();
void loadRequest(NetworkProcess&, WebCore::ResourceRequest&&);
Modified: trunk/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm (287038 => 287039)
--- trunk/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm 2021-12-14 19:59:26 UTC (rev 287038)
+++ trunk/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm 2021-12-14 20:06:12 UTC (rev 287039)
@@ -639,12 +639,15 @@
{
WTFEmitSignpost(m_task.get(), "DataTask", "resume");
+ if (m_failureScheduled)
+ return;
+
auto& cocoaSession = static_cast<NetworkSessionCocoa&>(*m_session);
if (cocoaSession.deviceManagementRestrictionsEnabled() && m_isForMainResourceNavigationForAnyFrame) {
auto didDetermineDeviceRestrictionPolicyForURL = makeBlockPtr([this, protectedThis = Ref { *this }](BOOL isBlocked) mutable {
callOnMainRunLoop([this, protectedThis = WTFMove(protectedThis), isBlocked] {
if (isBlocked) {
- scheduleFailure(RestrictedURLFailure);
+ scheduleFailure(FailureType::RestrictedURL);
return;
}
Modified: trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp (287038 => 287039)
--- trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp 2021-12-14 19:59:26 UTC (rev 287038)
+++ trunk/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp 2021-12-14 20:06:12 UTC (rev 287039)
@@ -151,7 +151,7 @@
return;
if (!m_currentRequest.url().protocolIsInHTTPFamily()) {
- scheduleFailure(InvalidURLFailure);
+ scheduleFailure(FailureType::InvalidURL);
return;
}
@@ -159,7 +159,7 @@
m_soupMessage = m_currentRequest.createSoupMessage(m_session->blobRegistry());
if (!m_soupMessage) {
- scheduleFailure(InvalidURLFailure);
+ scheduleFailure(FailureType::InvalidURL);
return;
}
Modified: trunk/Source/WebKit/Shared/WebErrors.cpp (287038 => 287039)
--- trunk/Source/WebKit/Shared/WebErrors.cpp 2021-12-14 19:59:26 UTC (rev 287038)
+++ trunk/Source/WebKit/Shared/WebErrors.cpp 2021-12-14 20:06:12 UTC (rev 287039)
@@ -62,6 +62,11 @@
return ResourceError(API::Error::webKitPolicyErrorDomain(), API::Error::Policy::FrameLoadInterruptedByPolicyChange, request.url(), WEB_UI_STRING("Frame load interrupted", "WebKitErrorFrameLoadInterruptedByPolicyChange description"));
}
+ResourceError ftpDisabledError(const ResourceRequest& request)
+{
+ return ResourceError(errorDomainWebKitInternal, 0, request.url(), "FTP URLs are disabled"_s, ResourceError::Type::AccessControl);
+}
+
ResourceError failedCustomProtocolSyncLoad(const ResourceRequest& request)
{
return ResourceError(errorDomainWebKitInternal, 0, request.url(), WEB_UI_STRING("Error handling synchronous load with custom protocol", "Custom protocol synchronous load failure description"));
Modified: trunk/Source/WebKit/Shared/WebErrors.h (287038 => 287039)
--- trunk/Source/WebKit/Shared/WebErrors.h 2021-12-14 19:59:26 UTC (rev 287038)
+++ trunk/Source/WebKit/Shared/WebErrors.h 2021-12-14 20:06:12 UTC (rev 287039)
@@ -41,6 +41,7 @@
WebCore::ResourceError cannotShowURLError(const WebCore::ResourceRequest&);
WebCore::ResourceError wasBlockedByRestrictionsError(const WebCore::ResourceRequest&);
WebCore::ResourceError interruptedForPolicyChangeError(const WebCore::ResourceRequest&);
+WebCore::ResourceError ftpDisabledError(const WebCore::ResourceRequest&);
WebCore::ResourceError failedCustomProtocolSyncLoad(const WebCore::ResourceRequest&);
#if ENABLE(CONTENT_FILTERING)
WebCore::ResourceError blockedByContentFilterError(const WebCore::ResourceRequest&);
Modified: trunk/Tools/ChangeLog (287038 => 287039)
--- trunk/Tools/ChangeLog 2021-12-14 19:59:26 UTC (rev 287038)
+++ trunk/Tools/ChangeLog 2021-12-14 20:06:12 UTC (rev 287039)
@@ -1,3 +1,12 @@
+2021-12-14 Alex Christensen <achristen...@webkit.org>
+
+ Move FTP disabling from NetworkLoad::start to NetworkDataTask::NetworkDataTask
+ https://bugs.webkit.org/show_bug.cgi?id=234280
+
+ Reviewed by Brady Eidson.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/Download.mm:
+
2021-12-11 Dean Jackson <d...@apple.com>
Allow override of system's preferred color scheme
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm (287038 => 287039)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm 2021-12-14 19:59:26 UTC (rev 287038)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/Download.mm 2021-12-14 20:06:12 UTC (rev 287039)
@@ -2154,6 +2154,21 @@
checkCallbackRecord(delegate.get(), {
DownloadCallback::DidFailWithError,
});
+
+ failed = false;
+ [webView startDownloadUsingRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"ftp:///"]] completionHandler:^(WKDownload *download) {
+ download.delegate = delegate.get();
+ delegate.get().didFailWithError = ^(WKDownload *download, NSError *error, NSData *resumeData) {
+ EXPECT_WK_STREQ(error.domain, WebKitErrorDomain);
+ EXPECT_EQ(error.code, 101);
+ failed = true;
+ };
+ }];
+ Util::run(&failed);
+
+ checkCallbackRecord(delegate.get(), {
+ DownloadCallback::DidFailWithError,
+ });
}
TEST(WKDownload, DownloadRequest404)
