Prevent zero-length items from being appended to a blob.

BUG=128266
TEST=as in the bug


Review URL: https://chromiumcodereview.appspot.com/10386183

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

7 files changed, 65 insertions, 34 deletions

diff --git a/content/browser/fileapi/fileapi_message_filter.cc b/content/browser/fileapi/fileapi_message_filter.cc
index 6d4e050..e5dad26 100644
--- a/content/browser/fileapi/fileapi_message_filter.cc
+++ b/content/browser/fileapi/fileapi_message_filter.cc
@@ -172,6 +172,11 @@ void FileAPIMessageFilter::UnregisterOperation(int request_id) {
 
 FileAPIMessageFilter::~FileAPIMessageFilter() {}
 
+void FileAPIMessageFilter::BadMessageReceived() {
+  content::RecordAction(UserMetricsAction("BadMessageTerminate_FAMF"));
+  BrowserMessageFilter::BadMessageReceived();
+}
+
 void FileAPIMessageFilter::OnOpen(
     int request_id, const GURL& origin_url, fileapi::FileSystemType type,
     int64 requested_size, bool create) {
@@ -463,12 +468,20 @@ void FileAPIMessageFilter::OnAppendBlobDataItem(
     OnRemoveBlob(url);
     return;
   }
+  if (item.length == 0) {
+    BadMessageReceived();
+    return;
+  }
   blob_storage_context_->controller()->AppendBlobDataItem(url, item);
 }
 
 void FileAPIMessageFilter::OnAppendSharedMemory(
     const GURL& url, base::SharedMemoryHandle handle, size_t buffer_size) {
   DCHECK(base::SharedMemory::IsHandleValid(handle));
+  if (!buffer_size) {
+    BadMessageReceived();
+    return;
+  }
 #if defined(OS_WIN)
   base::SharedMemory shared_memory(handle, true, peer_handle());
 #else
@@ -693,3 +706,4 @@ FileSystemOperationInterface* FileAPIMessageFilter::GetNewOperation(
   operations_.AddWithID(operation, request_id);
   return operation;
 }
+
diff --git a/content/browser/fileapi/fileapi_message_filter.h b/content/browser/fileapi/fileapi_message_filter.h
index c6d94ed9..88b8197 100644
--- a/content/browser/fileapi/fileapi_message_filter.h
+++ b/content/browser/fileapi/fileapi_message_filter.h
@@ -68,6 +68,8 @@ class FileAPIMessageFilter : public content::BrowserMessageFilter {
  protected:
   virtual ~FileAPIMessageFilter();
 
+  virtual void BadMessageReceived() OVERRIDE;
+
  private:
   void OnOpen(int request_id,
               const GURL& origin_url,
diff --git a/content/common/fileapi/webblobregistry_impl.cc b/content/common/fileapi/webblobregistry_impl.cc
index 924e919..9a80fb9 100644
--- a/content/common/fileapi/webblobregistry_impl.cc
+++ b/content/common/fileapi/webblobregistry_impl.cc
@@ -39,6 +39,8 @@ void WebBlobRegistryImpl::registerBlobURL(
       case WebBlobData::Item::TypeData: {
         // WebBlobData does not allow partial data items.
         DCHECK(!data_item.offset && data_item.length == -1);
+        if (data_item.data.size() == 0)
+          break;
         if (data_item.data.size() < kLargeThresholdBytes) {
           item.SetToData(data_item.data.data(), data_item.data.size());
           child_thread_->Send(new BlobHostMsg_AppendBlobDataItem(url, item));
@@ -64,12 +66,14 @@ void WebBlobRegistryImpl::registerBlobURL(
         break;
       }
       case WebBlobData::Item::TypeFile:
-        item.SetToFile(
-            webkit_glue::WebStringToFilePath(data_item.filePath),
-            static_cast<uint64>(data_item.offset),
-            static_cast<uint64>(data_item.length),
-            base::Time::FromDoubleT(data_item.expectedModificationTime));
-        child_thread_->Send(new BlobHostMsg_AppendBlobDataItem(url, item));
+        if (data_item.length) {
+          item.SetToFile(
+              webkit_glue::WebStringToFilePath(data_item.filePath),
+              static_cast<uint64>(data_item.offset),
+              static_cast<uint64>(data_item.length),
+              base::Time::FromDoubleT(data_item.expectedModificationTime));
+          child_thread_->Send(new BlobHostMsg_AppendBlobDataItem(url, item));
+        }
         break;
       case WebBlobData::Item::TypeBlob:
         if (data_item.length) {
@@ -77,8 +81,8 @@ void WebBlobRegistryImpl::registerBlobURL(
               data_item.blobURL,
               static_cast<uint64>(data_item.offset),
               static_cast<uint64>(data_item.length));
+          child_thread_->Send(new BlobHostMsg_AppendBlobDataItem(url, item));
         }
-        child_thread_->Send(new BlobHostMsg_AppendBlobDataItem(url, item));
         break;
       default:
         NOTREACHED();
diff --git a/webkit/blob/blob_data.cc b/webkit/blob/blob_data.cc
index 33090e7..fb20ad9 100644
--- a/webkit/blob/blob_data.cc
+++ b/webkit/blob/blob_data.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -84,4 +84,35 @@ BlobData::BlobData(const WebBlobData& data) {
 
 BlobData::~BlobData() {}
 
+void BlobData::AppendData(const char* data, size_t length) {
+  DCHECK(length > 0);
+  items_.push_back(Item());
+  items_.back().SetToData(data, length);
+}
+
+void BlobData::AppendFile(const FilePath& file_path, uint64 offset,
+                          uint64 length,
+                          const base::Time& expected_modification_time) {
+  DCHECK(length > 0);
+  items_.push_back(Item());
+  items_.back().SetToFile(file_path, offset, length,
+                          expected_modification_time);
+}
+
+void BlobData::AppendBlob(const GURL& blob_url, uint64 offset, uint64 length) {
+  DCHECK(length > 0);
+  items_.push_back(Item());
+  items_.back().SetToBlob(blob_url, offset, length);
+}
+
+int64 BlobData::GetMemoryUsage() const {
+  int64 memory = 0;
+  for (std::vector<Item>::const_iterator iter = items_.begin();
+       iter != items_.end(); ++iter) {
+    if (iter->type == TYPE_DATA)
+      memory += iter->data.size();
+  }
+  return memory;
+}
+
 }  // namespace webkit_blob
diff --git a/webkit/blob/blob_data.h b/webkit/blob/blob_data.h
index c1296a3..3d276f0 100644
--- a/webkit/blob/blob_data.h
+++ b/webkit/blob/blob_data.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -85,24 +85,12 @@ class BLOB_EXPORT BlobData : public base::RefCounted<BlobData> {
     AppendData(data.c_str(), data.size());
   }
 
-  void AppendData(const char* data, size_t length) {
-    if (length > 0) {
-      items_.push_back(Item());
-      items_.back().SetToData(data, length);
-    }
-  }
+  void AppendData(const char* data, size_t length);
 
   void AppendFile(const FilePath& file_path, uint64 offset, uint64 length,
-                  const base::Time& expected_modification_time) {
-    items_.push_back(Item());
-    items_.back().SetToFile(file_path, offset, length,
-                            expected_modification_time);
-  }
+                  const base::Time& expected_modification_time);
 
-  void AppendBlob(const GURL& blob_url, uint64 offset, uint64 length) {
-    items_.push_back(Item());
-    items_.back().SetToBlob(blob_url, offset, length);
-  }
+  void AppendBlob(const GURL& blob_url, uint64 offset, uint64 length);
 
   void AttachShareableFileReference(ShareableFileReference* reference) {
     shareable_files_.push_back(reference);
@@ -122,15 +110,7 @@ class BLOB_EXPORT BlobData : public base::RefCounted<BlobData> {
     content_disposition_ = content_disposition;
   }
 
-  int64 GetMemoryUsage() const {
-    int64 memory = 0;
-    for (std::vector<Item>::const_iterator iter = items_.begin();
-         iter != items_.end(); ++iter) {
-      if (iter->type == TYPE_DATA)
-        memory += iter->data.size();
-    }
-    return memory;
-  }
+  int64 GetMemoryUsage() const;
 
  private:
   friend class base::RefCounted<BlobData>;
diff --git a/webkit/blob/blob_storage_controller.cc b/webkit/blob/blob_storage_controller.cc
index ed617e6..9dcbc33 100644
--- a/webkit/blob/blob_storage_controller.cc
+++ b/webkit/blob/blob_storage_controller.cc
@@ -68,6 +68,7 @@ void BlobStorageController::AppendBlobDataItem(
   // All the Blob items in the passing blob data are resolved and expanded into
   // a set of Data and File items.
 
+  DCHECK(item.length > 0);
   switch (item.type) {
     case BlobData::TYPE_DATA:
       // WebBlobData does not allow partial data.
diff --git a/webkit/fileapi/file_system_operation_write_unittest.cc b/webkit/fileapi/file_system_operation_write_unittest.cc
index 2b3fec8..2288c82 100644
--- a/webkit/fileapi/file_system_operation_write_unittest.cc
+++ b/webkit/fileapi/file_system_operation_write_unittest.cc
@@ -234,7 +234,6 @@ TEST_F(FileSystemOperationWriteTest, TestWriteSuccess) {
 TEST_F(FileSystemOperationWriteTest, TestWriteZero) {
   GURL blob_url("blob:zero");
   scoped_refptr<webkit_blob::BlobData> blob_data(new webkit_blob::BlobData());
-  blob_data->AppendData("");
 
   TestURLRequestContext url_request_context;
   url_request_context.blob_storage_controller()->AddFinishedBlob(