Fix content verification code for undreadable and deleted files.

chrome/browser/extensions/extension_protocols_unittest.cc

 // Copyright 2014 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 <stddef.h>
 
 #include <memory>
 #include <string>
 
+#include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
+#include "base/test/test_file_util.h"
 #include "base/values.h"
+#include "chrome/browser/extensions/chrome_content_verifier_delegate.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/testing_profile.h"
+#include "components/crx_file/id_util.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/previews_state.h"
 #include "content/public/test/mock_resource_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_utils.h"
+#include "extensions/browser/content_verifier.h"
 #include "extensions/browser/extension_protocols.h"
 #include "extensions/browser/info_map.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/extension_builder.h"
 #include "extensions/common/file_util.h"
 #include "net/base/request_priority.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_job_factory_impl.h"
 #include "net/url_request/url_request_status.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using content::ResourceType;
 
 namespace extensions {
 namespace {
 
 base::FilePath GetTestPath(const std::string& name) {
   base::FilePath path;
   EXPECT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path));
   return path.AppendASCII("extensions").AppendASCII(name);
 }
 
+// Helper function that creates a file at |relative_path| within |directory|
+// and fills it with |content|.
+bool AddFileToDirectory(const base::FilePath& directory,
+                        const base::FilePath& relative_path,
+                        const std::string& content) {
+  base::FilePath full_path = directory.Append(relative_path);
+  if (!base::CreateDirectory(full_path.DirName()))

[Devlin, 2017/03/24 02:01:44]

In practice, shouldn't this be created as part of CreateUniqueTempDir()?

[lazyboy, 2017/03/24 18:27:47]

Of course. I copy pasted this function.
Done.

+    return false;
+  int result = base::WriteFile(full_path, content.data(), content.size());
+  return (static_cast<size_t>(result) == content.size());
+}
+
+std::unique_ptr<TestingProfile> GetTestingProfile() {
+  TestingProfile::Builder profile_builder;
+  return profile_builder.Build();
+}
+
 scoped_refptr<Extension> CreateTestExtension(const std::string& name,
                                              bool incognito_split_mode) {
   base::DictionaryValue manifest;
   manifest.SetString("name", name);
   manifest.SetString("version", "1");
   manifest.SetInteger("manifest_version", 2);
   manifest.SetString("incognito", incognito_split_mode ? "split" : "spanning");
 
   base::FilePath path = GetTestPath("response_headers");
 
@@ skipping 35 matching lines @@
   base::FilePath path = GetTestPath("response_headers");
 
   std::string error;
   scoped_refptr<Extension> extension(
       Extension::Create(path, Manifest::UNPACKED, manifest,
                         Extension::NO_FLAGS, &error));
   EXPECT_TRUE(extension.get()) << error;
   return extension;
 }
 
