Add a SafeSearch preference, policy and implementation.

chrome/browser/net/chrome_network_delegate.cc

 // 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.
 
 #include "chrome/browser/net/chrome_network_delegate.h"
 
+#include <string.h>  // For strlen

[Joao da Silva, 2012/10/16 16:26:46]

nit: newline

[Sergiu, 2012/10/17 14:48:58]

Done.

 #include "base/logging.h"
 #include "base/base_paths.h"
 #include "base/path_service.h"
 #include "chrome/browser/api/prefs/pref_member.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/content_settings/cookie_settings.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/custom_handlers/protocol_handler_registry.h"
 #include "chrome/browser/extensions/api/proxy/proxy_api.h"
 #include "chrome/browser/extensions/api/web_request/web_request_api.h"
 #include "chrome/browser/extensions/event_router_forwarder.h"
 #include "chrome/browser/extensions/extension_info_map.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
+#include "chrome/browser/google/google_util.h"
 #include "chrome/browser/net/load_time_stats.h"
 #include "chrome/browser/performance_monitor/performance_monitor.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/task_manager/task_manager.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/resource_request_info.h"
@@ skipping 109 matching lines @@
 
 ChromeNetworkDelegate::ChromeNetworkDelegate(
     extensions::EventRouterForwarder* event_router,
     ExtensionInfoMap* extension_info_map,
     const policy::URLBlacklistManager* url_blacklist_manager,
     const ManagedModeURLFilter* managed_mode_url_filter,
     void* profile,
     CookieSettings* cookie_settings,
     BooleanPrefMember* enable_referrers,
     BooleanPrefMember* enable_do_not_track,
+    BooleanPrefMember* enable_safesearch,
     chrome_browser_net::LoadTimeStats* load_time_stats)
     : event_router_(event_router),
       profile_(profile),
       cookie_settings_(cookie_settings),
       extension_info_map_(extension_info_map),
       enable_referrers_(enable_referrers),
       enable_do_not_track_(enable_do_not_track),
+      enable_safesearch_(enable_safesearch),
       url_blacklist_manager_(url_blacklist_manager),
       managed_mode_url_filter_(managed_mode_url_filter),
       load_time_stats_(load_time_stats) {
   DCHECK(event_router);
   DCHECK(enable_referrers);
   DCHECK(!profile || cookie_settings);
 }
 
 ChromeNetworkDelegate::~ChromeNetworkDelegate() {}
 
 // static
 void ChromeNetworkDelegate::NeverThrottleRequests() {
   g_never_throttle_requests_ = true;
 }
 
 // static
 void ChromeNetworkDelegate::InitializePrefsOnUIThread(
     BooleanPrefMember* enable_referrers,
     BooleanPrefMember* enable_do_not_track,
+    BooleanPrefMember* enable_safesearch,
     PrefService* pref_service) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   enable_referrers->Init(prefs::kEnableReferrers, pref_service, NULL);
   enable_referrers->MoveToThread(BrowserThread::IO);
   if (enable_do_not_track) {
     enable_do_not_track->Init(prefs::kEnableDoNotTrack, pref_service, NULL);
     enable_do_not_track->MoveToThread(BrowserThread::IO);
   }
+  if (enable_safesearch) {
+    enable_safesearch->Init(prefs::kSafeSearchEnabled, pref_service, NULL);
+    enable_safesearch->MoveToThread(BrowserThread::IO);
+  }
 }
 
 // static
 void ChromeNetworkDelegate::AllowAccessToAllFiles() {
   g_allow_file_access_ = true;
 }
 
 int ChromeNetworkDelegate::OnBeforeURLRequest(
     net::URLRequest* request,
     const net::CompletionCallback& callback,
@@ skipping 20 matching lines @@
     return net::ERR_NETWORK_ACCESS_DENIED;
   }
 #endif
 
   ForwardRequestStatus(REQUEST_STARTED, request, profile_);
 
   if (!enable_referrers_->GetValue())
     request->set_referrer(std::string());
   if (enable_do_not_track_ && enable_do_not_track_->GetValue())
     request->SetExtraRequestHeaderByName(kDNTHeader, "1", true /* override */);
