Restrict file protocol on chromeos to certain whitelisted directories. Disable this for tests.

BUG=chromium-os:3412
TEST=Access file: directories on chromeos. browser, ui, interactive ui and unit tests should continue to pass.

Review URL: http://codereview.chromium.org/4160003

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

13 files changed, 93 insertions, 6 deletions

diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index 90f7ce3..41c2322 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -97,6 +97,7 @@
 #include "net/socket/tcp_client_socket.h"
 #include "net/spdy/spdy_session.h"
 #include "net/spdy/spdy_session_pool.h"
+#include "net/url_request/url_request.h"
 
 #if defined(USE_LINUX_BREAKPAD)
 #include "base/linux_util.h"
@@ -1215,6 +1216,11 @@ int BrowserMain(const MainFunctionParams& parameters) {
   // notification it needs to track the logged in user.
   g_browser_process->profile_manager()->GetDefaultProfile();
 
+  // Allow access to file:// on ChromeOS for tests.
+  if (parsed_command_line.HasSwitch(switches::kAllowFileAccess)) {
+    URLRequest::AllowFileAccess();
+  }
+
   // There are two use cases for kLoginUser:
   //   1) if passed in tandem with kLoginPassword, to drive a "StubLogin"
   //   2) if passed alone, to signal that the indicated user has already
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 8f115c4..1466d96 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -22,6 +22,10 @@ const char kActivateOnLaunch[]              = "activate-on-launch";
 // override for developers who need the old behavior for testing.
 const char kAllowFileAccessFromFiles[]      = "allow-file-access-from-files";
 
+// On ChromeOS, file:// access is disabled except for certain whitelisted
+// directories. This switch re-enables file:// for testing.
+const char kAllowFileAccess[]               = "allow-file-access";
+
 // Disable checking of the renegotiation extension and any future checks over
 // and above what a "traditional" SSL stack might do. This has been requested
 // in order to support some web development tools that intercept SSL
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index f6e6f32..901abbf 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -22,6 +22,7 @@ namespace switches {
 // alongside the definition of their values in the .cc file.
 extern const char kActivateOnLaunch[];
 extern const char kAllowFileAccessFromFiles[];
+extern const char kAllowFileAccess[];
 extern const char kAllowSSLMITMProxies[];
 extern const char kAllowSandboxDebugging[];
 extern const char kAllowScriptingGallery[];
diff --git a/chrome/test/out_of_proc_test_runner.cc b/chrome/test/out_of_proc_test_runner.cc
index a14a38a..43a7525 100644
--- a/chrome/test/out_of_proc_test_runner.cc
+++ b/chrome/test/out_of_proc_test_runner.cc
@@ -273,6 +273,9 @@ int RunTest(const std::string& test_name) {
   }
   new_cmd_line.AppendSwitchPath(switches::kUserDataDir, temp_dir.path());
 
+  // file:// access for ChromeOS.
+  new_cmd_line.AppendSwitch(switches::kAllowFileAccess);
+
   base::ProcessHandle process_handle;
 #if defined(OS_POSIX)
   const char* browser_wrapper = getenv("BROWSER_WRAPPER");
diff --git a/chrome/test/ui/ui_test.cc b/chrome/test/ui/ui_test.cc
index 29075b3..34cd0d2 100644
--- a/chrome/test/ui/ui_test.cc
+++ b/chrome/test/ui/ui_test.cc
@@ -751,6 +751,9 @@ bool UITestBase::LaunchBrowserHelper(const CommandLine& arguments,
   // Disable TabCloseableStateWatcher for tests.
   command_line.AppendSwitch(switches::kDisableTabCloseableStateWatcher);
 
+  // Allow file:// access on ChromeOS.
+  command_line.AppendSwitch(switches::kAllowFileAccess);
+
   test_launcher_utils::PrepareBrowserCommandLineForTests(&command_line);
 
   DebugFlags::ProcessDebugFlags(
diff --git a/net/proxy/proxy_script_fetcher_impl_unittest.cc b/net/proxy/proxy_script_fetcher_impl_unittest.cc
index 2642f4d..4734997 100644
--- a/net/proxy/proxy_script_fetcher_impl_unittest.cc
+++ b/net/proxy/proxy_script_fetcher_impl_unittest.cc
@@ -4,6 +4,8 @@
 
 #include "net/proxy/proxy_script_fetcher_impl.h"
 
+#include <string>
+
 #include "base/file_path.h"
 #include "base/compiler_specific.h"
 #include "base/path_service.h"
@@ -73,6 +75,10 @@ class ProxyScriptFetcherImplTest : public PlatformTest {
       : test_server_(net::TestServer::TYPE_HTTP, FilePath(kDocRoot)) {
   }
 
+  static void SetUpTestCase() {
+    URLRequest::AllowFileAccess();
+  }
+
  protected:
   net::TestServer test_server_;
 };
diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index c4f20c4..7898848 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -274,6 +274,17 @@ bool URLRequest::IsHandledURL(const GURL& url) {
   return IsHandledProtocol(url.scheme());
 }
 
+// static
+void URLRequest::AllowFileAccess() {
+  GetJobManager()->set_enable_file_access(true);
+}
+
+// static
+bool URLRequest::IsFileAccessAllowed() {
+  return GetJobManager()->enable_file_access();
+}
+
+
 void URLRequest::set_first_party_for_cookies(
     const GURL& first_party_for_cookies) {
   first_party_for_cookies_ = first_party_for_cookies;
diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h
index f573111..01f5984 100644
--- a/net/url_request/url_request.h
+++ b/net/url_request/url_request.h
@@ -251,6 +251,10 @@ class URLRequest : public NonThreadSafe {
   // to handle those.
   static bool IsHandledURL(const GURL& url);
 
+  // Allow access to file:// on ChromeOS for tests.
+  static void AllowFileAccess();
+  static bool IsFileAccessAllowed();
+
   // The original url is the url used to initialize the request, and it may
   // differ from the url if the request was redirected.
   const GURL& original_url() const { return original_url_; }
diff --git a/net/url_request/url_request_file_job.cc b/net/url_request/url_request_file_job.cc
index fff85c3..526dabf 100644
--- a/net/url_request/url_request_file_job.cc
+++ b/net/url_request/url_request_file_job.cc
@@ -33,6 +33,7 @@
 #include "net/base/net_util.h"
 #include "net/http/http_util.h"
 #include "net/url_request/url_request.h"
+#include "net/url_request/url_request_error_job.h"
 #include "net/url_request/url_request_file_dir_job.h"
 
 #if defined(OS_WIN)
@@ -40,8 +41,8 @@
 #endif
 
 #if defined(OS_WIN)
-class URLRequestFileJob::AsyncResolver :
-    public base::RefCountedThreadSafe<URLRequestFileJob::AsyncResolver> {
+class URLRequestFileJob::AsyncResolver
+    : public base::RefCountedThreadSafe<URLRequestFileJob::AsyncResolver> {
  public:
   explicit AsyncResolver(URLRequestFileJob* owner)
       : owner_(owner), owner_loop_(MessageLoop::current()) {
@@ -84,7 +85,15 @@ class URLRequestFileJob::AsyncResolver :
 // static
 URLRequestJob* URLRequestFileJob::Factory(
     URLRequest* request, const std::string& scheme) {
+
   FilePath file_path;
+  const bool is_file = net::FileURLToFilePath(request->url(), &file_path);
+
+#if defined(OS_CHROMEOS)
+  // Check file access.
+  if (AccessDisabled(file_path))
+    return new URLRequestErrorJob(request, net::ERR_ACCESS_DENIED);
+#endif
 
   // We need to decide whether to create URLRequestFileJob for file access or
   // URLRequestFileDirJob for directory access. To avoid accessing the
@@ -92,7 +101,7 @@ URLRequestJob* URLRequestFileJob::Factory(
   // The code in the URLRequestFileJob::Start() method discovers that a path,
   // which doesn't end with a slash, should really be treated as a directory,
   // and it then redirects to the URLRequestFileDirJob.
-  if (net::FileURLToFilePath(request->url(), &file_path) &&
+  if (is_file &&
       file_util::EndsWithSeparator(file_path) &&
       file_path.IsAbsolute())
     return new URLRequestFileDirJob(request, file_path);
@@ -346,3 +355,31 @@ bool URLRequestFileJob::IsRedirectResponse(GURL* location,
   return false;
 #endif
 }
+
+#if defined(OS_CHROMEOS)
+static const char* const kLocalAccessWhiteList[] = {
+  "/home/chronos/user/Downloads",
+  "/mnt/partner_partition",
+  "/usr/share/chromeos-assets",
+  "/tmp",
+  "/var/log",
+};
+
+// static
+bool URLRequestFileJob::AccessDisabled(const FilePath& file_path) {
+  if (URLRequest::IsFileAccessAllowed()) {  // for tests.
+    return false;
+  }
+
+  for (size_t i = 0; i < arraysize(kLocalAccessWhiteList); ++i) {
+    const FilePath white_listed_path(kLocalAccessWhiteList[i]);
+    // FilePath::operator== should probably handle trailing seperators.
+    if (white_listed_path == file_path.StripTrailingSeparators() ||
+        white_listed_path.IsParent(file_path)) {
+      return false;
+    }
+  }
+  return true;
+}
+#endif
+
diff --git a/net/url_request/url_request_file_job.h b/net/url_request/url_request_file_job.h
index c5e1d2f..4512c1c 100644
--- a/net/url_request/url_request_file_job.h
+++ b/net/url_request/url_request_file_job.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2006-2010 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.
 
@@ -45,6 +45,9 @@ class URLRequestFileJob : public URLRequestJob {
  private:
   void DidResolve(bool exists, const base::PlatformFileInfo& file_info);
   void DidRead(int result);
+#if defined(OS_CHROMEOS)
+  static bool AccessDisabled(const FilePath& file_path);
+#endif
 
   net::CompletionCallbackImpl<URLRequestFileJob> io_callback_;
   net::FileStream stream_;
diff --git a/net/url_request/url_request_job_manager.cc b/net/url_request/url_request_job_manager.cc
index edc9f75..e8a4820 100644
--- a/net/url_request/url_request_job_manager.cc
+++ b/net/url_request/url_request_job_manager.cc
@@ -36,7 +36,7 @@ static const SchemeToFactory kBuiltinFactories[] = {
   { "data", URLRequestDataJob::Factory },
 };
 
-URLRequestJobManager::URLRequestJobManager() {
+URLRequestJobManager::URLRequestJobManager() : enable_file_access_(false) {
 #ifndef NDEBUG
   allowed_thread_ = 0;
   allowed_thread_initialized_ = false;
diff --git a/net/url_request/url_request_job_manager.h b/net/url_request/url_request_job_manager.h
index c93df76..f459f40 100644
--- a/net/url_request/url_request_job_manager.h
+++ b/net/url_request/url_request_job_manager.h
@@ -7,6 +7,7 @@
 #pragma once
 
 #include <map>
+#include <string>
 #include <vector>
 
 #include "base/lock.h"
@@ -60,13 +61,17 @@ class URLRequestJobManager {
   void RegisterRequestInterceptor(URLRequest::Interceptor* interceptor);
   void UnregisterRequestInterceptor(URLRequest::Interceptor* interceptor);
 
+  void set_enable_file_access(bool enable) { enable_file_access_ = enable; }
+  bool enable_file_access() const { return enable_file_access_; }
+
  private:
-  typedef std::map<std::string,URLRequest::ProtocolFactory*> FactoryMap;
+  typedef std::map<std::string, URLRequest::ProtocolFactory*> FactoryMap;
   typedef std::vector<URLRequest::Interceptor*> InterceptorList;
 
   mutable Lock lock_;
   FactoryMap factories_;
   InterceptorList interceptors_;
+  bool enable_file_access_;
 
 #ifndef NDEBUG
   // We use this to assert that CreateJob and the registration functions all
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index d0f488d..2c44a1c 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -117,6 +117,10 @@ void CheckSSLInfo(const net::SSLInfo& ssl_info) {
 
 // Inherit PlatformTest since we require the autorelease pool on Mac OS X.f
 class URLRequestTest : public PlatformTest {
+ public:
+  static void SetUpTestCase() {
+    URLRequest::AllowFileAccess();
+  }
 };
 
 class URLRequestTestHTTP : public URLRequestTest {