Switch LocalTwoPhaseTestServer to the EmbeddedTestServer.

It's one of only two remaining consumers of the SpawnedTestServer in
HTTP mode.  While the still need the SpawnedTestServer for some special
cases (e.g., client certs), it's no longer needed in any simple HTTP
case, and it's been an unending source of test flake.  Finally being
able to remove HTTP support from it is a big step towards deprecating
it.

Bug: 492672
Change-Id: If8fb9994d675950b0bbd718d4933da4925f0f64d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2510372Reviewed-by: Daniel Rubery <drubery@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822839}

chrome/browser/safe_browsing/download_protection/two_phase_uploader_unittest.cc

@@ -108,7 +108,7 @@ TEST_F(TwoPhaseUploaderTest, UploadFile) {
   Delegate delegate;
   std::unique_ptr<TwoPhaseUploader> uploader(TwoPhaseUploader::Create(
       shared_url_loader_factory_, task_runner_.get(),
-      test_server.GetURL("start"), "metadata", GetTestFilePath(),
+      test_server.GetURL("/start"), "metadata", GetTestFilePath(),
       base::BindOnce(&Delegate::FinishCallback, base::Unretained(&delegate),
                      runner),
       TRAFFIC_ANNOTATION_FOR_TESTS));
@@ -131,7 +131,7 @@ TEST_F(TwoPhaseUploaderTest, BadPhaseOneResponse) {
   Delegate delegate;
   std::unique_ptr<TwoPhaseUploader> uploader(TwoPhaseUploader::Create(
       shared_url_loader_factory_, task_runner_.get(),
-      test_server.GetURL("start?p1code=500"), "metadata", GetTestFilePath(),
+      test_server.GetURL("/start?p1code=500"), "metadata", GetTestFilePath(),
       base::BindOnce(&Delegate::FinishCallback, base::Unretained(&delegate),
                      runner),
       TRAFFIC_ANNOTATION_FOR_TESTS));
@@ -150,7 +150,7 @@ TEST_F(TwoPhaseUploaderTest, BadPhaseTwoResponse) {
   Delegate delegate;
   std::unique_ptr<TwoPhaseUploader> uploader(TwoPhaseUploader::Create(
       shared_url_loader_factory_, task_runner_.get(),
-      test_server.GetURL("start?p2code=500"), "metadata", GetTestFilePath(),
+      test_server.GetURL("/start?p2code=500"), "metadata", GetTestFilePath(),
       base::BindOnce(&Delegate::FinishCallback, base::Unretained(&delegate),
                      runner),
       TRAFFIC_ANNOTATION_FOR_TESTS));
@@ -173,7 +173,7 @@ TEST_F(TwoPhaseUploaderTest, PhaseOneConnectionClosed) {
   Delegate delegate;
   std::unique_ptr<TwoPhaseUploader> uploader(TwoPhaseUploader::Create(
       shared_url_loader_factory_, task_runner_.get(),
-      test_server.GetURL("start?p1close=1"), "metadata", GetTestFilePath(),
+      test_server.GetURL("/start?p1close=1"), "metadata", GetTestFilePath(),
       base::BindOnce(&Delegate::FinishCallback, base::Unretained(&delegate),
                      runner),
       TRAFFIC_ANNOTATION_FOR_TESTS));
@@ -191,7 +191,7 @@ TEST_F(TwoPhaseUploaderTest, PhaseTwoConnectionClosed) {
   Delegate delegate;
   std::unique_ptr<TwoPhaseUploader> uploader(TwoPhaseUploader::Create(
       shared_url_loader_factory_, task_runner_.get(),
-      test_server.GetURL("start?p2close=1"), "metadata", GetTestFilePath(),
+      test_server.GetURL("/start?p2close=1"), "metadata", GetTestFilePath(),
       base::BindOnce(&Delegate::FinishCallback, base::Unretained(&delegate),
                      runner),
       TRAFFIC_ANNOTATION_FOR_TESTS));

chrome/browser/safe_browsing/local_two_phase_testserver.cc

@@ -4,37 +4,133 @@
 
 #include "chrome/browser/safe_browsing/local_two_phase_testserver.h"
 
+#include <string>
+#include <vector>
+
+#include "base/bind.h"
 #include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/hash/sha1.h"
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/values.h"
 #include "chrome/common/chrome_paths.h"
+#include "net/base/url_util.h"
+#include "net/http/http_status_code.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
 #include "net/test/python_utils.h"
-#include "net/test/spawned_test_server/spawned_test_server.h"
 
 namespace safe_browsing {
 
-LocalTwoPhaseTestServer::LocalTwoPhaseTestServer()
-    : net::LocalTestServer(net::SpawnedTestServer::TYPE_HTTP,
-                           base::FilePath()) {}
+namespace {
 
-LocalTwoPhaseTestServer::~LocalTwoPhaseTestServer() {}
+// Computes the SHA-1 of input string, and returns it as an ASCII-encoded string
+// of hex characters.
+std::string SHA1HexEncode(const std::string& in) {
+  std::string raw_sha1 = base::SHA1HashString(in);
+  return base::ToLowerASCII(base::HexEncode(raw_sha1.c_str(), raw_sha1.size()));
+}
+
+const char kStartHeader[] = "x-goog-resumable";
+
+std::unique_ptr<net::test_server::HttpResponse> HandleTwoPhaseRequest(
+    const net::test_server::HttpRequest& request) {
+  auto response = std::make_unique<net::test_server::BasicHttpResponse>();
+  GURL url = request.GetURL();
+  if (request.method == net::test_server::METHOD_POST) {
+    const auto start_header = request.headers.find(kStartHeader);
+    if (start_header == request.headers.end()) {
+      response->set_code(net::HTTP_BAD_REQUEST);
+      LOG(WARNING) << "Missing header: " << kStartHeader;
+      return response;
+    }
+    if (start_header->second != "start") {
+      response->set_code(net::HTTP_BAD_REQUEST);
+      LOG(WARNING) << "Invalid " << kStartHeader
+                   << " value: " << start_header->second;
+      return response;
+    }
+
+    std::string medadata_hash = SHA1HexEncode(request.content);
+
+    // Default response code.
+    int p1code = 201;
+
+    std::string p2close = "";
+    std::string p2code = "200";
+
+    for (net::QueryIterator it(url); !it.IsAtEnd(); it.Advance()) {
+      // Hang up without sending data, in the case of "p1close".
+      if (it.GetKey() == "p1close")
+        return std::make_unique<net::test_server::RawHttpResponse>("", "");
 
-bool LocalTwoPhaseTestServer::GetTestServerPath(
-    base::FilePath* testserver_path) const {
-  base::FilePath testserver_dir;
-  if (!base::PathService::Get(chrome::DIR_TEST_DATA, &testserver_dir)) {
-    LOG(ERROR) << "Failed to get DIR_TEST_DATA";
-    return false;
+      if (it.GetKey() == "p1code") {
+        CHECK(base::StringToInt(it.GetValue(), &p1code));
+      }
+      if (it.GetKey() == "p2close")
+        p2close = "1";
+      if (it.GetKey() == "p2code")
+        p2code = it.GetValue();
+    }
+
+    std::string put_url = base::StringPrintf(
+        "%sput?/%s,%s,%s,%s", request.base_url.spec().c_str(),
+        url.path().c_str(), medadata_hash.c_str(), p2close.c_str(),
+        p2code.c_str());
+    response->set_code(static_cast<net::HttpStatusCode>(p1code));
+    response->AddCustomHeader("Location", put_url);
+    return response;
+  }
+  if (request.method == net::test_server::METHOD_PUT) {
+    if (url.path_piece() != "/put") {
+      response->set_code(net::HTTP_BAD_REQUEST);
+      LOG(WARNING) << "Invalid path on 2nd phase: " << url.path();
+      return response;
+    }
+
+    std::vector<std::string> args =
+        base::SplitString(url.query_piece().substr(1), ",",
+                          base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
+    if (args.size() != 4) {
+      response->set_code(net::HTTP_BAD_REQUEST);
+      LOG(WARNING) << "Invalid query string 2nd phase: " << url.query();
+      return response;
+    }
+
+    std::string initial_path = args[0];
+    std::string metadata_hash = args[1];
+    std::string p2close = args[2];
+    int p2code = 200;
+    CHECK(base::StringToInt(args[3], &p2code));
+
+    // Hang up without sending data, in the case of "p2close".
+    if (!p2close.empty())
+      return std::make_unique<net::test_server::RawHttpResponse>("", "");
+
+    response->set_code(static_cast<net::HttpStatusCode>(p2code));
+    response->set_content(base::StringPrintf(
+        "%s\n%s\n%s\n", initial_path.c_str(), metadata_hash.c_str(),
+        SHA1HexEncode(request.content).c_str()));
+    return response;
   }
+  response->set_code(net::HTTP_BAD_REQUEST);
+  LOG(WARNING) << "Unexpected method: " << request.method_string;
+  return response;
+}
 
-  testserver_dir = testserver_dir
-      .Append(FILE_PATH_LITERAL("safe_browsing"));
+}  // namespace
 
-  *testserver_path = testserver_dir.Append(FILE_PATH_LITERAL(
-      "two_phase_testserver.py"));
-  return true;
+LocalTwoPhaseTestServer::LocalTwoPhaseTestServer()
+    : embedded_test_server_(net::EmbeddedTestServer::TYPE_HTTP) {
+  embedded_test_server_.RegisterRequestHandler(
+      base::BindRepeating(&HandleTwoPhaseRequest));
 }
 
+LocalTwoPhaseTestServer::~LocalTwoPhaseTestServer() {}
+
 }  // namespace safe_browsing

chrome/browser/safe_browsing/local_two_phase_testserver.h

@@ -5,26 +5,33 @@
 #ifndef CHROME_BROWSER_SAFE_BROWSING_LOCAL_TWO_PHASE_TESTSERVER_H_
 #define CHROME_BROWSER_SAFE_BROWSING_LOCAL_TWO_PHASE_TESTSERVER_H_
 
+#include <string>
+
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
-#include "net/test/spawned_test_server/local_test_server.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "url/gurl.h"
 
 namespace safe_browsing {
 
-// Runs a Python-based two phase upload test server on the same machine in which
-// the LocalTwoPhaseTestServer runs.
-class LocalTwoPhaseTestServer : public net::LocalTestServer {
+// Runs an in-process two phase upload test server.
+class LocalTwoPhaseTestServer {
  public:
   // Initialize a two phase protocol test server.
   LocalTwoPhaseTestServer();
 
-  ~LocalTwoPhaseTestServer() override;
+  ~LocalTwoPhaseTestServer();
+
+  GURL GetURL(const std::string& relative_path) {
+    return embedded_test_server_.GetURL(relative_path);
+  }
 
-  // Returns the path to two_phase_testserver.py.
-  bool GetTestServerPath(base::FilePath* testserver_path) const override;
+  bool Start() WARN_UNUSED_RESULT { return embedded_test_server_.Start(); }
 
  private:
+  net::EmbeddedTestServer embedded_test_server_;
+
   DISALLOW_COPY_AND_ASSIGN(LocalTwoPhaseTestServer);
 };
 

chrome/test/data/safe_browsing/two_phase_testserver.py

-#!/usr/bin/env python
-# Copyright 2013 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.
-
-"""Testserver for the two phase upload protocol."""
-
-import base64
-import BaseHTTPServer
-import hashlib
-import os
-import sys
-import urlparse
-
-BASE_DIR = os.path.dirname(os.path.abspath(__file__))
-
-sys.path.append(os.path.join(BASE_DIR, '..', '..', '..', '..', 'net',
-                             'tools', 'testserver'))
-import testserver_base
-
-
-class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
-  def ReadRequestBody(self):
-    """This function reads the body of the current HTTP request, handling
-    both plain and chunked transfer encoded requests."""
-
-    if self.headers.getheader('transfer-encoding') == 'chunked':
-      return ''
-
-    length = int(self.headers.getheader('content-length'))
-    return self.rfile.read(length)
-
-  def do_GET(self):
-    print 'GET', self.path
-    self.send_error(400, 'GET not supported')
-
-  def do_POST(self):
-    request_body = self.ReadRequestBody()
-    print 'POST', repr(self.path), repr(request_body)
-
-    kStartHeader = 'x-goog-resumable'
-    if kStartHeader not in self.headers:
-      self.send_error(400, 'Missing header: ' + kStartHeader)
-      return
-    if self.headers.get(kStartHeader) != 'start':
-      self.send_error(400, 'Invalid %s header value: %s' % (
-          kStartHeader, self.headers.get(kStartHeader)))
-      return
-
-    metadata_hash = hashlib.sha1(request_body).hexdigest()
-    _, _, url_path, _, query, _ = urlparse.urlparse(self.path)
-    query_args = urlparse.parse_qs(query)
-
-    if query_args.get('p1close'):
-      self.close_connection = 1
-      return
-
-    put_url = 'http://%s:%d/put?%s,%s,%s' % (self.server.server_address[0],
-                                          self.server.server_port,
-                                          url_path,
-                                          metadata_hash,
-                                          base64.urlsafe_b64encode(query))
-    self.send_response(int(query_args.get('p1code', [201])[0]))
-    self.send_header('Location', put_url)
-    self.end_headers()
-
-  def do_PUT(self):
-    _, _, url_path, _, query, _ = urlparse.urlparse(self.path)
-    if url_path != '/put':
-      self.send_error(400, 'invalid path on 2nd phase: ' + url_path)
-      return
-
-    initial_path, metadata_hash, config_query_b64 = query.split(',', 2)
-    config_query = urlparse.parse_qs(base64.urlsafe_b64decode(config_query_b64))
-
-    request_body = self.ReadRequestBody()
-    print 'PUT', repr(self.path), len(request_body), 'bytes'
-
-    if config_query.get('p2close'):
-      self.close_connection = 1
-      return
-
-    self.send_response(int(config_query.get('p2code', [200])[0]))
-    self.end_headers()
-    self.wfile.write('%s\n%s\n%s\n' % (
-        initial_path,
-        metadata_hash,
-        hashlib.sha1(request_body).hexdigest()))
-
-
-class ServerRunner(testserver_base.TestServerRunner):
-  """TestServerRunner for safebrowsing_test_server.py."""
-
-  def create_server(self, server_data):
-    server = BaseHTTPServer.HTTPServer((self.options.host, self.options.port),
-                                       RequestHandler)
-    print 'server started on port %d...' % server.server_port
-    server_data['port'] = server.server_port
-
-    return server
-
-
-if __name__ == '__main__':
-  sys.exit(ServerRunner().main())