[webkit-changes] [173962] releases/WebKitGTK/webkit-2.4/Source/WebKit2

Thu, 25 Sep 2014 07:06:14 -0700

Title: [173962] releases/WebKitGTK/webkit-2.4/Source/WebKit2
Revision
173962
Author
[email protected]
Date
2014-09-25 07:05:25 -0700 (Thu, 25 Sep 2014)

Log Message

Merge r173252 - [SOUP] Race condition when downloading a file due to the intermediate temporary file
https://bugs.webkit.org/show_bug.cgi?id=136423

Patch by Michael Catanzaro <[email protected]> on 2014-09-03
Reviewed by Carlos Garcia Campos.

* Shared/Downloads/soup/DownloadSoup.cpp:
(WebKit::DownloadClient::DownloadClient): Replace m_destinationURI with
m_destinationFile and add m_createdDestination.
(WebKit::DownloadClient::deleteFilesIfNeeded): Added.
(WebKit::DownloadClient::downloadFailed): Call deleteFilesIfNeeded.
(WebKit::DownloadClient::didReceiveResponse): Attempt to create the
destination file before the intermediate file. Fail here if the file
exists and overwrite is not allowed, so we don't erroneously fire the
didCreateDestination event or waste time downloading the file when we
know the download will fail.
(WebKit::DownloadClient::didFinishLoading): Unconditionally overwrite
the empty destination file.
(WebKit::DownloadClient::cancel): Call deleteFilesIfNeeded.
(WebKit::DownloadClient::deleteIntermediateFileInNeeded): Deleted.

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.4/Source/WebKit2/ChangeLog (173961 => 173962)


--- releases/WebKitGTK/webkit-2.4/Source/WebKit2/ChangeLog	2014-09-25 13:56:41 UTC (rev 173961)
+++ releases/WebKitGTK/webkit-2.4/Source/WebKit2/ChangeLog	2014-09-25 14:05:25 UTC (rev 173962)
@@ -1,3 +1,25 @@
+2014-09-03  Michael Catanzaro  <[email protected]>
+
+        [SOUP] Race condition when downloading a file due to the intermediate temporary file
+        https://bugs.webkit.org/show_bug.cgi?id=136423
+
+        Reviewed by Carlos Garcia Campos.
+
+        * Shared/Downloads/soup/DownloadSoup.cpp:
+        (WebKit::DownloadClient::DownloadClient): Replace m_destinationURI with
+        m_destinationFile and add m_createdDestination.
+        (WebKit::DownloadClient::deleteFilesIfNeeded): Added.
+        (WebKit::DownloadClient::downloadFailed): Call deleteFilesIfNeeded.
+        (WebKit::DownloadClient::didReceiveResponse): Attempt to create the
+        destination file before the intermediate file. Fail here if the file
+        exists and overwrite is not allowed, so we don't erroneously fire the
+        didCreateDestination event or waste time downloading the file when we
+        know the download will fail.
+        (WebKit::DownloadClient::didFinishLoading): Unconditionally overwrite
+        the empty destination file.
+        (WebKit::DownloadClient::cancel): Call deleteFilesIfNeeded.
+        (WebKit::DownloadClient::deleteIntermediateFileInNeeded): Deleted.
+
 2014-09-01  Michael Catanzaro  <[email protected]>
 
         [SOUP] WebKitDownload cannot overwrite existing file

Modified: releases/WebKitGTK/webkit-2.4/Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp (173961 => 173962)


--- releases/WebKitGTK/webkit-2.4/Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp	2014-09-25 13:56:41 UTC (rev 173961)
+++ releases/WebKitGTK/webkit-2.4/Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp	2014-09-25 14:05:25 UTC (rev 173962)
@@ -60,16 +60,20 @@
             g_source_remove(m_handleResponseLaterID);
     }
 
-    void deleteIntermediateFileInNeeded()
+    void deleteFilesIfNeeded()
     {
-        if (!m_intermediateFile)
-            return;
-        g_file_delete(m_intermediateFile.get(), nullptr, nullptr);
+        if (m_destinationFile)
+            g_file_delete(m_destinationFile.get(), nullptr, nullptr);
+
+        if (m_intermediateFile) {
+            ASSERT(m_destinationFile);
+            g_file_delete(m_intermediateFile.get(), nullptr, nullptr);
+        }
     }
 
     void downloadFailed(const ResourceError& error)
     {
-        deleteIntermediateFileInNeeded();
+        deleteFilesIfNeeded();
         m_download->didFail(error, IPC::DataReference());
     }
 
