XMLHTTPRequest POSTs blob data to a custom WKURLSchemeHandler protocol crash
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 Jan 2020 22:57:25 +0000 (22:57 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 Jan 2020 22:57:25 +0000 (22:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=205685

Reviewed by Alex Christensen.

Source/WebCore:

There is no blob registry in the UIProcess.
This should not matter since we do not yet support blobs in custom scheme handlers.
But we are calling the blob registry when creating a request body, which does not work in UIProcess.
Instead, pass a lambda that will be called in case of blobs.
Covered by API test.

* platform/network/FormData.cpp:
(WebCore::FormDataElement::lengthInBytes const):
(WebCore::FormData::resolveBlobReferences):
* platform/network/FormData.h:
* platform/network/cf/FormDataStreamCFNet.cpp:
(WebCore::createHTTPBodyCFReadStream):

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@254089 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/network/FormData.cpp
Source/WebCore/platform/network/FormData.h
Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm

index 52f2029..9cb800f 100644 (file)
@@ -1,3 +1,23 @@
+2020-01-06  youenn fablet  <youenn@apple.com>
+
+        XMLHTTPRequest POSTs blob data to a custom WKURLSchemeHandler protocol crash
+        https://bugs.webkit.org/show_bug.cgi?id=205685
+
+        Reviewed by Alex Christensen.
+
+        There is no blob registry in the UIProcess.
+        This should not matter since we do not yet support blobs in custom scheme handlers.
+        But we are calling the blob registry when creating a request body, which does not work in UIProcess.
+        Instead, pass a lambda that will be called in case of blobs.
+        Covered by API test.
+
+        * platform/network/FormData.cpp:
+        (WebCore::FormDataElement::lengthInBytes const):
+        (WebCore::FormData::resolveBlobReferences):
+        * platform/network/FormData.h:
+        * platform/network/cf/FormDataStreamCFNet.cpp:
+        (WebCore::createHTTPBodyCFReadStream):
+
 2020-01-06  Mark Lam  <mark.lam@apple.com>
 
         Convert ASSERT_DISABLED to ASSERT_ENABLED, and fix some tests of NDEBUG that should actually test for ASSERT_ENABLED.
index 0f8b9d0..53649a2 100644 (file)
@@ -123,9 +123,9 @@ Ref<FormData> FormData::isolatedCopy() const
     return formData;
 }
 
-static inline uint64_t computeLengthInBytes(const FormDataElement& element, const Function<uint64_t(const URL&)>& blobSize)
+uint64_t FormDataElement::lengthInBytes(const Function<uint64_t(const URL&)>& blobSize) const
 {
-    return switchOn(element.data,
+    return switchOn(data,
         [] (const Vector<char>& bytes) {
             return static_cast<uint64_t>(bytes.size());
         }, [] (const FormDataElement::EncodedFileData& fileData) {
@@ -141,16 +141,9 @@ static inline uint64_t computeLengthInBytes(const FormDataElement& element, cons
     );
 }
 
-uint64_t FormDataElement::lengthInBytes(BlobRegistryImpl* blobRegistry) const
-{
-    return computeLengthInBytes(*this, [&](auto& url) {
-        return blobRegistry ? blobRegistry->blobSize(url) : 0;
-    });
-}
-
 uint64_t FormDataElement::lengthInBytes() const
 {
-    return computeLengthInBytes(*this, [](auto& url) {
+    return lengthInBytes([](auto& url) {
         return blobRegistry().blobSize(url);
     });
 }
@@ -323,7 +316,7 @@ static void appendBlobResolved(BlobRegistryImpl* blobRegistry, FormData& formDat
     }
 }
 
-Ref<FormData> FormData::resolveBlobReferences(BlobRegistryImpl* blobRegistry)
+Ref<FormData> FormData::resolveBlobReferences(BlobRegistryImpl* blobRegistryImpl)
 {
     // First check if any blobs needs to be resolved, or we can take the fast path.
     bool hasBlob = false;
@@ -349,7 +342,7 @@ Ref<FormData> FormData::resolveBlobReferences(BlobRegistryImpl* blobRegistry)
             }, [&] (const FormDataElement::EncodedFileData& fileData) {
                 newFormData->appendFileRange(fileData.filename, fileData.fileStart, fileData.fileLength, fileData.expectedFileModificationTime);
             }, [&] (const FormDataElement::EncodedBlobData& blobData) {
-                appendBlobResolved(blobRegistry, newFormData.get(), blobData.url);
+                appendBlobResolved(blobRegistryImpl ? blobRegistryImpl : blobRegistry().blobRegistryImpl(), newFormData.get(), blobData.url);
             }
         );
     }
index c54dd1a..8c2d953 100644 (file)
@@ -50,7 +50,7 @@ struct FormDataElement {
     explicit FormDataElement(const URL& blobURL)
         : data(EncodedBlobData { blobURL }) { }
 
-    uint64_t lengthInBytes(BlobRegistryImpl*) const;
+    uint64_t lengthInBytes(const Function<uint64_t(const URL&)>&) const;
     uint64_t lengthInBytes() const;
 
     FormDataElement isolatedCopy() const;
@@ -220,7 +220,7 @@ public:
 
     // 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*);
+    WEBCORE_EXPORT Ref<FormData> resolveBlobReferences(BlobRegistryImpl* = nullptr);
 
     WEBCORE_EXPORT FormDataForUpload prepareForUpload();
 
index ca7baf9..456be34 100644 (file)
@@ -29,8 +29,7 @@
 #include "config.h"
 #include "FormDataStreamCFNet.h"
 
-#include "BlobData.h"
-#include "BlobRegistry.h"
+#include "BlobRegistryImpl.h"
 #include "FormData.h"
 #include <sys/stat.h>
 #include <sys/types.h>
@@ -371,14 +370,16 @@ static void formEventCallback(CFReadStreamRef stream, CFStreamEventType type, vo
 
 RetainPtr<CFReadStreamRef> createHTTPBodyCFReadStream(FormData& formData)
 {
-    auto resolvedFormData = formData.resolveBlobReferences(blobRegistry().blobRegistryImpl());
+    auto resolvedFormData = formData.resolveBlobReferences();
     auto dataForUpload = resolvedFormData->prepareForUpload();
 
     // Precompute the content length so CFNetwork doesn't use chunked mode.
     unsigned long long length = 0;
-    for (auto& element : dataForUpload.data().elements())
-        length += element.lengthInBytes(blobRegistry().blobRegistryImpl());
-
+    for (auto& element : dataForUpload.data().elements()) {
+        length += element.lengthInBytes([](auto& url) {
+            return blobRegistry().blobRegistryImpl()->blobSize(url);
+        });
+    }
     FormCreationContext* formContext = new FormCreationContext { WTFMove(dataForUpload), length };
     CFReadStreamCallBacksV1 callBacks = { 1, formCreate, formFinalize, nullptr, formOpen, nullptr, formRead, nullptr, formCanRead, formClose, formCopyProperty, nullptr, nullptr, formSchedule, formUnschedule };
     return adoptCF(CFReadStreamCreate(nullptr, static_cast<const void*>(&callBacks), formContext));
index 2cff346..af47273 100644 (file)
@@ -1,3 +1,12 @@
+2020-01-06  youenn fablet  <youenn@apple.com>
+
+        XMLHTTPRequest POSTs blob data to a custom WKURLSchemeHandler protocol crash
+        https://bugs.webkit.org/show_bug.cgi?id=205685
+
+        Reviewed by Alex Christensen.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKURLSchemeHandler-1.mm:
+
 2020-01-06  Alex Christensen  <achristensen@webkit.org>
 
         Remove WebsiteDataStore::setServiceWorkerRegistrationDirectory
index 307cf96..07cc32a 100644 (file)
@@ -623,6 +623,12 @@ window.onload = function()
     }
     {
         var xhr = new XMLHttpRequest();
+        xhr.open('POST', '/string-upload');
+        var upload = xhr.upload;
+        xhr.send('foo=bar2');
+    }
+    {
+        var xhr = new XMLHttpRequest();
         xhr.open('POST', '/document');
         xhr.send(window.document);
     }
@@ -665,6 +671,21 @@ TEST(URLSchemeHandler, XHRPost)
             reached = true;
             EXPECT_EQ(task.request.HTTPBody.length, 7u);
             EXPECT_STREQ(static_cast<const char*>(task.request.HTTPBody.bytes), "foo=bar");
+        } else if ([task.request.URL.absoluteString isEqualToString:@"xhrpost://example/string-upload"]) {
+            static bool reached;
+            EXPECT_FALSE(reached);
+            reached = true;
+            auto stream = task.request.HTTPBodyStream;
+            EXPECT_TRUE(!!stream);
+            [stream open];
+            EXPECT_TRUE(stream.hasBytesAvailable);
+            uint8_t buffer[9];
+            memset(buffer, 0, 9);
+            auto length = [stream read:buffer maxLength:9];
+            EXPECT_EQ(length, 8);
+            EXPECT_STREQ(reinterpret_cast<const char*>(buffer), "foo=bar2");
+            EXPECT_FALSE(stream.hasBytesAvailable);
+            [stream close];
         } else if ([task.request.URL.absoluteString isEqualToString:@"xhrpost://example/arraybuffer"]) {
             static bool reached;
             EXPECT_FALSE(reached);
@@ -704,7 +725,7 @@ TEST(URLSchemeHandler, XHRPost)
         [task didReceiveResponse:response.get()];
         [task didFinish];
         
-        if (++seenTasks == 4)
+        if (++seenTasks == 5)
             done = true;
     }];