+
+  // Enforce SafeSearch.
+  if (enable_safesearch_ && enable_safesearch_->GetValue()) {

[battre, 2012/10/16 16:05:49]

how about moving lines 235-264 into a function? 

[Bernhard Bauer, 2012/10/16 16:37:33]

Yesplz. That would make unit-testing it a lot easier.

[Sergiu, 2012/10/17 14:48:58]

Done.

[Sergiu, 2012/10/17 14:48:58]

Done.

+    const bool is_search_url = google_util::IsGoogleSearchUrl(
+        request->url().spec());
+    const bool is_homepage_url = google_util::IsGoogleHomePageUrl(
+        request->url().spec());

[Bernhard Bauer, 2012/10/16 16:37:33]

Can we add a sanity-DCHECK that |is_search_url| and |is_homepage_url| are
mutually exclusive?

[Sergiu, 2012/10/17 14:48:58]

Well, they're not completely, that's the problem. For example:
http://google.com -> !is_search_url && is_homepage_url
http://google.com?q=test -> is_search_url && is_homepage_url
http://google.com/search?q=test -> is_search_url && !is_homepage_url
http://cnn.com -> !is_search_url && !is_homepage_url

+    if (mod_by_managed_mode_.find(request) == mod_by_managed_mode_.end() &&

[Bernhard Bauer, 2012/10/16 16:37:33]

Can OnBeforeURLRequest() be called multiple times for the same request?

[Sergiu, 2012/10/17 14:48:58]

No longer relevant since we changed the way we do it

+        (is_search_url || is_homepage_url)) {
+      std::string query = request->url().query();
+      std::string append_param;
+
+      if (is_homepage_url && query.length() == 0) {

[Bernhard Bauer, 2012/10/16 16:37:33]

`query.length() == 0` can be written a bit shorter as `query.empty()`.

[Sergiu, 2012/10/17 14:48:58]

Done.

+        append_param = chrome::kSSearchParamsSingle;
+      } else {
+        append_param = chrome::kSSearchParamsMultiple;
+      }
+      // Make sure that the last our parameters are either the only ones

[battre, 2012/10/16 16:05:49]

nit: grammar in sentence above "the last our"

[Sergiu, 2012/10/17 14:48:58]

Done.

+      // or have a & before them so that we don't accept &bogussafe=active.
+      if ((query.rfind(chrome::kSSearchParamsSingle) != std::string::npos &&

[battre, 2012/10/16 16:05:49]

KSSearch --> this is again an abbreviation that is not obvious. Please expand
the abbrev.

[Sergiu, 2012/10/17 14:48:58]

Done.

+          query.length() == strlen(chrome::kSSearchParamsSingle)) ||

[battre, 2012/10/16 16:05:49]

if query contains kSSearchParamsSingle and has the same length it is equal
right? Why don't you test that?

[Sergiu, 2012/10/17 14:48:58]

Done.

+          query.rfind(chrome::kSSearchParamsMultiple) != std::string::npos)
+        return net::OK;

[Joao da Silva, 2012/10/16 16:26:46]

|request| is not added to mod_by_managed_mode_ in this case?

[Bernhard Bauer, 2012/10/16 16:37:33]

Nit: newline

[Sergiu, 2012/10/17 14:48:58]

Done.

[Sergiu, 2012/10/17 14:48:58]

Changed the way this works, see new patch.

+      // Add this parameter at the end of this query since it will override the
+      // other parameters.
+      query.append(append_param);
+
+      GURL::Replacements replacements;
+      replacements.SetQueryStr(query);
+      *new_url = request->url().ReplaceComponents(replacements);
+      // Add it to the set so that we know that it can't be altered by an
+      // extension.
+      mod_by_managed_mode_.insert(request);

[Joao da Silva, 2012/10/16 16:26:46]

Suggestion: instead of using this std::set, why not store a private key in the
request? net::URLRequest implements base::SupportsUserData, and you can attach
anything to it. In this case, you could attach a bogus value (e.g. a new int)
and then later test that GetUserData(g_safe_search_key) is not NULL.

+
+      return net::OK;
+    }
+  }
+
   return ExtensionWebRequestEventRouter::GetInstance()->OnBeforeRequest(
       profile_, extension_info_map_.get(), request, callback, new_url);
 }
 
 int ChromeNetworkDelegate::OnBeforeSendHeaders(
     net::URLRequest* request,
     const net::CompletionCallback& callback,
     net::HttpRequestHeaders* headers) {
   return ExtensionWebRequestEventRouter::GetInstance()->OnBeforeSendHeaders(
       profile_, extension_info_map_.get(), request, callback, headers);
@@ skipping 11 matching lines @@
     const net::CompletionCallback& callback,
     net::HttpResponseHeaders* original_response_headers,
     scoped_refptr<net::HttpResponseHeaders>* override_response_headers) {
   return ExtensionWebRequestEventRouter::GetInstance()->OnHeadersReceived(
       profile_, extension_info_map_.get(), request, callback,
       original_response_headers, override_response_headers);
 }
 
 void ChromeNetworkDelegate::OnBeforeRedirect(net::URLRequest* request,
                                              const GURL& new_location) {
-  ExtensionWebRequestEventRouter::GetInstance()->OnBeforeRedirect(
-      profile_, extension_info_map_.get(), request, new_location);
+  // If the request has been modified by the managed mode then do not route

[Pam (message me for reviews), 2012/10/17 11:37:37]

"...by policy" rather than "by the managed mode", for generality

[Sergiu, 2012/10/17 14:48:58]

No longer applicable (done).

+  // it through the extension so that it doesn't get overwritten.
+  if (mod_by_managed_mode_.find(request) != mod_by_managed_mode_.end()) {

[Joao da Silva, 2012/10/16 16:26:46]

I'm not sure about this, because the extension will see other events
(OnHeadersReceived, OnBeforeSendHeaders, etc) but not these. This might confuse
some extensions that expect to see them all. I leave it to Dominic to decide if
this is acceptable.

[Bernhard Bauer, 2012/10/16 16:37:33]

+1. Can't we just apply the filter after any extensions?

[Pam (message me for reviews), 2012/10/17 11:37:37]

Agreed, it's odd to remove these requests from what an extension sees. We don't
know what the extension might be doing; it could be something else entirely, not
affecting the URL.

[Sergiu, 2012/10/17 14:48:58]

Please review the new way in which we are doing this.

+    mod_by_managed_mode_.erase(request);

[battre, 2012/10/16 16:05:49]

I would consider adding a return statement here and deleting the else below.

[Sergiu, 2012/10/17 14:48:58]

Done.

+  } else {
+    ExtensionWebRequestEventRouter::GetInstance()->OnBeforeRedirect(
+        profile_, extension_info_map_.get(), request, new_location);
+  }
 }
 
 
 void ChromeNetworkDelegate::OnResponseStarted(net::URLRequest* request) {
   ExtensionWebRequestEventRouter::GetInstance()->OnResponseStarted(
       profile_, extension_info_map_.get(), request);
   ForwardProxyErrors(request, event_router_.get(), profile_);
 }
 
 void ChromeNetworkDelegate::OnRawBytesRead(const net::URLRequest& request,
@@ skipping 183 matching lines @@
 #endif
   return net::OK;
 }
 
 void ChromeNetworkDelegate::OnRequestWaitStateChange(
     const net::URLRequest& request,
     RequestWaitState state) {
   if (load_time_stats_)
     load_time_stats_->OnRequestWaitStateChange(request, state);
 }