@@ -91,8 +95,8 @@
             suggestedFilename = decodeURLEscapeSequences(url.lastPathComponent());
         }
 
-        m_destinationURI = m_download->decideDestinationWithSuggestedFilename(suggestedFilename, m_allowOverwrite);
-        if (m_destinationURI.isEmpty()) {
+        String destinationURI = m_download->decideDestinationWithSuggestedFilename(suggestedFilename, m_allowOverwrite);
+        if (destinationURI.isEmpty()) {
 #if PLATFORM(GTK)
             GUniquePtr<char> buffer(g_strdup_printf(_("Cannot determine destination URI for download with suggested filename %s"), suggestedFilename.utf8().data()));
             String errorMessage = String::fromUTF8(buffer.get());
@@ -103,16 +107,28 @@
             return;
         }
 
-        String intermediateURI = m_destinationURI + ".wkdownload";
+        m_destinationFile = adoptGRef(g_file_new_for_uri(destinationURI.utf8().data()));
+        GRefPtr<GFileOutputStream> outputStream;
+        GUniqueOutPtr<GError> error;
+        if (m_allowOverwrite)
+            outputStream = adoptGRef(g_file_replace(m_destinationFile.get(), nullptr, FALSE, G_FILE_CREATE_NONE, nullptr, &error.outPtr()));
+        else
+            outputStream = adoptGRef(g_file_create(m_destinationFile.get(), G_FILE_CREATE_NONE, nullptr, &error.outPtr()));
+        if (!outputStream) {
+            m_destinationFile.clear();
+            downloadFailed(platformDownloadDestinationError(response, error->message));
+            return;
+        }
+
+        String intermediateURI = destinationURI + ".wkdownload";
         m_intermediateFile = adoptGRef(g_file_new_for_uri(intermediateURI.utf8().data()));
-        GUniqueOutPtr<GError> error;
         m_outputStream = adoptGRef(g_file_replace(m_intermediateFile.get(), 0, TRUE, G_FILE_CREATE_NONE, 0, &error.outPtr()));
         if (!m_outputStream) {
             downloadFailed(platformDownloadDestinationError(response, error->message));
             return;
         }
 
-        m_download->didCreateDestination(m_destinationURI);
+        m_download->didCreateDestination(destinationURI);
     }
 
     void didReceiveData(ResourceHandle*, const char* data, unsigned length, int /*encodedDataLength*/)
@@ -136,10 +152,10 @@
     {
         m_outputStream = 0;
 
+        ASSERT(m_destinationFile);
         ASSERT(m_intermediateFile);
-        GRefPtr<GFile> destinationFile = adoptGRef(g_file_new_for_uri(m_destinationURI.utf8().data()));
         GUniqueOutPtr<GError> error;
-        if (!g_file_move(m_intermediateFile.get(), destinationFile.get(), m_allowOverwrite ? G_FILE_COPY_OVERWRITE : G_FILE_COPY_NONE, nullptr, nullptr, nullptr, &error.outPtr())) {
+        if (!g_file_move(m_intermediateFile.get(), m_destinationFile.get(), G_FILE_COPY_OVERWRITE, nullptr, nullptr, nullptr, &error.outPtr())) {
             downloadFailed(platformDownloadDestinationError(m_response, error->message));
             return;
         }
@@ -148,7 +164,7 @@
         CString uri = m_response.url().string().utf8();
         g_file_info_set_attribute_string(info.get(), "metadata::download-uri", uri.data());
         g_file_info_set_attribute_string(info.get(), "xattr::xdg.origin.url", uri.data());
-        g_file_set_attributes_async(destinationFile.get(), info.get(), G_FILE_QUERY_INFO_NONE, G_PRIORITY_DEFAULT, nullptr, nullptr, nullptr);
+        g_file_set_attributes_async(m_destinationFile.get(), info.get(), G_FILE_QUERY_INFO_NONE, G_PRIORITY_DEFAULT, nullptr, nullptr, nullptr);
 
         m_download->didFinish();
     }
@@ -171,7 +187,7 @@
     void cancel(ResourceHandle* handle)
     {
         handle->cancel();
-        deleteIntermediateFileInNeeded();
+        deleteFilesIfNeeded();
         m_download->didCancel(IPC::DataReference());
     }
 
@@ -202,7 +218,7 @@
     Download* m_download;
     GRefPtr<GFileOutputStream> m_outputStream;
     ResourceResponse m_response;
-    String m_destinationURI;
+    GRefPtr<GFile> m_destinationFile;
     GRefPtr<GFile> m_intermediateFile;
     ResourceResponse m_delayedResponse;
     unsigned m_handleResponseLaterID;

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to