Fix the case where the browser livelocks if we cannot open a file.

If one tries to upload a file that one doesn't have read access to,
the browser livelocks. It tries to read from the file, gets nothing
but spins forever because it knows that it hasn't finished reading.

To address this, firstly we add a check at stat() time to make sure
that we can read the file. However, this doesn't take care of the case
where the access() call was incorrect, or the permissions have changed
under us. In this case, we replace the missing file with NULs.

(Land attempt three: first in r39446, reverted in r39448. Second in
r39899, reverted in r39901.)

http://codereview.chromium.org/541022
BUG=30850
TEST=Try to upload a file that isn't readable (i.e. /etc/shadow). The resulting upload should be a 0 byte file.

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@40146 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 38 insertions, 9 deletions

diff --git a/net/base/upload_data.h b/net/base/upload_data.h
index 3aab835..d01b435 100644
--- a/net/base/upload_data.h
+++ b/net/base/upload_data.h
@@ -9,6 +9,7 @@
 
 #include "base/basictypes.h"
 #include "base/file_path.h"
+#include "base/platform_file.h"
 #include "base/ref_counted.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
 
@@ -26,7 +27,8 @@ class UploadData : public base::RefCounted<UploadData> {
   class Element {
    public:
     Element() : type_(TYPE_BYTES), file_range_offset_(0),
-                file_range_length_(kuint64max),
+                file_range_length_(0),
+                file_(base::kInvalidPlatformFileValue),
                 override_content_length_(false) {
     }
 
@@ -45,19 +47,42 @@ class UploadData : public base::RefCounted<UploadData> {
       SetToFilePathRange(path, 0, kuint64max);
     }
 
-    void SetToFilePathRange(const FilePath& path,
-                            uint64 offset, uint64 length) {
-      type_ = TYPE_FILE;
-      file_path_ = path;
-      file_range_offset_ = offset;
-      file_range_length_ = length;
-    }
+    void SetToFilePathRange(const FilePath& path, uint64 offset, uint64 length);
 
     // Returns the byte-length of the element.  For files that do not exist, 0
     // is returned.  This is done for consistency with Mozilla.
-    uint64 GetContentLength() const;
+    uint64 GetContentLength() const {
+      if (override_content_length_)
+        return content_length_;
+
+      if (type_ == TYPE_BYTES) {
+        return bytes_.size();
+      } else {
+        return file_range_length_;
+      }
+    }
+
+    // For a TYPE_FILE, return a handle to the file. The caller does not take
+    // ownership and should not close the file handle.
+    base::PlatformFile platform_file() const;
+
+    // For a TYPE_FILE, this closes the file handle. It's a fatal error to call
+    // platform_file() after this.
+    void Close();
 
    private:
+    // type_ == TYPE_BYTES:
+    //   bytes_ is valid
+    // type_ == TYPE_FILE:
+    //   file_path_ should always be valid.
+    //
+    //   platform_file() may be invalid, in which case file_range_* are 0 and
+    //   file_ is invalid. This occurs when we cannot open the requested file.
+    //
+    //   Else, then file_range_* are within range of the length of the file
+    //   that we found when opening the file. Also, the sum of offset and
+    //   length will not overflow a uint64. file_ will be handle to the file.
+
     // Allows tests to override the result of GetContentLength.
     void SetContentLength(uint64 content_length) {
       override_content_length_ = true;
@@ -69,6 +94,7 @@ class UploadData : public base::RefCounted<UploadData> {
     FilePath file_path_;
     uint64 file_range_offset_;
     uint64 file_range_length_;
+    base::PlatformFile file_;
     bool override_content_length_;
     uint64 content_length_;
 
@@ -109,6 +135,9 @@ class UploadData : public base::RefCounted<UploadData> {
     elements_.swap(*elements);
   }
 
+  // CloseFiles closes the file handles of all Elements of type TYPE_FILE.
+  void CloseFiles();
+
   // Identifies a particular upload instance, which is used by the cache to
   // formulate a cache key.  This value should be unique across browser
   // sessions.  A value of 0 is used to indicate an unspecified identifier.