Detect file system errors during downloads.

content/browser/download/download_file_manager.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/download/download_file_manager.h"
 
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/task.h"
@@ skipping 150 matching lines @@
 // DownloadFile has been deleted.
 void DownloadFileManager::UpdateDownload(int id, DownloadBuffer* buffer) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   std::vector<DownloadBuffer::Contents> contents;
   {
     base::AutoLock auto_lock(buffer->lock);
     contents.swap(buffer->contents);
   }
 
   DownloadFile* download_file = GetDownloadFile(id);
+  bool had_error = false;
   for (size_t i = 0; i < contents.size(); ++i) {
     net::IOBuffer* data = contents[i].first;
     const int data_len = contents[i].second;
-    if (download_file)
-      download_file->AppendDataToFile(data->data(), data_len);
+    if (!had_error && download_file) {
+      bool write_succeeded =
+          download_file->AppendDataToFile(data->data(), data_len);
+      if (!write_succeeded) {
+        // Write failed: interrupt the download.
+        std::string hash;

[cbentzel, 2011/08/19 18:44:21]

hash does not appear to be used.

[ahendrickson, 2011/08/22 14:53:30]

Ah, leftover from a refactor.

Removed.

+        DownloadManager* download_manager = download_file->GetDownloadManager();
+        had_error = true;
+
+        // Calling this here in case we get more data, to avoid calling

[cbentzel, 2011/08/19 18:44:21]

Update this comment to discuss the issue with writing to a corrupted file, it's
a more compelling argument than this one.

[Randy Smith (Not in Mondays), 2011/08/19 20:09:13]

I'd suggest "this"->"CancelDownload" in the comment since you're a line away
with another function call in between.

[ahendrickson, 2011/08/22 14:53:30]

Moved the comment.

[ahendrickson, 2011/08/22 14:53:30]

Done.

+        // |OnDownloadError()| more than once.
+        int64 bytes_downloaded = download_file->bytes_so_far();
+        CancelDownload(id);
+

[cbentzel, 2011/08/19 18:44:21]

Perhaps you should do a 

download_file = NULL;

after CancelDownload().

This makes it clear that it should no longer be dereferenced. It would also let
you remove |had_error|.

[ahendrickson, 2011/08/22 14:53:30]

Done.

+        if (download_manager) {
+          BrowserThread::PostTask(
+              BrowserThread::UI,
+              FROM_HERE,
+              NewRunnableMethod(
+                  download_manager,
+                  &DownloadManager::OnDownloadError,
+                  id,
+                  bytes_downloaded,
+                  net::ERR_FAILED));
+        }
+      }
+    }
     data->Release();
   }
 }
 
 void DownloadFileManager::OnResponseCompleted(
     int id,
     DownloadBuffer* buffer,
-    int os_error,
+    int net_error,
     const std::string& security_info) {
   VLOG(20) << __FUNCTION__ << "()" << " id = " << id
-           << " os_error = " << os_error
+           << " net_error = " << net_error
            << " security_info = \"" << security_info << "\"";
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   delete buffer;
   DownloadFile* download_file = GetDownloadFile(id);
   if (!download_file)
     return;
 
   download_file->Finish();
 
   DownloadManager* download_manager = download_file->GetDownloadManager();
   if (!download_manager) {
     CancelDownload(id);
     return;
   }
 
   std::string hash;
   if (!download_file->GetSha256Hash(&hash))
     hash.clear();
 
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      NewRunnableMethod(
-        download_manager, &DownloadManager::OnResponseCompleted,
-        id, download_file->bytes_so_far(), os_error, hash));
+  // ERR_CONNECTION_CLOSED is allowed since a number of servers in the wild
+  // advertise a larger Content-Length than the amount of bytes in the message
+  // body, and then close the connection. Other browsers - IE8, Firefox 4.0.1,
+  // and Safari 5.0.4 - treat the download as complete in this case, so we
+  // follow their lead.
+  if (net_error == net::OK || net_error == net::ERR_CONNECTION_CLOSED) {
+    BrowserThread::PostTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        NewRunnableMethod(
+            download_manager,
+            &DownloadManager::OnResponseCompleted,
+            id,
+            download_file->bytes_so_far(),
+            hash));
+  } else {
+    BrowserThread::PostTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        NewRunnableMethod(
+            download_manager,
+            &DownloadManager::OnDownloadError,
+            id,
+            download_file->bytes_so_far(),
+            net_error));
+  }
   // We need to keep the download around until the UI thread has finalized
   // the name.
 }
 
 // This method will be sent via a user action, or shutdown on the UI thread, and
 // run on the download thread. Since this message has been sent from the UI
 // thread, the download may have already completed and won't exist in our map.
 void DownloadFileManager::CancelDownload(int id) {
   VLOG(20) << __FUNCTION__ << "()" << " id = " << id;
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
@@ skipping 65 matching lines @@
   DownloadFile* download_file = GetDownloadFile(id);
   if (!download_file)
     return;
 
   VLOG(20) << __FUNCTION__ << "()"
            << " download_file = " << download_file->DebugString();
 
   if (!download_file->Rename(full_path)) {
     // Error. Between the time the UI thread generated 'full_path' to the time
     // this code runs, something happened that prevents us from renaming.
-    CancelDownloadOnRename(id);
+    CancelDownloadOnRename(id, net::ERR_FAILED);
   }
 }
 
 // The DownloadManager in the UI thread has provided a final name for the
 // download specified by 'id'. Rename the download that's in the process
 // of completing.
 //
 // There are 2 possible rename cases where this method can be called:
 // 1. foo.crdownload -> foo (final, safe)
 // 2. Unconfirmed.xxx.crdownload -> xxx (final, validated)
