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

Unified Diff: net/url_request/url_request_unittest.cc

Issue 2786583002: chromeos: Check both original and absolute paths for file: scheme (Closed)
Patch Set: remove dcheck Created 3 years, 7 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 | « net/url_request/url_request_test_util.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/url_request/url_request_unittest.cc
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index 2d158a0492764e8196d54bd59c0bd6f0be81e459..5d299633657a17d02769a9deec133ac989639e33 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -835,6 +835,41 @@ class URLRequestTest : public PlatformTest {
TestURLRequestContext default_context_;
};
+// This NetworkDelegate is picky about what files are accessible. Only
+// whitelisted files are allowed.
+class CookieBlockingNetworkDelegate : public TestNetworkDelegate {
+ public:
+ CookieBlockingNetworkDelegate(){};
+
+ // Adds |directory| to the access white list.
+ void AddToWhitelist(const base::FilePath& directory) {
+ whitelist_.insert(directory);
+ }
+
+ private:
+ // Returns true if |path| matches the white list.
+ bool OnCanAccessFileInternal(const base::FilePath& path) const {
+ for (const auto& directory : whitelist_) {
+ if (directory == path || directory.IsParent(path))
+ return true;
+ }
+ return false;
+ }
+
+ // Returns true only if both |original_path| and |absolute_path| match the
+ // white list.
+ bool OnCanAccessFile(const URLRequest& request,
+ const base::FilePath& original_path,
+ const base::FilePath& absolute_path) const override {
+ return (OnCanAccessFileInternal(original_path) &&
+ OnCanAccessFileInternal(absolute_path));
+ }
+
+ std::set<base::FilePath> whitelist_;
+
+ DISALLOW_COPY_AND_ASSIGN(CookieBlockingNetworkDelegate);
+};
+
TEST_F(URLRequestTest, AboutBlankTest) {
TestDelegate d;
{
@@ -1083,39 +1118,176 @@ TEST_F(URLRequestTest, FileTestMultipleRanges) {
TEST_F(URLRequestTest, AllowFileURLs) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ // Get an absolute path since |temp_dir| can contain a symbolic link. As of
+ // now, Mac and Android bots return a path with a symbolic link.
+ base::FilePath absolute_temp_dir =
+ base::MakeAbsoluteFilePath(temp_dir.GetPath());
+
base::FilePath test_file;
- ASSERT_TRUE(base::CreateTemporaryFileInDir(temp_dir.GetPath(), &test_file));
+ ASSERT_TRUE(base::CreateTemporaryFileInDir(absolute_temp_dir, &test_file));
+ // The directory part of the path returned from CreateTemporaryFileInDir()
+ // can be slightly different from |absolute_temp_dir| on Windows.
+ // Example: C:\\Users\\CHROME~2 -> C:\\Users\\chrome-bot
+ // Hence the test should use the directory name of |test_file|, rather than
+ // |absolute_temp_dir|, for whitelisting.
+ base::FilePath real_temp_dir = test_file.DirName();
std::string test_data("monkey");
base::WriteFile(test_file, test_data.data(), test_data.size());
GURL test_file_url = FilePathToFileURL(test_file);
-
{
TestDelegate d;
- TestNetworkDelegate network_delegate;
- network_delegate.set_can_access_files(true);
+ CookieBlockingNetworkDelegate network_delegate;
+ network_delegate.AddToWhitelist(real_temp_dir);
default_context_.set_network_delegate(&network_delegate);
std::unique_ptr<URLRequest> r(default_context_.CreateRequest(
test_file_url, DEFAULT_PRIORITY, &d, TRAFFIC_ANNOTATION_FOR_TESTS));
r->Start();
base::RunLoop().Run();
+ // This should be allowed as the file path is whitelisted.
EXPECT_FALSE(d.request_failed());
EXPECT_EQ(test_data, d.data_received());
}
{
TestDelegate d;
- TestNetworkDelegate network_delegate;
- network_delegate.set_can_access_files(false);
+ CookieBlockingNetworkDelegate network_delegate;
default_context_.set_network_delegate(&network_delegate);
std::unique_ptr<URLRequest> r(default_context_.CreateRequest(
test_file_url, DEFAULT_PRIORITY, &d, TRAFFIC_ANNOTATION_FOR_TESTS));
r->Start();
base::RunLoop().Run();
+ // This should be rejected as the file path is not whitelisted.
EXPECT_TRUE(d.request_failed());
EXPECT_EQ("", d.data_received());
+ EXPECT_EQ(ERR_ACCESS_DENIED, d.request_status());
}
}
+#if defined(OS_POSIX) // Bacause of symbolic links.
+
+TEST_F(URLRequestTest, SymlinksToFiles) {
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ // Get an absolute path since temp_dir can contain a symbolic link.
+ base::FilePath absolute_temp_dir =
+ base::MakeAbsoluteFilePath(temp_dir.GetPath());
+
+ // Create a good directory (will be whitelisted) and a good file.
+ base::FilePath good_dir = absolute_temp_dir.AppendASCII("good");
+ ASSERT_TRUE(base::CreateDirectory(good_dir));
+ base::FilePath good_file;
+ ASSERT_TRUE(base::CreateTemporaryFileInDir(good_dir, &good_file));
+ std::string good_data("good");
+ base::WriteFile(good_file, good_data.data(), good_data.size());
+ // See the comment in AllowFileURLs() for why this is done.
+ base::FilePath real_good_dir = good_file.DirName();
+
+ // Create a bad directory (will not be whitelisted) and a bad file.
+ base::FilePath bad_dir = absolute_temp_dir.AppendASCII("bad");
+ ASSERT_TRUE(base::CreateDirectory(bad_dir));
+ base::FilePath bad_file;
+ ASSERT_TRUE(base::CreateTemporaryFileInDir(bad_dir, &bad_file));
+ std::string bad_data("bad");
+ base::WriteFile(bad_file, bad_data.data(), bad_data.size());
+
+ // This symlink will point to the good file. Access to the symlink will be
+ // allowed as both the symlink and the destination file are in the same
+ // good directory.
+ base::FilePath good_symlink = good_dir.AppendASCII("good_symlink");
+ ASSERT_TRUE(base::CreateSymbolicLink(good_file, good_symlink));
+ GURL good_file_url = FilePathToFileURL(good_symlink);
+ // This symlink will point to the bad file. Even though the symlink is in
+ // the good directory, access to the symlink will be rejected since it
+ // points to the bad file.
+ base::FilePath bad_symlink = good_dir.AppendASCII("bad_symlink");
+ ASSERT_TRUE(base::CreateSymbolicLink(bad_file, bad_symlink));
+ GURL bad_file_url = FilePathToFileURL(bad_symlink);
+
+ CookieBlockingNetworkDelegate network_delegate;
+ network_delegate.AddToWhitelist(real_good_dir);
+ {
+ TestDelegate d;
+ default_context_.set_network_delegate(&network_delegate);
+ std::unique_ptr<URLRequest> r(
+ default_context_.CreateRequest(good_file_url, DEFAULT_PRIORITY, &d));
+ r->Start();
+ base::RunLoop().Run();
+ // good_file_url should be allowed.
+ EXPECT_FALSE(d.request_failed());
+ EXPECT_EQ(good_data, d.data_received());
+ }
+
+ {
+ TestDelegate d;
+ default_context_.set_network_delegate(&network_delegate);
+ std::unique_ptr<URLRequest> r(
+ default_context_.CreateRequest(bad_file_url, DEFAULT_PRIORITY, &d));
+ r->Start();
+ base::RunLoop().Run();
+ // bad_file_url should be rejected.
+ EXPECT_TRUE(d.request_failed());
+ EXPECT_EQ("", d.data_received());
+ EXPECT_EQ(ERR_ACCESS_DENIED, d.request_status());
+ }
+}
+
+TEST_F(URLRequestTest, SymlinksToDirs) {
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ // Get an absolute path since temp_dir can contain a symbolic link.
+ base::FilePath absolute_temp_dir =
+ base::MakeAbsoluteFilePath(temp_dir.GetPath());
+
+ // Create a good directory (will be whitelisted).
+ base::FilePath good_dir = absolute_temp_dir.AppendASCII("good");
+ ASSERT_TRUE(base::CreateDirectory(good_dir));
+
+ // Create a bad directory (will not be whitelisted).
+ base::FilePath bad_dir = absolute_temp_dir.AppendASCII("bad");
+ ASSERT_TRUE(base::CreateDirectory(bad_dir));
+
+ // This symlink will point to the good directory. Access to the symlink
+ // will be allowed as the symlink is in the good dir that'll be white
+ // listed.
+ base::FilePath good_symlink = good_dir.AppendASCII("good_symlink");
+ ASSERT_TRUE(base::CreateSymbolicLink(good_dir, good_symlink));
+ GURL good_file_url = FilePathToFileURL(good_symlink);
+ // This symlink will point to the bad directory. Even though the symlink is
+ // in the good directory, access to the symlink will be rejected since it
+ // points to the bad directory.
+ base::FilePath bad_symlink = good_dir.AppendASCII("bad_symlink");
+ ASSERT_TRUE(base::CreateSymbolicLink(bad_dir, bad_symlink));
+ GURL bad_file_url = FilePathToFileURL(bad_symlink);
+
+ CookieBlockingNetworkDelegate network_delegate;
+ network_delegate.AddToWhitelist(good_dir);
+ {
+ TestDelegate d;
+ default_context_.set_network_delegate(&network_delegate);
+ std::unique_ptr<URLRequest> r(
+ default_context_.CreateRequest(good_file_url, DEFAULT_PRIORITY, &d));
+ r->Start();
+ base::RunLoop().Run();
+ // good_file_url should be allowed.
+ EXPECT_FALSE(d.request_failed());
+ ASSERT_NE(d.data_received().find("good_symlink"), std::string::npos);
+ }
+
+ {
+ TestDelegate d;
+ default_context_.set_network_delegate(&network_delegate);
+ std::unique_ptr<URLRequest> r(
+ default_context_.CreateRequest(bad_file_url, DEFAULT_PRIORITY, &d));
+ r->Start();
+ base::RunLoop().Run();
+ // bad_file_url should be rejected.
+ EXPECT_TRUE(d.request_failed());
+ EXPECT_EQ("", d.data_received());
+ EXPECT_EQ(ERR_ACCESS_DENIED, d.request_status());
+ }
+}
+
+#endif // defined(OS_POSIX)
TEST_F(URLRequestTest, FileDirCancelTest) {
// Put in mock resource provider.
@@ -1167,9 +1339,8 @@ TEST_F(URLRequestTest, FileDirOutputSanity) {
EXPECT_GT(info.size, 0);
std::string sentinel_output = GetDirectoryListingEntry(
base::string16(sentinel_name, sentinel_name + strlen(sentinel_name)),
- std::string(sentinel_name),
- false /* is_dir */,
- info.size,
+ std::string(sentinel_name), false /* is_dir */, info.size,
+
info.last_modified);
ASSERT_LT(0, d.bytes_received());
« no previous file with comments | « net/url_request/url_request_test_util.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698