Validate image files before writing them to media galleries.

chrome/browser/media_galleries/fileapi/supported_image_type_validator.cc

Index: chrome/browser/media_galleries/fileapi/supported_image_type_validator.cc
diff --git a/chrome/browser/media_galleries/fileapi/supported_image_type_validator.cc b/chrome/browser/media_galleries/fileapi/supported_image_type_validator.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9b503a8eed5dd31a8b229ec1c2d4c716cfbf86be
--- /dev/null
+++ b/chrome/browser/media_galleries/fileapi/supported_image_type_validator.cc
@@ -0,0 +1,154 @@
+// Copyright 2013 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 "chrome/browser/media_galleries/fileapi/supported_image_type_validator.h"
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/location.h"
+#include "base/logging.h"
+#include "base/memory/scoped_generic_obj.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/threading/sequenced_worker_pool.h"
+#include "base/threading/thread_restrictions.h"
+#include "chrome/browser/image_decoder.h"
+#include "content/public/browser/browser_thread.h"
+
+using content::BrowserThread;
+
+namespace chrome {
+
+namespace {
+
+const int kMaxImageFileSize = 50*1014*1024;
+
+class PlatformFileCloser {
+ public:
+  void operator()(base::PlatformFile file) const {
+    if (file != base::kInvalidPlatformFileValue)
+      base::ClosePlatformFile(file);
+  }
+};
+
+typedef ScopedGenericObj<base::PlatformFile, PlatformFileCloser>

[Lei Zhang, 2013/05/25 00:09:01]

I think you can replace this with a scoped_ptr that has a custom deleter.
ScopedGenericObj seems to be on its way out.

[vandebo (ex-Chrome), 2013/05/29 06:09:15]

Done.

+    ScopedPlatformFile;
+
+scoped_ptr<std::string> ReadOnFileThread(const base::FilePath& path) {
+  base::ThreadRestrictions::AssertIOAllowed();
+  scoped_ptr<std::string> empty_result;
+
+  ScopedPlatformFile file(base::CreatePlatformFile(
+      path, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ, NULL, NULL));
+  if (file == base::kInvalidPlatformFileValue)
+    return empty_result.Pass();
+
+  base::PlatformFileInfo file_info;
+  if (!base::GetPlatformFileInfo(file.get(), &file_info) ||
+      file_info.size > kMaxImageFileSize) {

[Lei Zhang, 2013/05/25 00:09:01]

Is this to prevent the utility process from going OOM? How does the rest of the
ImageDecoder users handle large images?

[vandebo (ex-Chrome), 2013/05/29 06:09:15]

It's a sanity check. An image file (in a supported format) that is > 50MB is
unreasonable.  I'm happy to adjust the limit if you like.

+    return empty_result.Pass();
+  }
+
+  scoped_ptr<char[]> data(new char[file_info.size]);

[Lei Zhang, 2013/05/25 00:09:01]

You can probably get rid of |data| and the need to construct a new std::string
if you turn |empty_result| into |result|, and here do:

result->resize(file_info.size);
... ReadPlatformFile(..., string_as_array(result), ...) ...

string_as_array() is in base/stl_util.h.

[vandebo (ex-Chrome), 2013/05/29 06:09:15]

Done.

+  if (base::ReadPlatformFile(file, 0, data.get(),
+                             file_info.size) != file_info.size) {
+    return empty_result.Pass();
+  }
+
+  return make_scoped_ptr(new std::string(data.get(), file_info.size)).Pass();
+}
+
+class ImageDecoderDelegateAdapter : public ImageDecoder::Delegate {
+ public:
+  ImageDecoderDelegateAdapter(
+      scoped_ptr<std::string> data,
+      const fileapi::CopyOrMoveFileValidator::ResultCallback& callback)
+      : data_(data.Pass()),
+        callback_(callback) {
+    DCHECK(data_);
+  }
+
+  const std::string& data() {
+    return *data_;
+  }
+
+  // ImageDecoder::Delegate methods.
+  virtual void OnImageDecoded(const ImageDecoder* /*decoder*/,
+                              const SkBitmap& /*decoded_image*/) {

[Lei Zhang, 2013/05/25 00:09:01]

OVERRIDE, same with failed.

[vandebo (ex-Chrome), 2013/05/29 06:09:15]

Done.

+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(callback_, base::PLATFORM_FILE_OK));
+    delete this;
+  }
+
+  virtual void OnDecodeImageFailed(const ImageDecoder* /*decoder*/) {
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(callback_,
+                                       base::PLATFORM_FILE_ERROR_SECURITY));
+    delete this;
+  }
+
+ private:
+  scoped_ptr<std::string> data_;
+  fileapi::CopyOrMoveFileValidator::ResultCallback callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(ImageDecoderDelegateAdapter);
+};
+
+}  // namespace
+
+SupportedImageTypeValidator::~SupportedImageTypeValidator() {}
+
+// static
+bool SupportedImageTypeValidator::SupportsFileType(const base::FilePath& path) {
+  base::FilePath::StringType extension = path.Extension();
+  return extension == FILE_PATH_LITERAL(".bmp") ||
+         extension == FILE_PATH_LITERAL(".gif") ||
+         extension == FILE_PATH_LITERAL(".jfif") ||
+         extension == FILE_PATH_LITERAL(".jpeg") ||
+         extension == FILE_PATH_LITERAL(".jpg") ||
+         extension == FILE_PATH_LITERAL(".pjp") ||
+         extension == FILE_PATH_LITERAL(".pjpeg") ||
+         extension == FILE_PATH_LITERAL(".png") ||
+         extension == FILE_PATH_LITERAL(".webp");
+}
+
+void SupportedImageTypeValidator::StartValidation(
+    const fileapi::CopyOrMoveFileValidator::ResultCallback& result_callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  callback_ = result_callback;

[Lei Zhang, 2013/05/25 00:09:01]

It seems the current usage is to create a new validator per file, so this method
is not re-entrant. At least DCHECK(!callback_) as a pre-condition?

[vandebo (ex-Chrome), 2013/05/29 06:09:15]

Done.

+
+  BrowserThread::PostTaskAndReplyWithResult(
+      BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(&ReadOnFileThread, path_),
+      base::Bind(&SupportedImageTypeValidator::OnFileOpen,
+                 weak_factory_.GetWeakPtr()));
+}
+
+SupportedImageTypeValidator::SupportedImageTypeValidator(
+    const base::FilePath& path)
+    : path_(path),
+      weak_factory_(this) {
+}
+
+void SupportedImageTypeValidator::OnFileOpen(scoped_ptr<std::string> data) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  if (!data.get()) {

[Lei Zhang, 2013/05/25 00:09:01]

What about when the file is 0 bytes?

[vandebo (ex-Chrome), 2013/05/29 06:09:15]

It can't happen because a zero byte file will not pass the mime checker.  But by
the same reason, it can't be a valid image file at zero bytes.

+    callback_.Run(base::PLATFORM_FILE_ERROR_SECURITY);
+    return;
+  }
+
+  // |adapter| will delete itself after a completion message is received.
+  ImageDecoderDelegateAdapter* adapter =
+      new ImageDecoderDelegateAdapter(data.Pass(), callback_);
+  decoder_ = new ImageDecoder(adapter, adapter->data(),
+                              ImageDecoder::DEFAULT_CODEC);
+  base::SequencedWorkerPool* workerpool = BrowserThread::GetBlockingPool();
+  decoder_->Start(workerpool->GetSequencedTaskRunnerWithShutdownBehavior(
+        workerpool->GetSequenceToken(),
+        base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN));
+}
+
+}  // namespace chrome