@@ skipping 27 matching lines @@
     uniquifier = download_util::GetUniquePathNumber(new_path);
     if (uniquifier > 0) {
       download_util::AppendNumberToPath(&new_path, uniquifier);
     }
   }
 
   // Rename the file, overwriting if necessary.
   if (!download_file->Rename(new_path)) {
     // Error. Between the time the UI thread generated 'full_path' to the time
     // this code runs, something happened that prevents us from renaming.
-    CancelDownloadOnRename(id);
+    CancelDownloadOnRename(id, net::ERR_FAILED);
     return;
   }
 
 #if defined(OS_MACOSX)
   // Done here because we only want to do this once; see
   // http://crbug.com/13120 for details.
   download_file->AnnotateWithSourceInformation();
 #endif
 
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       NewRunnableMethod(
           download_manager, &DownloadManager::OnDownloadRenamedToFinalName, id,
           new_path, uniquifier));
 }
 
 // Called only from RenameInProgressDownloadFile and
 // RenameCompletingDownloadFile on the FILE thread.
-void DownloadFileManager::CancelDownloadOnRename(int id) {
+void DownloadFileManager::CancelDownloadOnRename(int id, int rename_error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   DownloadFile* download_file = GetDownloadFile(id);
   if (!download_file)
     return;
 
   DownloadManager* download_manager = download_file->GetDownloadManager();
   if (!download_manager) {
     // Without a download manager, we can't cancel the request normally, so we
     // need to do it here.  The normal path will also update the download
-    // history before cancelling the request.
+    // history before canceling the request.
     download_file->CancelDownloadRequest();
     return;
   }
 
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       NewRunnableMethod(download_manager,
-                        &DownloadManager::DownloadCancelled, id));
+                        &DownloadManager::OnDownloadError,
+                        id,
+                        download_file->bytes_so_far(),
+                        rename_error));
 }
 
 void DownloadFileManager::EraseDownload(int id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   if (!ContainsKey(downloads_, id))
     return;
 
   DownloadFile* download_file = downloads_[id];
 
   VLOG(20) << " " << __FUNCTION__ << "()"
            << " id = " << id
            << " download_file = " << download_file->DebugString();
 
   downloads_.erase(id);
 
   delete download_file;
 
   if (downloads_.empty())
     StopUpdateTimer();
 }