Potential crash under NetworkDataTaskBlob::dispatchDidReceiveResponse()

https://bugs.webkit.org/show_bug.cgi?id=258951
rdar://111798349

Reviewed by Youenn Fablet.

In getSizeForNext(), we call seek() and then dispatchDidReceiveResponse().
After 261968@main, seek() could call fail internally and call didFail().
However, we could still call dispatchDidReceiveResponse() right after in
case of failure.

We now propagate the error state out of seek() and have the caller call
didFail() and then early return instead of calling dispatchDidReceiveResponse().

* Source/WebKit/NetworkProcess/NetworkDataTaskBlob.cpp:
(WebKit::NetworkDataTaskBlob::getSizeForNext):
(WebKit::NetworkDataTaskBlob::seek):
* Source/WebKit/NetworkProcess/NetworkDataTaskBlob.h:

Canonical link: https://commits.webkit.org/265848@main

Source/WebCore/platform/network/BlobResourceHandle.cpp

@@ -187,6 +187,7 @@ void BlobResourceHandle::start()
 void BlobResourceHandle::doStart()
 {
     ASSERT(isMainThread());
+    Ref protectedThis { *this };
 
     // Do not continue if the request is aborted or an error occurs.
     if (erroredOrAborted())
@@ -227,13 +228,14 @@ void BlobResourceHandle::getSizeForNext()
 
     // Do we finish validating and counting size for all items?
     if (m_sizeItemCount >= m_blobData->items().size()) {
-        seek();
+        if (auto error = seek()) {
+            notifyFail(*error);
+            return;
+        }
 
         // Start reading if in asynchronous mode.
-        if (m_async) {
-            Ref<BlobResourceHandle> protectedThis(*this);
+        if (m_async)
             notifyResponse();
-        }
         return;
     }
 
@@ -257,6 +259,7 @@ void BlobResourceHandle::getSizeForNext()
 void BlobResourceHandle::didGetSize(long long size)
 {
     ASSERT(isMainThread());
+    Ref protectedThis { *this };
 
     // Do not continue if the request is aborted or an error occurs.
     if (erroredOrAborted())
@@ -284,23 +287,21 @@ void BlobResourceHandle::didGetSize(long long size)
     getSizeForNext();
 }
 
-void BlobResourceHandle::seek()
+auto BlobResourceHandle::seek() -> std::optional<Error>
 {
     ASSERT(isMainThread());
 
     // Bail out if the range is not provided.
     if (!m_isRangeRequest)
-        return;
+        return std::nullopt;
 
     // Adjust m_rangeStart / m_rangeEnd
     if (m_rangeStart == kPositionNotSpecified) {
         m_rangeStart = m_totalSize - m_rangeEnd;
         m_rangeEnd = m_rangeStart + m_rangeEnd - 1;
     } else {
-        if (m_rangeStart >= m_totalSize) {
-            notifyFail(Error::RangeError);
-            return;
-        }
+        if (m_rangeStart >= m_totalSize)
+            return Error::RangeError;
         if (m_rangeEnd == kPositionNotSpecified || m_rangeEnd >= m_totalSize)
             m_rangeEnd = m_totalSize - 1;
     }
@@ -317,6 +318,7 @@ void BlobResourceHandle::seek()
     long long rangeSize = m_rangeEnd - m_rangeStart + 1;
     if (m_totalRemainingSize > rangeSize)
         m_totalRemainingSize = rangeSize;
+    return std::nullopt;
 }
 
 int BlobResourceHandle::readSync(uint8_t* buf, int length)

Source/WebCore/platform/network/BlobResourceHandle.h

@@ -78,7 +78,7 @@ class BlobResourceHandle final : public FileStreamClient, public ResourceHandle
 
     void doStart();
     void getSizeForNext();
-    void seek();
+    std::optional<Error> seek();
     void consumeData(const uint8_t* data, int bytesRead);
     void failed(Error);
 

Source/WebKit/NetworkProcess/NetworkDataTaskBlob.cpp

@@ -164,7 +164,10 @@ void NetworkDataTaskBlob::getSizeForNext()
 
     // Do we finish validating and counting size for all items?
     if (m_sizeItemCount >= m_blobData->items().size()) {
-        seek();
+        if (auto error = seek()) {
+            didFail(*error);
+            return;
+        }
         dispatchDidReceiveResponse();
         return;
     }
@@ -186,6 +189,7 @@ void NetworkDataTaskBlob::getSizeForNext()
 void NetworkDataTaskBlob::didGetSize(long long size)
 {
     ASSERT(RunLoop::isMain());
+    Ref protectedThis { *this };
 
     if (m_state == State::Canceling || m_state == State::Completed || (!m_client && !isDownload())) {
         clearStream();
@@ -214,23 +218,21 @@ void NetworkDataTaskBlob::didGetSize(long long size)
     getSizeForNext();
 }
 
-void NetworkDataTaskBlob::seek()
+auto NetworkDataTaskBlob::seek() -> std::optional<Error>
 {
     ASSERT(RunLoop::isMain());
 
     // Bail out if the range is not provided.
     if (!m_isRangeRequest)
-        return;
+        return std::nullopt;
 
     // Adjust m_rangeStart / m_rangeEnd
     if (m_rangeStart == kPositionNotSpecified) {
         m_rangeStart = m_totalSize - m_rangeEnd;
         m_rangeEnd = m_rangeStart + m_rangeEnd - 1;
     } else {
-        if (m_rangeStart >= m_totalSize) {
-            didFail(Error::RangeError);
-            return;
-        }
+        if (m_rangeStart >= m_totalSize)
+            return Error::RangeError;
         if (m_rangeEnd == kPositionNotSpecified || m_rangeEnd >= m_totalSize)
             m_rangeEnd = m_totalSize - 1;
     }
@@ -247,6 +249,7 @@ void NetworkDataTaskBlob::seek()
     long long rangeSize = m_rangeEnd - m_rangeStart + 1;
     if (m_totalRemainingSize > rangeSize)
         m_totalRemainingSize = rangeSize;
+    return std::nullopt;
 }
 
 void NetworkDataTaskBlob::dispatchDidReceiveResponse()

Source/WebKit/NetworkProcess/NetworkDataTaskBlob.h

@@ -84,7 +84,7 @@ class NetworkDataTaskBlob final : public NetworkDataTask, public WebCore::FileSt
     void clearStream();
     void getSizeForNext();
     void dispatchDidReceiveResponse();
-    void seek();
+    std::optional<Error> seek();
     void consumeData(const uint8_t* data, int bytesRead);
     void read();
     void readData(const WebCore::BlobDataItem&);