- Revision
- 268904
- Author
- [email protected]
- Date
- 2020-10-22 21:25:13 -0700 (Thu, 22 Oct 2020)
Log Message
Null check platformStrategies when making blob read stream for an NSURLRequest
https://bugs.webkit.org/show_bug.cgi?id=218112
<rdar://problem/70507102>
Patch by Alex Christensen <[email protected]> on 2020-10-22
Reviewed by Wenson Hsieh.
Source/WebCore:
r266187 made it possible to create a DataTransfer without a user event, which allows you to make a FileList,
which allows you to submit a multipart form without a user event that calls decidePolicyForNavigationAction
with a request that has a form data body that would make a blob upload stream. This causes us to use
platformStrategies in the UI process, which dereferences null. Since the blob only really exists in the network
process, just return a nil HTTPBody in such an exotic case instead of crashing.
Covered by an API test that would hit this crash. Web platform tests were insufficient because WebKitTestRunner does not
access WKNavigationAction.request, but Safari does.
* platform/PlatformStrategies.cpp:
(WebCore::hasPlatformStrategies):
* platform/PlatformStrategies.h:
* platform/network/FormData.cpp:
(WebCore::FormData::containsBlobElement const):
(WebCore::FormData::resolveBlobReferences):
* platform/network/FormData.h:
* platform/network/cf/FormDataStreamCFNet.cpp:
(WebCore::createHTTPBodyCFReadStream):
Tools:
* TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm:
(TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (268903 => 268904)
--- trunk/Source/WebCore/ChangeLog 2020-10-23 03:49:41 UTC (rev 268903)
+++ trunk/Source/WebCore/ChangeLog 2020-10-23 04:25:13 UTC (rev 268904)
@@ -1,3 +1,30 @@
+2020-10-22 Alex Christensen <[email protected]>
+
+ Null check platformStrategies when making blob read stream for an NSURLRequest
+ https://bugs.webkit.org/show_bug.cgi?id=218112
+ <rdar://problem/70507102>
+
+ Reviewed by Wenson Hsieh.
+
+ r266187 made it possible to create a DataTransfer without a user event, which allows you to make a FileList,
+ which allows you to submit a multipart form without a user event that calls decidePolicyForNavigationAction
+ with a request that has a form data body that would make a blob upload stream. This causes us to use
+ platformStrategies in the UI process, which dereferences null. Since the blob only really exists in the network
+ process, just return a nil HTTPBody in such an exotic case instead of crashing.
+
+ Covered by an API test that would hit this crash. Web platform tests were insufficient because WebKitTestRunner does not
+ access WKNavigationAction.request, but Safari does.
+
+ * platform/PlatformStrategies.cpp:
+ (WebCore::hasPlatformStrategies):
+ * platform/PlatformStrategies.h:
+ * platform/network/FormData.cpp:
+ (WebCore::FormData::containsBlobElement const):
+ (WebCore::FormData::resolveBlobReferences):
+ * platform/network/FormData.h:
+ * platform/network/cf/FormDataStreamCFNet.cpp:
+ (WebCore::createHTTPBodyCFReadStream):
+
2020-10-22 Simon Fraser <[email protected]>
REGRESSION(r268476): [ macOS ] tiled-drawing/scrolling/non-fast-region/handlers-in-iframes.html is a flaky failure
Modified: trunk/Source/WebCore/platform/PlatformStrategies.cpp (268903 => 268904)
--- trunk/Source/WebCore/platform/PlatformStrategies.cpp 2020-10-23 03:49:41 UTC (rev 268903)
+++ trunk/Source/WebCore/platform/PlatformStrategies.cpp 2020-10-23 04:25:13 UTC (rev 268904)
@@ -30,6 +30,11 @@
static PlatformStrategies* s_platformStrategies;
+bool hasPlatformStrategies()
+{
+ return !!s_platformStrategies;
+}
+
PlatformStrategies* platformStrategies()
{
ASSERT(s_platformStrategies);
Modified: trunk/Source/WebCore/platform/PlatformStrategies.h (268903 => 268904)
--- trunk/Source/WebCore/platform/PlatformStrategies.h 2020-10-23 03:49:41 UTC (rev 268903)
+++ trunk/Source/WebCore/platform/PlatformStrategies.h 2020-10-23 04:25:13 UTC (rev 268904)
@@ -81,6 +81,7 @@
BlobRegistry* m_blobRegistry { };
};
+bool hasPlatformStrategies();
WEBCORE_EXPORT PlatformStrategies* platformStrategies();
WEBCORE_EXPORT void setPlatformStrategies(PlatformStrategies*);
Modified: trunk/Source/WebCore/platform/network/FormData.cpp (268903 => 268904)
--- trunk/Source/WebCore/platform/network/FormData.cpp 2020-10-23 03:49:41 UTC (rev 268903)
+++ trunk/Source/WebCore/platform/network/FormData.cpp 2020-10-23 04:25:13 UTC (rev 268904)
@@ -316,18 +316,19 @@
}
}
-Ref<FormData> FormData::resolveBlobReferences(BlobRegistryImpl* blobRegistryImpl)
+bool FormData::containsBlobElement() const
{
- // First check if any blobs needs to be resolved, or we can take the fast path.
- bool hasBlob = false;
for (auto& element : m_elements) {
- if (WTF::holds_alternative<FormDataElement::EncodedBlobData>(element.data)) {
- hasBlob = true;
- break;
- }
+ if (WTF::holds_alternative<FormDataElement::EncodedBlobData>(element.data))
+ return true;
}
+ return false;
+}
- if (!hasBlob)
+Ref<FormData> FormData::resolveBlobReferences(BlobRegistryImpl* blobRegistryImpl)
+{
+ // First check if any blobs needs to be resolved, or we can take the fast path.
+ if (!containsBlobElement())
return *this;
// Create a copy to append the result into.
Modified: trunk/Source/WebCore/platform/network/FormData.h (268903 => 268904)
--- trunk/Source/WebCore/platform/network/FormData.h 2020-10-23 03:49:41 UTC (rev 268903)
+++ trunk/Source/WebCore/platform/network/FormData.h 2020-10-23 04:25:13 UTC (rev 268904)
@@ -221,6 +221,7 @@
// Resolve all blob references so we only have file and data.
// If the FormData has no blob references to resolve, this is returned.
WEBCORE_EXPORT Ref<FormData> resolveBlobReferences(BlobRegistryImpl* = nullptr);
+ bool containsBlobElement() const;
WEBCORE_EXPORT FormDataForUpload prepareForUpload();
Modified: trunk/Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp (268903 => 268904)
--- trunk/Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp 2020-10-23 03:49:41 UTC (rev 268903)
+++ trunk/Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp 2020-10-23 04:25:13 UTC (rev 268904)
@@ -31,6 +31,7 @@
#include "BlobRegistryImpl.h"
#include "FormData.h"
+#include "PlatformStrategies.h"
#include <sys/stat.h>
#include <sys/types.h>
#include <wtf/Assertions.h>
@@ -370,6 +371,9 @@
RetainPtr<CFReadStreamRef> createHTTPBodyCFReadStream(FormData& formData)
{
+ if (!hasPlatformStrategies() && formData.containsBlobElement())
+ return nullptr;
+
auto resolvedFormData = formData.resolveBlobReferences();
auto dataForUpload = resolvedFormData->prepareForUpload();
Modified: trunk/Tools/ChangeLog (268903 => 268904)
--- trunk/Tools/ChangeLog 2020-10-23 03:49:41 UTC (rev 268903)
+++ trunk/Tools/ChangeLog 2020-10-23 04:25:13 UTC (rev 268904)
@@ -1,3 +1,14 @@
+2020-10-22 Alex Christensen <[email protected]>
+
+ Null check platformStrategies when making blob read stream for an NSURLRequest
+ https://bugs.webkit.org/show_bug.cgi?id=218112
+ <rdar://problem/70507102>
+
+ Reviewed by Wenson Hsieh.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm:
+ (TEST):
+
2020-10-22 Jonathan Bedard <[email protected]>
[webkitpy] Use webkitcorepy's autoinstaller for chrome and gecko drivers
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm (268903 => 268904)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm 2020-10-23 03:49:41 UTC (rev 268903)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/NavigationAction.mm 2020-10-23 04:25:13 UTC (rev 268904)
@@ -25,8 +25,10 @@
#import "config.h"
+#import "HTTPServer.h"
#import "PlatformUtilities.h"
#import "Test.h"
+#import "TestNavigationDelegate.h"
#import "TestWKWebView.h"
#import <WebKit/WKNavigationActionPrivate.h>
#import <wtf/RetainPtr.h>
@@ -175,3 +177,41 @@
EXPECT_NOT_NULL(navigationDelegate.get().navigationAction);
EXPECT_FALSE(navigationDelegate.get().navigationAction._shouldPerformDownload);
}
+
+TEST(WKNavigationAction, BlobRequestBody)
+{
+ NSString *html = @""
+ "<script>"
+ "function bodyLoaded() {"
+ "const form = Object.assign(document.createElement('form'), {"
+ "action: '/formAction', method: 'POST', enctype: 'multipart/form-data',"
+ "});"
+ "document.body.append(form);"
+ "const fileInput = Object.assign(document.createElement('input'), {"
+ "type: 'file', name: 'file',"
+ "});"
+ "form.append(fileInput);"
+ "const dataTransfer = new DataTransfer;"
+ "dataTransfer.items.add(new File(['a'], 'filename'));"
+ "fileInput.files = dataTransfer.files;"
+ "form.submit();"
+ "}"
+ "</script>"
+ "<body _onload_='bodyLoaded()'>";
+ auto delegate = [[TestNavigationDelegate new] autorelease];
+ auto webView = [[WKWebView new] autorelease];
+ webView.navigationDelegate = delegate;
+ __block bool done = false;
+ delegate.decidePolicyForNavigationAction = ^(WKNavigationAction *action, void (^completionHandler)(WKNavigationActionPolicy)) {
+ EXPECT_NULL(action.request.HTTPBody);
+ if ([action.request.URL.absoluteString isEqualToString:@"about:blank"])
+ completionHandler(WKNavigationActionPolicyAllow);
+ else {
+ EXPECT_WK_STREQ(action.request.URL.absoluteString, "/formAction");
+ completionHandler(WKNavigationActionPolicyCancel);
+ done = true;
+ }
+ };
+ [webView loadHTMLString:html baseURL:nil];
+ TestWebKitAPI::Util::run(&done);
+}
