Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4956)

Unified Diff: chrome/browser/extensions/extension_protocols_unittest.cc

Issue 2771953003: Fix content verification code for undreadable and deleted files. (Closed)
Patch Set: address comments change DCHECK Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/extensions/content_verifier_browsertest.cc ('k') | extensions/browser/BUILD.gn » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/extensions/extension_protocols_unittest.cc
diff --git a/chrome/browser/extensions/extension_protocols_unittest.cc b/chrome/browser/extensions/extension_protocols_unittest.cc
index e3da5f8a4a3d0642b7ef4b413e386b2063fc8c56..c762e8d5599b815cced82bf59933402c5a3a1010 100644
--- a/chrome/browser/extensions/extension_protocols_unittest.cc
+++ b/chrome/browser/extensions/extension_protocols_unittest.cc
@@ -7,23 +7,31 @@
#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"
@@ -43,6 +51,16 @@ base::FilePath GetTestPath(const std::string& name) {
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);
+ int result = base::WriteFile(full_path, content.data(), content.size());
+ return static_cast<size_t>(result) == content.size();
+}
+
scoped_refptr<Extension> CreateTestExtension(const std::string& name,
bool incognito_split_mode) {
base::DictionaryValue manifest;
@@ -98,30 +116,96 @@ scoped_refptr<Extension> CreateTestResponseHeaderExtension() {
return extension;
}
+// A ContentVerifyJob::TestDelegate that observes DoneReading().
+class JobDelegate : public ContentVerifyJob::TestDelegate {
+ public:
+ explicit JobDelegate(const std::string& expected_contents)
+ : expected_contents_(expected_contents), run_loop_(new base::RunLoop()) {
+ ContentVerifyJob::SetDelegateForTests(this);
+ }
+ ~JobDelegate() override { ContentVerifyJob::SetDelegateForTests(nullptr); }
+
+ ContentVerifyJob::FailureReason BytesRead(const ExtensionId& id,
+ int count,
+ const char* data) override {
+ read_contents_.append(data, count);
+ return ContentVerifyJob::NONE;
+ }
+
+ ContentVerifyJob::FailureReason DoneReading(const ExtensionId& id) override {
+ seen_done_reading_extension_ids_.insert(id);
+ if (waiting_for_extension_id_ == id)
+ run_loop_->Quit();
+
+ if (!base::StartsWith(expected_contents_, read_contents_,
+ base::CompareCase::SENSITIVE)) {
+ ADD_FAILURE() << "Unexpected read, expected: " << expected_contents_
+ << ", but found: " << read_contents_;
+ }
+ 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_ = id;
+ run_loop_->Run();
+ }
+
+ void Reset() {
+ read_contents_.clear();
+ waiting_for_extension_id_.reset();
+ seen_done_reading_extension_ids_.clear();
+ run_loop_ = base::MakeUnique<base::RunLoop>();
+ }
+
+ private:
+ std::string expected_contents_;
+ std::string read_contents_;
+ std::set<ExtensionId> seen_done_reading_extension_ids_;
+ base::Optional<ExtensionId> waiting_for_extension_id_;
+ std::unique_ptr<base::RunLoop> run_loop_;
+
+ 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_ = TestingProfile::Builder().Build();
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_);
+ content_verifier_->Shutdown();
}
void SetProtocolHandler(bool is_incognito) {
@@ -175,13 +259,15 @@ class ExtensionProtocolTest : public testing::Test {
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);
@@ -264,7 +350,7 @@ void CheckForContentLengthHeader(net::URLRequest* request) {
// 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);
@@ -301,7 +387,7 @@ TEST_F(ExtensionProtocolTest, ComponentResourceRequest) {
// 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);
@@ -339,7 +425,7 @@ TEST_F(ExtensionProtocolTest, ResourceRequestResponseHeaders) {
// 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);
@@ -386,8 +472,7 @@ TEST_F(ExtensionProtocolTest, AllowFrameRequests) {
}
}
-
-TEST_F(ExtensionProtocolTest, MetadataFolder) {
+TEST_F(ExtensionProtocolsTest, MetadataFolder) {
SetProtocolHandler(false);
base::FilePath extension_dir = GetTestPath("metadata_folder");
@@ -414,4 +499,58 @@ TEST_F(ExtensionProtocolTest, MetadataFolder) {
DoRequest(*extension, relative_path.AsUTF8Unsafe()));
}
+// Tests that unreadable files and deleted files correctly go through
+// ContentVerifyJob.
+TEST_F(ExtensionProtocolsTest, VerificationSeenForFileAccessErrors) {
+ const char kFooJsContents[] = "hello world.";
+ JobDelegate test_job_delegate(kFooJsContents);
+ 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("foo.js"));
+ ASSERT_TRUE(AddFileToDirectory(temp_dir.GetPath(), foo_js, kFooJsContents))
+ << "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());
+
+ ASSERT_TRUE(extension.get());
+ content_verifier_->OnExtensionLoaded(testing_profile_.get(), extension.get());
+ // Wait for PostTask to ContentVerifierIOData::AddData() to finish.
+ content::RunAllPendingInMessageLoop();
+
+ // Valid and readable foo.js.
+ 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
« no previous file with comments | « chrome/browser/extensions/content_verifier_browsertest.cc ('k') | extensions/browser/BUILD.gn » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698