Recognize the --net-log-capture-mode switch in Network Service.

Bug: 962523
Change-Id: I7f1d20a2864470b0addb8064f6e553ea3d95435c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1614697
Commit-Queue: Eric Roman <eroman@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661101}

chrome/browser/BUILD.gn

@@ -834,8 +834,6 @@ jumbo_split_static_library("browser") {
     "net/chrome_extensions_network_delegate.h",
     "net/chrome_mojo_proxy_resolver_factory.cc",
     "net/chrome_mojo_proxy_resolver_factory.h",
-    "net/chrome_net_log_helper.cc",
-    "net/chrome_net_log_helper.h",
     "net/chrome_network_delegate.cc",
     "net/chrome_network_delegate.h",
     "net/chrome_report_sender.cc",

chrome/browser/browser_process_impl.cc

@@ -59,7 +59,6 @@
 #include "chrome/browser/metrics/chrome_metrics_services_manager_client.h"
 #include "chrome/browser/metrics/metrics_reporting_state.h"
 #include "chrome/browser/metrics/thread_watcher.h"
-#include "chrome/browser/net/chrome_net_log_helper.h"
 #include "chrome/browser/net/system_network_context_manager.h"
 #include "chrome/browser/notifications/notification_platform_bridge.h"
 #include "chrome/browser/notifications/system_notification_helper.h"
@@ -1178,7 +1177,9 @@ void BrowserProcessImpl::PreCreateThreads(
       log_file = user_data_dir.AppendASCII("netlog.json");
     }
     net_log_->StartWritingToFile(
-        log_file, GetNetCaptureModeFromCommandLine(command_line),
+        log_file,
+        net::GetNetCaptureModeFromCommandLine(
+            command_line, network::switches::kNetLogCaptureMode),
         command_line.GetCommandLineString(), chrome::GetChannelName());
   }
 

chrome/browser/net/chrome_net_log_helper.cc

-// Copyright 2015 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_net_log_helper.h"
-
-#include "base/command_line.h"
-#include "base/logging.h"
-#include "chrome/common/chrome_switches.h"
-
-net::NetLogCaptureMode GetNetCaptureModeFromCommandLine(
-    const base::CommandLine& command_line) {
-  if (command_line.HasSwitch(switches::kNetLogCaptureMode)) {
-    std::string capture_mode_string =
-        command_line.GetSwitchValueASCII(switches::kNetLogCaptureMode);
-    if (capture_mode_string == "Default")
-      return net::NetLogCaptureMode::Default();
-    if (capture_mode_string == "IncludeCookiesAndCredentials")
-      return net::NetLogCaptureMode::IncludeCookiesAndCredentials();
-    if (capture_mode_string == "IncludeSocketBytes")
-      return net::NetLogCaptureMode::IncludeSocketBytes();
-
-    LOG(ERROR) << "Unrecognized value for --" << switches::kNetLogCaptureMode;
-  }
-
-  return net::NetLogCaptureMode::Default();
-}

chrome/browser/net/chrome_net_log_helper.h

-// Copyright 2015 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.
-
-#ifndef CHROME_BROWSER_NET_CHROME_NET_LOG_HELPER_H_
-#define CHROME_BROWSER_NET_CHROME_NET_LOG_HELPER_H_
-
-#include "net/log/net_log_capture_mode.h"
-
-namespace base {
-class CommandLine;
-}
-
-// Returns the capture mode to be used with the net log.
-net::NetLogCaptureMode GetNetCaptureModeFromCommandLine(
-    const base::CommandLine& command_line);
-
-#endif  // CHROME_BROWSER_NET_CHROME_NET_LOG_HELPER_H_

chrome/browser/net/log_net_log_browsertest.cc