+// A ContentVerifyJob::TestDelegate that observes DoneReading().
+class JobDelegate : public ContentVerifyJob::TestDelegate {
+ public:
+  JobDelegate() {}
+  ~JobDelegate() override {}
+
+  ContentVerifyJob::FailureReason BytesRead(const ExtensionId& id,
+                                            int count,

[Devlin, 2017/03/24 02:01:44]

Is it worth verifying the bytes read against expectations?

[lazyboy, 2017/03/24 18:27:47]

Not in particular for this test, but doesn't hurt.
Done.

+                                            const char* data) override {
+    return ContentVerifyJob::NONE;
+  }
+
+  ContentVerifyJob::FailureReason DoneReading(const ExtensionId& id) override {
+    seen_done_reading_extension_ids_.insert(id);
+    if (waiting_for_extension_id_ && *waiting_for_extension_id_ == id &&
+        loop_runner_.get() && loop_runner_->loop_running()) {
+      loop_runner_->Quit();
+    }
+    return ContentVerifyJob::NONE;
+  }
+
+  void WaitForDoneReading(const ExtensionId& id) {
+    ASSERT_FALSE(waiting_for_extension_id_);
+    if (seen_done_reading_extension_ids_.count(id))
+      return;
+    waiting_for_extension_id_ = base::MakeUnique<ExtensionId>(id);
+    loop_runner_ = new content::MessageLoopRunner();

[Devlin, 2017/03/24 02:01:44]

I typically prefer base::RunLoop for this, since the Quit() syntax is a little
easier and more forgiving (e.g., you can call Quit() when the loop isn't
running, etc).  Is there a reason to use MessageLoopRunner here?

[lazyboy, 2017/03/24 18:27:47]

Thanks.
Done.

+    loop_runner_->Run();
+    loop_runner_ = nullptr;
+  }
+
+  void Reset() {
+    waiting_for_extension_id_.reset();
+    seen_done_reading_extension_ids_.clear();
+  }
+
+ private:
+  std::set<ExtensionId> seen_done_reading_extension_ids_;
+  std::unique_ptr<ExtensionId> waiting_for_extension_id_;

[Devlin, 2017/03/24 02:01:44]

nit: I think base::Optional is a better fit here.

[lazyboy, 2017/03/24 18:27:47]

Done.

+  scoped_refptr<content::MessageLoopRunner> loop_runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(JobDelegate);
+};
+
 }  // namespace
 
 // This test lives in src/chrome instead of src/extensions because it tests
 // functionality delegated back to Chrome via ChromeExtensionsBrowserClient.
 // See chrome/browser/extensions/chrome_url_request_util.cc.
-class ExtensionProtocolTest : public testing::Test {
+class ExtensionProtocolsTest : public testing::Test {
  public:
-  ExtensionProtocolTest()
+  ExtensionProtocolsTest()
       : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP),
         old_factory_(NULL),
         resource_context_(&test_url_request_context_) {}
 
   void SetUp() override {
     testing::Test::SetUp();
+    testing_profile_ = GetTestingProfile();

[Devlin, 2017/03/24 02:01:44]

nitty nit: maybe just inline:
testing_profile_ = TestingProfile::Builder().Build();

[lazyboy, 2017/03/24 18:27:47]

Done.

     extension_info_map_ = new InfoMap();
     net::URLRequestContext* request_context =
         resource_context_.GetRequestContext();
     old_factory_ = request_context->job_factory();
+
+    // Set up content verification.
+    base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+    command_line->AppendSwitchASCII(
+        switches::kExtensionContentVerification,
+        switches::kExtensionContentVerificationEnforce);
+    content_verifier_ = new ContentVerifier(
+        testing_profile_.get(),
+        new ChromeContentVerifierDelegate(testing_profile_.get()));
+    extension_info_map_->SetContentVerifier(content_verifier_.get());
   }
 
   void TearDown() override {
     net::URLRequestContext* request_context =
         resource_context_.GetRequestContext();
     request_context->set_job_factory(old_factory_);
   }
 
   void SetProtocolHandler(bool is_incognito) {
     net::URLRequestContext* request_context =
@@ skipping 39 matching lines @@
   }
 
  protected:
   content::TestBrowserThreadBundle thread_bundle_;
   scoped_refptr<InfoMap> extension_info_map_;
   net::URLRequestJobFactoryImpl job_factory_;
   const net::URLRequestJobFactory* old_factory_;
   net::TestDelegate test_delegate_;
   net::TestURLRequestContext test_url_request_context_;
   content::MockResourceContext resource_context_;
+  scoped_refptr<ContentVerifier> content_verifier_;
+  std::unique_ptr<TestingProfile> testing_profile_;
 };
 
 // Tests that making a chrome-extension request in an incognito context is
 // only allowed under the right circumstances (if the extension is allowed
 // in incognito, and it's either a non-main-frame request or a split-mode
 // extension).
