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.

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@39446 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 59 insertions, 18 deletions

diff --git a/net/base/upload_data.cc b/net/base/upload_data.cc
index 0496873..d030b3d 100644
--- a/net/base/upload_data.cc
+++ b/net/base/upload_data.cc
@@ -5,40 +5,81 @@
 #include "net/base/upload_data.h"
 
 #include "base/file_util.h"
+#include "base/platform_file.h"
 #include "base/logging.h"
 
 namespace net {
 
-uint64 UploadData::GetContentLength() const {
+uint64 UploadData::GetContentLength() {
   uint64 len = 0;
-  std::vector<Element>::const_iterator it = elements_.begin();
+  std::vector<Element>::iterator it = elements_.begin();
   for (; it != elements_.end(); ++it)
     len += (*it).GetContentLength();
   return len;
 }
 
-uint64 UploadData::Element::GetContentLength() const {
-  if (override_content_length_)
-    return content_length_;
+void UploadData::CloseFiles() {
+  std::vector<Element>::iterator it = elements_.begin();
+  for (; it != elements_.end(); ++it) {
+    if (it->type() == TYPE_FILE)
+      it->Close();
+  }
+}
+
+base::PlatformFile UploadData::Element::platform_file() const {
+  DCHECK(type_ == TYPE_FILE) << "platform_file on non-file Element";
+
+  if (file_ != NULL) {
+    return file_->get();
+  } else {
+    return base::kInvalidPlatformFileValue;
+  }
+}
+
+void UploadData::Element::Close() {
+  DCHECK(type_ == TYPE_FILE) << "Close on non-file Element";
+
+  if (file_ != NULL)
+    file_->release();
+}
 
-  if (type_ == TYPE_BYTES)
-    return static_cast<uint64>(bytes_.size());
+void UploadData::Element::SetToFilePathRange(const FilePath& path,
+                                             uint64 offset,
+                                             uint64 length) {
+  type_ = TYPE_FILE;
+  file_range_offset_ = 0;
+  file_range_length_ = 0;
 
-  DCHECK(type_ == TYPE_FILE);
+  if (offset + length < offset) {
+    LOG(ERROR) << "Upload file offset and length overflow 64-bits. Ignoring.";
+    return;
+  }
 
-  // TODO(darin): This size calculation could be out of sync with the state of
-  // the file when we get around to reading it.  We should probably find a way
-  // to lock the file or somehow protect against this error condition.
+  base::PlatformFile file = base::CreatePlatformFile(
+      path, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ, NULL);
+  if (file == base::kInvalidPlatformFileValue) {
+    // This case occurs when the user selects a file that isn't readable.
+    file_path_= path;
+    return;
+  }
 
-  int64 length = 0;
-  if (!file_util::GetFileSize(file_path_, &length))
-    return 0;
+  uint64 file_size;
+  if (!base::GetPlatformFileSize(file, &file_size)) {
+    base::ClosePlatformFile(file);
+    return;
+  }
 
-  if (file_range_offset_ >= static_cast<uint64>(length))
-    return 0;  // range is beyond eof
+  if (offset > file_size) {
+    base::ClosePlatformFile(file);
+    return;
+  }
+  if (offset + length > file_size)
+    length = file_size - offset;
 
-  // compensate for the offset and clip file_range_length_ to eof
-  return std::min(length - file_range_offset_, file_range_length_);
+  file_ = new base::RefCountedPlatformFile(file);
+  file_path_ = path;
+  file_range_offset_ = offset;
+  file_range_length_ = length;
 }
 
 }  // namespace net