@@ -13,8 +13,13 @@
 namespace chrome_browser_net {
 namespace {
 
-// Tests for the --log-net-log command line flag.
-class LogNetLogTest : public InProcessBrowserTest {
+// Test fixture for running tests with --log-net-log, and a parameterized value
+// for --net-log-capture-mode.
+//
+// Asserts that a netlog file was created, appears valid, and stripped cookies
+// in accordance to the --net-log-capture-mode flag.
+class LogNetLogTest : public InProcessBrowserTest,
+                      public testing::WithParamInterface<const char*> {
  public:
   LogNetLogTest() = default;
 
@@ -24,12 +29,18 @@ class LogNetLogTest : public InProcessBrowserTest {
 
     command_line->AppendSwitchPath(network::switches::kLogNetLog,
                                    net_log_path_);
+
+    if (GetParam()) {
+      command_line->AppendSwitchASCII(network::switches::kNetLogCaptureMode,
+                                      GetParam());
+    }
   }
 
   void TearDownInProcessBrowserTestFixture() override { VerifyNetLog(); }
 
  private:
-  // Verify that the netlog file was written and appears to be well formed.
+  // Verify that the netlog file was written, appears to be well formed, and
+  // includes the requested level of data.
   void VerifyNetLog() {
     // Read the netlog from disk.
     std::string file_contents;
@@ -53,6 +64,20 @@ class LogNetLogTest : public InProcessBrowserTest {
     base::ListValue* events;
     ASSERT_TRUE(main->GetList("events", &events));
     ASSERT_FALSE(events->empty());
+
+    // Verify that cookies were stripped when the --net-log-capture-mode flag
+    // was omitted, and not stripped when it was given a value of
+    // IncludeCookiesAndCredentials
+    bool include_cookies = GetParam() && base::StringPiece(GetParam()) ==
+                                             "IncludeCookiesAndCredentials";
+
+    if (include_cookies) {
+      EXPECT_TRUE(file_contents.find("Set-Cookie: name=Good;Max-Age=3600") !=
+                  std::string::npos);
+    } else {
+      EXPECT_TRUE(file_contents.find("Set-Cookie: [22 bytes were stripped]") !=
+                  std::string::npos);
+    }
   }
 
   base::FilePath net_log_path_;
@@ -61,11 +86,15 @@ class LogNetLogTest : public InProcessBrowserTest {
   DISALLOW_COPY_AND_ASSIGN(LogNetLogTest);
 };
 
-IN_PROC_BROWSER_TEST_F(LogNetLogTest, Basic) {
-  // Do an action that will result in the output of netlog events. This isn't
-  // strictly necessary since there is other networking that will happen
-  // implicitly to generate events.
-  ui_test_utils::NavigateToURL(browser(), GURL("http://127.0.0.1/foo"));
+INSTANTIATE_TEST_SUITE_P(,
+                         LogNetLogTest,
+                         ::testing::Values(nullptr,
+                                           "IncludeCookiesAndCredentials"));
+
+IN_PROC_BROWSER_TEST_P(LogNetLogTest, Basic) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL url(embedded_test_server()->GetURL("/set_cookie_header.html"));
+  ui_test_utils::NavigateToURL(browser(), url);
 }
 
 }  // namespace

chrome/common/chrome_switches.cc

@@ -400,16 +400,6 @@ const char kMediaCacheSize[]                = "media-cache-size";
 // messages. Useful when running against a non-prod management server.
 const char kMonitoringDestinationID[]       = "monitoring-destination-id";
 
-// Sets the granularity of events to capture in the network log. The mode can be
-// set to one of the following values:
-//   "Default"
-//   "IncludeCookiesAndCredentials"
-//   "IncludeSocketBytes"
-//
-// See the functions of the corresponding name in net_log_capture_mode.h for a
-// description of their meaning.
-const char kNetLogCaptureMode[]             = "net-log-capture-mode";
-
 // Disables the default browser check. Useful for UI/browser tests where we
 // want to avoid having the default browser info-bar displayed.
 const char kNoDefaultBrowserCheck[]         = "no-default-browser-check";

chrome/common/chrome_switches.h

@@ -125,7 +125,6 @@ extern const char kLoadMediaRouterComponentExtension[];
 extern const char kMakeDefaultBrowser[];
 extern const char kMediaCacheSize[];
 extern const char kMonitoringDestinationID[];
-extern const char kNetLogCaptureMode[];
 extern const char kNewNetErrorPageUI[];
 extern const char kNoDefaultBrowserCheck[];
 extern const char kNoExperiments[];

content/browser/network_service_instance.cc

@@ -183,10 +183,12 @@ CONTENT_EXPORT network::mojom::NetworkService* GetNetworkServiceFromConnector(
           if (!file.IsValid()) {
             LOG(ERROR) << "Failed opening: " << log_path.value();
           } else {
-            // TODO(mmenke): Get capture mode from the command line.
+            net::NetLogCaptureMode capture_mode =
+                net::GetNetCaptureModeFromCommandLine(
+                    *command_line, network::switches::kNetLogCaptureMode);
+
             (*g_network_service_ptr)
-                ->StartNetLog(std::move(file),
-                              net::NetLogCaptureMode::Default(),
+                ->StartNetLog(std::move(file), capture_mode,
                               std::move(client_constants));
           }
         }

content/browser/utility_process_host.cc

@@ -372,6 +372,7 @@ bool UtilityProcessHost::StartProcess() {
       network::switches::kIgnoreCertificateErrorsSPKIList,
       network::switches::kIgnoreUrlFetcherCertRequests,
       network::switches::kLogNetLog,
+      network::switches::kNetLogCaptureMode,
       network::switches::kNoReferrers,
       network::switches::kExplicitlyAllowedPorts,
       service_manager::switches::kNoSandbox,

net/log/net_log_capture_mode.cc

@@ -6,6 +6,9 @@
 
 #include <algorithm>
 
+#include "base/command_line.h"
+#include "base/strings/string_piece.h"
+
 namespace net {
 
 namespace {
@@ -64,4 +67,23 @@ bool NetLogCaptureMode::operator!=(NetLogCaptureMode mode) const {
 NetLogCaptureMode::NetLogCaptureMode(uint32_t value) : value_(value) {
 }
 
+NetLogCaptureMode GetNetCaptureModeFromCommandLine(
+    const base::CommandLine& command_line,
+    base::StringPiece switch_name) {
+  if (command_line.HasSwitch(switch_name)) {
+    std::string value = command_line.GetSwitchValueASCII(switch_name);
+
+    if (value == "Default")
+      return NetLogCaptureMode::Default();
+    if (value == "IncludeCookiesAndCredentials")
+      return NetLogCaptureMode::IncludeCookiesAndCredentials();
+    if (value == "IncludeSocketBytes")
+      return NetLogCaptureMode::IncludeSocketBytes();
+
+    LOG(ERROR) << "Unrecognized value for --" << switch_name;
+  }
+
+  return net::NetLogCaptureMode::Default();
+}
+
 }  // namespace net

net/log/net_log_capture_mode.h

@@ -9,8 +9,13 @@
 
 #include <string>
 
+#include "base/strings/string_piece_forward.h"
 #include "net/base/net_export.h"
 
+namespace base {
+class CommandLine;
+}
+
 namespace net {
 
 // NetLogCaptureMode specifies the granularity of events that should be emitted
@@ -59,6 +64,11 @@ class NET_EXPORT NetLogCaptureMode {
   int32_t value_;
 };
 
+// Parses a NetLogCaptureMode given an optional command-line switch.
+NET_EXPORT NetLogCaptureMode
+GetNetCaptureModeFromCommandLine(const base::CommandLine& command_line,
+                                 base::StringPiece switch_name);
+
 }  // namespace net
 
 #endif  // NET_LOG_NET_LOG_CAPTURE_MODE_H_

services/network/public/cpp/network_switches.cc

@@ -40,6 +40,16 @@ const char kIgnoreCertificateErrorsSPKIList[] =
 // user data directory.
 const char kLogNetLog[] = "log-net-log";
 
+// Sets the granularity of events to capture in the network log. The mode can be
+// set to one of the following values:
+//   "Default"
+//   "IncludeCookiesAndCredentials"
+//   "IncludeSocketBytes"
+//
+// See the functions of the corresponding name in net_log_capture_mode.h for a
+// description of their meaning.
+const char kNetLogCaptureMode[] = "net-log-capture-mode";
+
 // Causes SSL key material to be logged to the specified file for debugging
 // purposes. See
 // https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/Key_Log_Format

services/network/public/cpp/network_switches.h

@@ -18,6 +18,7 @@ COMPONENT_EXPORT(NETWORK_CPP)
 extern const char kIgnoreCertificateErrorsSPKIList[];
 COMPONENT_EXPORT(NETWORK_CPP) extern const char kIgnoreUrlFetcherCertRequests[];
 COMPONENT_EXPORT(NETWORK_CPP) extern const char kLogNetLog[];
+COMPONENT_EXPORT(NETWORK_CPP) extern const char kNetLogCaptureMode[];
 COMPONENT_EXPORT(NETWORK_CPP) extern const char kSSLKeyLogFile[];
 COMPONENT_EXPORT(NETWORK_CPP) extern const char kNoReferrers[];
 COMPONENT_EXPORT(NETWORK_CPP) extern const char kExplicitlyAllowedPorts[];