-TEST_F(ExtensionProtocolTest, IncognitoRequest) {
+TEST_F(ExtensionProtocolsTest, IncognitoRequest) {
   // Register an incognito extension protocol handler.
   SetProtocolHandler(true);
 
   struct TestCase {
     // Inputs.
     std::string name;
     bool incognito_split_mode;
     bool incognito_enabled;
 
     // Expected results.
@@ skipping 62 matching lines @@
   request->GetResponseHeaderByName(net::HttpRequestHeaders::kContentLength,
                                   &content_length);
   EXPECT_FALSE(content_length.empty());
   int length_value = 0;
   EXPECT_TRUE(base::StringToInt(content_length, &length_value));
   EXPECT_GT(length_value, 0);
 }
 
 // Tests getting a resource for a component extension works correctly, both when
 // the extension is enabled and when it is disabled.
-TEST_F(ExtensionProtocolTest, ComponentResourceRequest) {
+TEST_F(ExtensionProtocolsTest, ComponentResourceRequest) {
   // Register a non-incognito extension protocol handler.
   SetProtocolHandler(false);
 
   scoped_refptr<Extension> extension = CreateWebStoreExtension();
   extension_info_map_->AddExtension(extension.get(),
                                     base::Time::Now(),
                                     false,
                                     false);
 
   // First test it with the extension enabled.
@@ skipping 16 matching lines @@
             extension->GetResourceURL("webstore_icon_16.png"),
             net::DEFAULT_PRIORITY, &test_delegate_));
     StartRequest(request.get(), content::RESOURCE_TYPE_MEDIA);
     EXPECT_EQ(net::OK, test_delegate_.request_status());
     CheckForContentLengthHeader(request.get());
   }
 }
 
 // Tests that a URL request for resource from an extension returns a few
 // expected response headers.
