[webkit-changes] [174153] releases/WebKitGTK/webkit-2.2/Source/WebKit2

Wed, 01 Oct 2014 01:56:21 -0700

Title: [174153] releases/WebKitGTK/webkit-2.2/Source/WebKit2
Revision
174153
Author
carlo...@webkit.org
Date
2014-10-01 01:55:08 -0700 (Wed, 01 Oct 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 <mcatanz...@igalia.com> 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.2/Source/WebKit2/ChangeLog (174152 => 174153)


--- releases/WebKitGTK/webkit-2.2/Source/WebKit2/ChangeLog	2014-10-01 08:49:58 UTC (rev 174152)
+++ releases/WebKitGTK/webkit-2.2/Source/WebKit2/ChangeLog	2014-10-01 08:55:08 UTC (rev 174153)
@@ -1,3 +1,25 @@
+2014-09-03  Michael Catanzaro  <mcatanz...@igalia.com>
+
+        [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  <mcatanz...@igalia.com>
 
         [SOUP] WebKitDownload cannot overwrite existing file

Modified: releases/WebKitGTK/webkit-2.2/Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp (174152 => 174153)


--- releases/WebKitGTK/webkit-2.2/Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp	2014-10-01 08:49:58 UTC (rev 174152)
+++ releases/WebKitGTK/webkit-2.2/Source/WebKit2/Shared/Downloads/soup/DownloadSoup.cpp	2014-10-01 08:55:08 UTC (rev 174153)
@@ -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, CoreIPC::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)
             GOwnPtr<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;
+        GOwnPtr<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()));
-        GOwnPtr<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, int 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()));
         GOwnPtr<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(CoreIPC::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
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to