SSL in EmbeddedTestServer

net/test/embedded_test_server/request_helpers.cc

Index: net/test/embedded_test_server/request_helpers.cc
diff --git a/net/test/embedded_test_server/request_helpers.cc b/net/test/embedded_test_server/request_helpers.cc
new file mode 100644
index 0000000000000000000000000000000000000000..94ae8d34cb2eb1d0152a25ce142718d8a0a8f54a
--- /dev/null
+++ b/net/test/embedded_test_server/request_helpers.cc
@@ -0,0 +1,246 @@
+// 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 "net/test/embedded_test_server/request_helpers.h"
+
+#include <stdlib.h>
+#include <ctime>
+#include <map>
+#include <sstream>
+#include <string>

[mmenke, 2015/10/21 20:21:37]

map, string should be in the header, per comment in other file.

[svaldez, 2015/10/21 21:22:29]

Done.

+
+#include "base/base64.h"
+#include "base/files/file_path.h"

[mmenke, 2015/10/21 20:21:37]

Already in header

[svaldez, 2015/10/21 21:22:29]

Done.

+#include "base/files/file_util.h"
+#include "base/format_macros.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
+#include "base/threading/thread_restrictions.h"
+#include "net/base/escape.h"
+#include "net/base/url_util.h"
+#include "net/http/http_byte_range.h"
+#include "net/http/http_util.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
+
+namespace net {
+namespace test_server {
+namespace {
+
+const UnescapeRule::Type kUnescapeAll =
+    UnescapeRule::SPACES | UnescapeRule::URL_SPECIAL_CHARS |
+    UnescapeRule::SPOOFING_AND_CONTROL_CHARS |
+    UnescapeRule::REPLACE_PLUS_WITH_SPACE;
+
+std::string GetContentType(const base::FilePath& path) {
+  if (path.MatchesExtension(FILE_PATH_LITERAL(".crx")))
+    return "application/x-chrome-extension";
+  if (path.MatchesExtension(FILE_PATH_LITERAL(".exe")))
+    return "application/octet-stream";
+  if (path.MatchesExtension(FILE_PATH_LITERAL(".gif")))
+    return "image/gif";
+  if (path.MatchesExtension(FILE_PATH_LITERAL(".jpeg")) ||
+      path.MatchesExtension(FILE_PATH_LITERAL(".jpg"))) {
+    return "image/jpeg";
+  }
+  if (path.MatchesExtension(FILE_PATH_LITERAL(".js")))
+    return "application/javascript";
+  if (path.MatchesExtension(FILE_PATH_LITERAL(".json")))
+    return "application/json";
+  if (path.MatchesExtension(FILE_PATH_LITERAL(".pdf")))
+    return "application/pdf";
+  if (path.MatchesExtension(FILE_PATH_LITERAL(".txt")))
+    return "text/plain";
+  if (path.MatchesExtension(FILE_PATH_LITERAL(".wav")))
+    return "audio/wav";
+  if (path.MatchesExtension(FILE_PATH_LITERAL(".xml")))
+    return "text/xml";
+  if (path.MatchesExtension(FILE_PATH_LITERAL(".html")) ||
+      path.MatchesExtension(FILE_PATH_LITERAL(".htm"))) {
+    return "text/html";
+  }
+  return "";
+}
+
+}  // namespace
+
+CustomHttpResponse::CustomHttpResponse(const std::string& headers,
+                                       const std::string& contents)
+    : headers_(headers), contents_(contents) {}
+
+void CustomHttpResponse::SendResponse(const SendBytesCallback& send,
+                                      const SendCompleteCallback& done) {
+  std::string response;
+  if (!headers_.empty() || !contents_.empty())
+    response = headers_ + "\r\n" + contents_;

[mmenke, 2015/10/21 20:21:37]

This seems kinda weird, if headers_ is empty but contents_ is not.  Then we'd be
returning an HTTP/0.9 response, with a bonus CRLF as the first characters of the
body.

Maybe:

if (!headers_.empty())
  response = headers_ + "\r\n" + contents_;
else
  response = contents_;

That gives us HTTP/0.9 support and sane handling of requests with contents but
no headers.

Or could just DCHECK in the constructor that if contents_ is non-null, neither
is headers_.

[svaldez, 2015/10/21 21:22:29]

Done.

+  send.Run(response, done);
+}
+
+bool ShouldHandle(const HttpRequest& request, const std::string& path_prefix) {
+  GURL url = request.ToURL();
+  return url.path() == path_prefix ||
+         base::StartsWith(url.path(), path_prefix + "/",
+                          base::CompareCase::SENSITIVE);
+}
+
+scoped_ptr<HttpResponse> HandlePrefixedRequest(
+    const std::string& prefix,
+    const EmbeddedTestServer::HandleRequestCallback& handler,
+    const HttpRequest& request) {
+  if (ShouldHandle(request, prefix))
+    return handler.Run(request);
+  return nullptr;
+}
+
+RequestQuery ParseQuery(const GURL& url) {
+  RequestQuery queries;
+  for (QueryIterator it(url); !it.IsAtEnd(); it.Advance()) {
+    queries[net::UnescapeURLComponent(it.GetKey(), kUnescapeAll)].push_back(
+        it.GetUnescapedValue());
+  }
+  return queries;
+}
+
+void GetFilePathWithReplacements(const std::string& original_file_path,
+                                 const base::StringPairs& text_to_replace,
+                                 std::string* replacement_path) {
+  std::string new_file_path = original_file_path;
+  bool first_query_parameter = true;
+  for (auto replacement : text_to_replace) {

[mmenke, 2015/10/21 20:21:37]

const auto& is preferred, I believe.

[svaldez, 2015/10/21 21:22:29]

Done.

+    const std::string& old_text = replacement.first;
+    const std::string& new_text = replacement.second;
+    std::string base64_old;
+    std::string base64_new;
+    base::Base64Encode(old_text, &base64_old);
+    base::Base64Encode(new_text, &base64_new);
+    if (first_query_parameter) {

[mmenke, 2015/10/21 20:21:37]

nit:  Maybe get rid of this variable and use new_file_path.empty()?

[svaldez, 2015/10/21 21:22:29]

Done.

+      new_file_path += "?";
+      first_query_parameter = false;
+    } else {
+      new_file_path += "&";
+    }
+    new_file_path += "replace_text=";
+    new_file_path += base64_old;
+    new_file_path += ":";
+    new_file_path += base64_new;
+  }
+
+  *replacement_path = new_file_path;
+}
+
+// Handles |request| by serving a file from under |server_root|.
+scoped_ptr<HttpResponse> HandleFileRequest(const base::FilePath& server_root,
+                                           const HttpRequest& request) {
+  // This is a test-only server. Ignore I/O thread restrictions.
+  // TODO(svaldez): Figure out why restriction is necessary.

[mmenke, 2015/10/21 20:21:37]

Think "why restriction is necessary" -> "why thread is I/O restricted in the
first place" is clearer.

[svaldez, 2015/10/21 21:22:29]

Done.

+  base::ThreadRestrictions::ScopedAllowIO allow_io;
+
+  std::string relative_path(request.relative_url);

[mmenke, 2015/10/21 20:21:37]

Not needed - can you overwrite it unconditionally a couple lines down.  Can just
get rid of this line and declare / initialize this there.

[svaldez, 2015/10/21 21:22:29]

Done.

+
+  // A proxy request will have an absolute path. Simulate the proxy by stripping
+  // the scheme, host, and port.
+  GURL request_url = request.ToURL();
+  relative_path = request_url.path();
+
+  std::string post_prefix("/post/");
+  if (base::StartsWith(relative_path, post_prefix,
+                       base::CompareCase::SENSITIVE)) {
+    relative_path = relative_path.substr(post_prefix.size() - 1);
+    if (request.method != METHOD_POST)
+      return nullptr;

[mmenke, 2015/10/21 20:21:37]

Suggest moving this above setting relative_path.  Doesn't really matter, just
seems weird to calculate the path and then bail.

[svaldez, 2015/10/21 21:22:29]

Done.

+  }
+
+  RequestQuery query = ParseQuery(request_url);
+
+  scoped_ptr<BasicHttpResponse> failed_response(new BasicHttpResponse);
+  failed_response->set_code(HTTP_NOT_FOUND);
+
+  if (query.find("expected_body") != query.end()) {
+    if (request.content.find(query["expected_body"].front()) ==
+        std::string::npos) {
+      return failed_response.Pass();
+    }
+  }
+
+  if (query.find("expected_headers") != query.end()) {
+    for (auto header : query["expected_headers"]) {

[mmenke, 2015/10/21 20:21:37]

const auto&

[svaldez, 2015/10/21 21:22:29]

Done.

+      if (header.find(":") == std::string::npos)
+        return failed_response.Pass();
+      std::string key = header.substr(0, header.find(":"));
+      std::string value = header.substr(header.find(":") + 1);

[mmenke, 2015/10/21 20:21:37]

Hrm...Thought I commented on this.  If header.find(":") is npos, what does this
do?  Use the same value as key and value?  That seems not great.

Same for the base::Base64Decode logic.

[svaldez, 2015/10/21 21:22:29]

But it won't be since we return a failure a few lines above?

+      if (request.headers.find(key) == request.headers.end() ||
+          request.headers.at(key) != value) {
+        return failed_response.Pass();
+      }
+    }
+  }
+
+  // Trim the first byte ('/').

[mmenke, 2015/10/21 20:21:37]

DCHECK that relative_path starts with "/"?

[svaldez, 2015/10/21 21:22:29]

Done.

+  std::string request_path = relative_path.substr(1);
+  base::FilePath file_path(server_root.AppendASCII(request_path));
+  std::string file_contents;
+  if (!base::ReadFileToString(file_path, &file_contents)) {
+    file_path = file_path.AppendASCII("index.html");
+    if (!base::ReadFileToString(file_path, &file_contents))
+      return nullptr;
+  }
+
+  if (request.method == METHOD_HEAD)
+    file_contents = "";
+
+  if (query.find("replace_text") != query.end()) {
+    for (auto replacement : query["replace_text"]) {

[mmenke, 2015/10/21 20:21:37]

const auto&

[svaldez, 2015/10/21 21:22:29]

Done.

+      std::string find;
+      std::string with;
+      base::Base64Decode(replacement.substr(0, replacement.find(":")), &find);
+      base::Base64Decode(replacement.substr(replacement.find(":") + 1), &with);
+      base::ReplaceSubstringsAfterOffset(&file_contents, 0, find, with);
+    }
+  }
+
+  base::FilePath headers_path(
+      file_path.AddExtension(FILE_PATH_LITERAL("mock-http-headers")));
+
+  if (base::PathExists(headers_path)) {
+    std::string headers_contents;
+
+    if (!base::ReadFileToString(headers_path, &headers_contents))
+      return nullptr;
+
+    scoped_ptr<CustomHttpResponse> http_response(
+        new CustomHttpResponse(headers_contents, file_contents));
+    return http_response.Pass();

[mmenke, 2015/10/21 20:21:37]

optional nit:  Could just do:

return make_scoped_ptr(
    new CustomHttpResponse(headers_contents, file_contents));

[svaldez, 2015/10/21 21:22:29]

Done.

+  }
+
+  scoped_ptr<BasicHttpResponse> http_response(new BasicHttpResponse);
+  http_response->set_code(HTTP_OK);
+
+  if (request.headers.find("Range") != request.headers.end()) {
+    std::vector<HttpByteRange> ranges;
+    HttpUtil::ParseRangeHeader(request.headers.at("Range"), &ranges);
+    if (ranges.size() == 1) {

[mmenke, 2015/10/21 20:21:37]

Can we fail if size() is not 1, or if ParseRangeHeader returns false?

[svaldez, 2015/10/21 21:22:29]

I think we should be ignoring Range we don't support and returning the full
body, as per spec.

[mmenke, 2015/10/21 21:36:38]

Good point - you should then include the result of HttpUtil::ParseRangeHeader in
your if statement.  If we can only parse part of the range, and the rest fails,
should return the full result, too, no?  HttpUtil::ParseRangeHeader has no
guarantees of not modifying ranges on failure.

+      ranges[0].ComputeBounds(file_contents.size());
+      size_t start = ranges[0].first_byte_position();
+      size_t end = ranges[0].last_byte_position();
+
+      http_response->set_code(HTTP_PARTIAL_CONTENT);
+      http_response->AddCustomHeader(
+          "Content-Range",
+          base::StringPrintf("bytes %" PRIuS "-%" PRIuS "/%" PRIuS, start, end,
+                             file_contents.size()));
+
+      file_contents = file_contents.substr(start, end - start + 1);
+    }
+  }
+
+  http_response->set_content_type(GetContentType(file_path));
+  http_response->AddCustomHeader("Accept-Ranges", "bytes");
+  http_response->AddCustomHeader("ETag", "'" + file_path.MaybeAsASCII() + "'");
+  http_response->set_content(file_contents);
+  return http_response.Pass();
+}
+
+}  // namespace test_server
+}  // namespace net