-TEST_F(ExtensionProtocolTest, ResourceRequestResponseHeaders) {
+TEST_F(ExtensionProtocolsTest, ResourceRequestResponseHeaders) {
   // Register a non-incognito extension protocol handler.
   SetProtocolHandler(false);
 
   scoped_refptr<Extension> extension = CreateTestResponseHeaderExtension();
   extension_info_map_->AddExtension(extension.get(),
                                     base::Time::Now(),
                                     false,
                                     false);
 
   {
@@ skipping 17 matching lines @@
     // We set test.dat as web-accessible, so it should have a CORS header.
     std::string access_control;
     request->GetResponseHeaderByName("Access-Control-Allow-Origin",
                                     &access_control);
     EXPECT_EQ("*", access_control);
   }
 }
 
 // Tests that a URL request for main frame or subframe from an extension
 // succeeds, but subresources fail. See http://crbug.com/312269.
-TEST_F(ExtensionProtocolTest, AllowFrameRequests) {
+TEST_F(ExtensionProtocolsTest, AllowFrameRequests) {
   // Register a non-incognito extension protocol handler.
   SetProtocolHandler(false);
 
   scoped_refptr<Extension> extension = CreateTestExtension("foo", false);
   extension_info_map_->AddExtension(extension.get(),
                                     base::Time::Now(),
                                     false,
                                     false);
 
   // All MAIN_FRAME requests should succeed. SUB_FRAME requests that are not
@@ skipping 26 matching lines @@
   {
     std::unique_ptr<net::URLRequest> request(
         resource_context_.GetRequestContext()->CreateRequest(
             extension->GetResourceURL("test.dat"), net::DEFAULT_PRIORITY,
             &test_delegate_));
     StartRequest(request.get(), content::RESOURCE_TYPE_MEDIA);
     EXPECT_EQ(net::ERR_BLOCKED_BY_CLIENT, test_delegate_.request_status());
   }
 }
 
-
-TEST_F(ExtensionProtocolTest, MetadataFolder) {
+TEST_F(ExtensionProtocolsTest, MetadataFolder) {
   SetProtocolHandler(false);
 
   base::FilePath extension_dir = GetTestPath("metadata_folder");
   std::string error;
   scoped_refptr<Extension> extension =
       file_util::LoadExtension(extension_dir, Manifest::INTERNAL,
                                Extension::NO_FLAGS, &error);
   ASSERT_NE(extension.get(), nullptr) << "error: " << error;
 
   // Loading "/test.html" should succeed.
   EXPECT_EQ(net::OK, DoRequest(*extension, "test.html"));
 
   // Loading "/_metadata/verified_contents.json" should fail.
   base::FilePath relative_path =
       base::FilePath(kMetadataFolder).Append(kVerifiedContentsFilename);
   EXPECT_TRUE(base::PathExists(extension_dir.Append(relative_path)));
   EXPECT_EQ(net::ERR_FAILED,
             DoRequest(*extension, relative_path.AsUTF8Unsafe()));
 
   // Loading "/_metadata/a.txt" should also fail.
   relative_path = base::FilePath(kMetadataFolder).AppendASCII("a.txt");
   EXPECT_TRUE(base::PathExists(extension_dir.Append(relative_path)));
   EXPECT_EQ(net::ERR_FAILED,
             DoRequest(*extension, relative_path.AsUTF8Unsafe()));
 }
 
+// Tests that unreadable files and deleted files correctly go through
+// ContentVerifyJob.
+TEST_F(ExtensionProtocolsTest, VerificationSeenForFileAccessErrors) {
+  JobDelegate test_job_delegate;
+  ContentVerifyJob::SetDelegateForTests(&test_job_delegate);

[Devlin, 2017/03/24 02:01:44]

nit: clean up state; unset this at the end.

[lazyboy, 2017/03/24 18:27:47]

Done.

+  SetProtocolHandler(false);
+
+  const std::string kFooJs("foo.js");
+  // Create a temporary directory that a fake extension will live in and fill
+  // it with some test files.
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+  base::FilePath foo_js(FILE_PATH_LITERAL(kFooJs));
+  ASSERT_TRUE(AddFileToDirectory(temp_dir.GetPath(), foo_js, "hello world."))
+      << "Failed to write " << temp_dir.GetPath().value() << "/"
+      << foo_js.value();
+
+  ExtensionBuilder builder;
+  builder
+      .SetManifest(DictionaryBuilder()
+                       .Set("name", "Foo")
+                       .Set("version", "1.0")
+                       .Set("manifest_version", 2)
+                       .Set("update_url",
+                            "https://clients2.google.com/service/update2/crx")
+                       .Build())
+      .SetID(crx_file::id_util::GenerateId("whatever"))
+      .SetPath(temp_dir.GetPath())
+      .SetLocation(Manifest::INTERNAL);
+  scoped_refptr<Extension> extension(builder.Build());
+
+  EXPECT_TRUE(extension.get());
+  content_verifier_->OnExtensionLoaded(testing_profile_.get(), extension.get());
+  // Wait for PostTask to ContentVerifierIOData::AddData() to finish.
+  content::RunAllPendingInMessageLoop();
+
+  // Valid and redable foo.js.

[Devlin, 2017/03/24 02:01:44]

*readable

[lazyboy, 2017/03/24 18:27:47]

Done.

+  EXPECT_EQ(net::OK, DoRequest(*extension, kFooJs));
+  test_job_delegate.WaitForDoneReading(extension->id());
+
+  // chmod -r foo.js.
+  base::FilePath foo_path = temp_dir.GetPath().AppendASCII(kFooJs);
+  ASSERT_TRUE(base::MakeFileUnreadable(foo_path));
+  test_job_delegate.Reset();
+  EXPECT_EQ(net::ERR_ACCESS_DENIED, DoRequest(*extension, kFooJs));
+  test_job_delegate.WaitForDoneReading(extension->id());
+
+  // Delete foo.js.
+  ASSERT_TRUE(base::DieFileDie(foo_path, false));
+  test_job_delegate.Reset();
+  EXPECT_EQ(net::ERR_FILE_NOT_FOUND, DoRequest(*extension, kFooJs));
+  test_job_delegate.WaitForDoneReading(extension->id());
+}
+
 }  // namespace extensions