Fix file access on Chrome for ChromeOS on Linux

so that we can open files in the user's Downloads
directory.

Shouldn't affect actual ChromeOS or other platforms.

BUG=chromium-os:29447
TEST=Ran on Linux, opened files from Downloads folder.


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

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

10 files changed, 56 insertions, 80 deletions

diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index f3d813d..2096833 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -348,16 +348,6 @@ bool URLRequest::IsHandledURL(const GURL& url) {
   return IsHandledProtocol(url.scheme());
 }
 
-// static
-void URLRequest::AllowFileAccess() {
-  URLRequestJobManager::GetInstance()->set_enable_file_access(true);
-}
-
-// static
-bool URLRequest::IsFileAccessAllowed() {
-  return URLRequestJobManager::GetInstance()->enable_file_access();
-}
-
 void URLRequest::set_first_party_for_cookies(
     const GURL& first_party_for_cookies) {
   first_party_for_cookies_ = first_party_for_cookies;
@@ -862,7 +852,7 @@ void URLRequest::NotifySSLCertificateError(const SSLInfo& ssl_info,
 bool URLRequest::CanGetCookies(const CookieList& cookie_list) const {
   DCHECK(!(load_flags_ & LOAD_DO_NOT_SEND_COOKIES));
   if (context_ && context_->network_delegate()) {
-    return context_->network_delegate()->NotifyReadingCookies(this,
+    return context_->network_delegate()->CanGetCookies(*this,
                                                               cookie_list);
   }
   return g_default_can_use_cookies;
@@ -872,7 +862,7 @@ bool URLRequest::CanSetCookie(const std::string& cookie_line,
                               CookieOptions* options) const {
   DCHECK(!(load_flags_ & LOAD_DO_NOT_SAVE_COOKIES));
   if (context_ && context_->network_delegate()) {
-    return context_->network_delegate()->NotifySettingCookie(this,
+    return context_->network_delegate()->CanSetCookie(*this,
                                                              cookie_line,
                                                              options);
   }
diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h
index b00cf88..4e25d37 100644
--- a/net/url_request/url_request.h
+++ b/net/url_request/url_request.h
@@ -310,10 +310,6 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe),
   // Profile.
   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 url_chain_.front(); }
diff --git a/net/url_request/url_request_context_builder.cc b/net/url_request/url_request_context_builder.cc
index 5f8a32c..6547c2b 100644
--- a/net/url_request/url_request_context_builder.cc
+++ b/net/url_request/url_request_context_builder.cc
@@ -84,14 +84,19 @@ class BasicNetworkDelegate : public NetworkDelegate {
     return NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION;
   }
 
-  virtual bool CanGetCookies(const URLRequest* request,
-                             const CookieList& cookie_list) OVERRIDE {
+  virtual bool OnCanGetCookies(const URLRequest& request,
+                               const CookieList& cookie_list) OVERRIDE {
     return true;
   }
 
-  virtual bool CanSetCookie(const URLRequest* request,
-                            const std::string& cookie_line,
-                            CookieOptions* options) OVERRIDE {
+  virtual bool OnCanSetCookie(const URLRequest& request,
+                              const std::string& cookie_line,
+                              CookieOptions* options) OVERRIDE {
+    return true;
+  }
+
+  virtual bool OnCanAccessFile(const net::URLRequest& request,
+                               const FilePath& path) const OVERRIDE {
     return true;
   }
 
diff --git a/net/url_request/url_request_file_job.cc b/net/url_request/url_request_file_job.cc
index 3c8deae..0b64f74 100644
--- a/net/url_request/url_request_file_job.cc
+++ b/net/url_request/url_request_file_job.cc
@@ -36,6 +36,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_context.h"
 #include "net/url_request/url_request_error_job.h"
 #include "net/url_request/url_request_file_dir_job.h"
 
@@ -94,15 +95,12 @@ URLRequestFileJob::URLRequestFileJob(URLRequest* request,
 // static
 URLRequestJob* URLRequestFileJob::Factory(URLRequest* request,
                                           const std::string& scheme) {
-
   FilePath file_path;
   const bool is_file = FileURLToFilePath(request->url(), &file_path);
 
-#if defined(OS_CHROMEOS)
-  // Check file access.
-  if (AccessDisabled(file_path))
+  // Check file access permissions.
+  if (!IsFileAccessAllowed(*request, file_path))
     return new URLRequestErrorJob(request, ERR_ACCESS_DENIED);
-#endif
 
   // We need to decide whether to create URLRequestFileJob for file access or
   // URLRequestFileDirJob for directory access. To avoid accessing the
@@ -120,35 +118,6 @@ URLRequestJob* URLRequestFileJob::Factory(URLRequest* request,
   return new URLRequestFileJob(request, file_path);
 }
 
-#if defined(OS_CHROMEOS)
-static const char* const kLocalAccessWhiteList[] = {
-  "/home/chronos/user/Downloads",
-  "/home/chronos/user/log",
-  "/media",
-  "/opt/oem",
-  "/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  // OS_CHROMEOS
-
 void URLRequestFileJob::Start() {
   DCHECK(!async_resolver_);
   async_resolver_ = new AsyncResolver(this);
@@ -281,6 +250,18 @@ void URLRequestFileJob::SetExtraRequestHeaders(
   }
 }
 
+// static
+bool URLRequestFileJob::IsFileAccessAllowed(const URLRequest& request,
+                                            const FilePath& path) {
+  const URLRequestContext* context = request.context();
+  if (!context)
+    return false;
+  const NetworkDelegate* delegate = context->network_delegate();
+  if (delegate)
+    return delegate->CanAccessFile(request, path);
+  return false;
+}
+
 URLRequestFileJob::~URLRequestFileJob() {
   DCHECK(!async_resolver_);
 }
diff --git a/net/url_request/url_request_file_job.h b/net/url_request/url_request_file_job.h
index 0c250f8..b02008a 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) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -53,6 +53,12 @@ class NET_EXPORT URLRequestFileJob : public URLRequestJob {
   FilePath file_path_;
 
  private:
+  // Tests to see if access to |path| is allowed. If g_allow_file_access_ is
+  // true, then this will return true. If the NetworkDelegate associated with
+  // the |request| says it's OK, then this will also return true.
+  static bool IsFileAccessAllowed(const URLRequest& request,
+                                  const FilePath& path);
+
   // Callback after fetching file info on a background thread.
   void DidResolve(bool exists, const base::PlatformFileInfo& file_info);
 
diff --git a/net/url_request/url_request_job_manager.cc b/net/url_request/url_request_job_manager.cc
index f149fb2..70dd592 100644
--- a/net/url_request/url_request_job_manager.cc
+++ b/net/url_request/url_request_job_manager.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -122,7 +122,7 @@ URLRequestJob* URLRequestJobManager::CreateJob(
 
   // If we reached here, then it means that a registered protocol factory
   // wasn't interested in handling the URL.  That is fairly unexpected, and we
-  // don't know have a specific error to report here :-(
+  // don't have a specific error to report here :-(
   LOG(WARNING) << "Failed to map: " << request->url().spec();
   return new URLRequestErrorJob(request, ERR_FAILED);
 }
@@ -264,8 +264,7 @@ void URLRequestJobManager::UnregisterRequestInterceptor(
 
 URLRequestJobManager::URLRequestJobManager()
     : allowed_thread_(0),
-      allowed_thread_initialized_(false),
-      enable_file_access_(false) {
+      allowed_thread_initialized_(false) {
 }
 
 URLRequestJobManager::~URLRequestJobManager() {}
diff --git a/net/url_request/url_request_job_manager.h b/net/url_request/url_request_job_manager.h
index 469cc66..ce70272 100644
--- a/net/url_request/url_request_job_manager.h
+++ b/net/url_request/url_request_job_manager.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -65,9 +65,6 @@ 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::vector<URLRequest::Interceptor*> InterceptorList;
@@ -90,7 +87,7 @@ class URLRequestJobManager {
 #else
     // The previous version of this check used GetCurrentThread on Windows to
     // get thread handles to compare. Unfortunately, GetCurrentThread returns
-    // a constant psuedo-handle (0xFFFFFFFE), and therefore IsAllowedThread
+    // a constant pseudo-handle (0xFFFFFFFE), and therefore IsAllowedThread
     // always returned true. The above code that's turned off is the correct
     // code, but causes the tree to turn red because some caller isn't
     // respecting our thread requirements. We're turning off the check for now;
@@ -108,7 +105,6 @@ class URLRequestJobManager {
   mutable base::Lock lock_;
   FactoryMap factories_;
   InterceptorList interceptors_;
-  bool enable_file_access_;
 
   DISALLOW_COPY_AND_ASSIGN(URLRequestJobManager);
 };
diff --git a/net/url_request/url_request_test_util.cc b/net/url_request/url_request_test_util.cc
index 7e565e4..7a1bc7f 100644
--- a/net/url_request/url_request_test_util.cc
+++ b/net/url_request/url_request_test_util.cc
@@ -495,8 +495,8 @@ net::NetworkDelegate::AuthRequiredResponse TestNetworkDelegate::OnAuthRequired(
   return net::NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION;
 }
 
-bool TestNetworkDelegate::CanGetCookies(const net::URLRequest* request,
-                                        const net::CookieList& cookie_list) {
+bool TestNetworkDelegate::OnCanGetCookies(const net::URLRequest& request,
+                                          const net::CookieList& cookie_list) {
   bool allow = true;
   if (cookie_options_bit_mask_ & NO_GET_COOKIES)
     allow = false;
@@ -508,9 +508,9 @@ bool TestNetworkDelegate::CanGetCookies(const net::URLRequest* request,
   return allow;
 }
 
-bool TestNetworkDelegate::CanSetCookie(const net::URLRequest* request,
-                                       const std::string& cookie_line,
-                                       net::CookieOptions* options) {
+bool TestNetworkDelegate::OnCanSetCookie(const net::URLRequest& request,
+                                         const std::string& cookie_line,
+                                         net::CookieOptions* options) {
   bool allow = true;
   if (cookie_options_bit_mask_ & NO_SET_COOKIE)
     allow = false;
@@ -527,6 +527,11 @@ bool TestNetworkDelegate::CanSetCookie(const net::URLRequest* request,
   return allow;
 }
 
+bool TestNetworkDelegate::OnCanAccessFile(const net::URLRequest& request,
+                                          const FilePath& path) const {
+  return true;
+}
+
 // static
 std::string ScopedCustomUrlRequestTestHttpHost::value_("127.0.0.1");
 
diff --git a/net/url_request/url_request_test_util.h b/net/url_request/url_request_test_util.h
index e882349..8c065cd 100644
--- a/net/url_request/url_request_test_util.h
+++ b/net/url_request/url_request_test_util.h
@@ -240,11 +240,13 @@ class TestNetworkDelegate : public net::NetworkDelegate {
       const net::AuthChallengeInfo& auth_info,
       const AuthCallback& callback,
       net::AuthCredentials* credentials) OVERRIDE;
-  virtual bool CanGetCookies(const net::URLRequest* request,
-                             const net::CookieList& cookie_list) OVERRIDE;
-  virtual bool CanSetCookie(const net::URLRequest* request,
-                            const std::string& cookie_line,
-                            net::CookieOptions* options) OVERRIDE;
+  virtual bool OnCanGetCookies(const net::URLRequest& request,
+                               const net::CookieList& cookie_list) OVERRIDE;
+  virtual bool OnCanSetCookie(const net::URLRequest& request,
+                              const std::string& cookie_line,
+                              net::CookieOptions* options) OVERRIDE;
+  virtual bool OnCanAccessFile(const net::URLRequest& request,
+                               const FilePath& path) const OVERRIDE;
 
   void InitRequestStatesIfNew(int request_id);
 
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index 204f223..25b2b0c 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -411,10 +411,6 @@ class URLRequestTest : public PlatformTest {
     default_context_->Init();
   }
 
-  static void SetUpTestCase() {
-    URLRequest::AllowFileAccess();
-  }
-
   // Adds the TestJobInterceptor to the default context.
   TestJobInterceptor* AddTestInterceptor() {
     TestJobInterceptor* interceptor = new TestJobInterceptor();