Revert "Reland "Make PUSH_PROMISE headers URL-parsing more robust and conform to HTTP/2 spec""

This reverts commit 5ef6e548.

Reason for revert: net_unittests failing on android_cronet.

Original change's description:
> Reland "Make PUSH_PROMISE headers URL-parsing more robust and conform to HTTP/2 spec"
> 
> Reland of https://chromium-review.googlesource.com/c/chromium/src/+/887970.
> 
> MSAN use-of-uninitialized-value should be fixed.
> 
> Change-Id: I392ad1a83a269b47ff0c8583f6c72e18a4a0e5b9
> Reviewed-on: https://chromium-review.googlesource.com/891302
> Commit-Queue: Yixin Wang <wangyix@chromium.org>
> Reviewed-by: Ryan Hamilton <rch@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#532669}

TBR=rch@chromium.org,wangyix@chromium.org

Change-Id: I19d5f50c17b052f72cfa6025bcea2b4770363666
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/893864Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Commit-Queue: Yixin Wang <wangyix@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532954}

net/BUILD.gn

@@ -5165,7 +5165,6 @@ test("net_unittests") {
     "quic/platform/api/quic_test_random.h",
     "quic/platform/api/quic_text_utils_test.cc",
     "quic/platform/api/quic_url_test.cc",
-    "quic/platform/api/quic_url_utils_test.cc",
     "quic/platform/impl/quic_chromium_clock_test.cc",
     "quic/platform/impl/quic_test_random_impl.cc",
     "quic/platform/impl/quic_test_random_impl.h",

net/quic/chromium/quic_http_stream_test.cc

@@ -339,7 +339,7 @@ class QuicHttpStreamTest
     promised_response_[":version"] = "HTTP/1.1";
     promised_response_["content-type"] = "text/plain";
 
-    promise_url_ = SpdyUtils::GetPromisedUrlFromHeaderBlock(push_promise_);
+    promise_url_ = SpdyUtils::GetUrlFromHeaderBlock(push_promise_);
   }
 
   void SetRequest(const string& method,
@@ -1873,7 +1873,7 @@ TEST_P(QuicHttpStreamTest, ServerPushCrossOriginOK) {
   // packet, but does it matter?
 
   push_promise_[":authority"] = "mail.example.org";
-  promise_url_ = SpdyUtils::GetPromisedUrlFromHeaderBlock(push_promise_);
+  promise_url_ = SpdyUtils::GetUrlFromHeaderBlock(push_promise_);
 
   ReceivePromise(promise_id_);
   EXPECT_NE(session_->GetPromisedByUrl(promise_url_), nullptr);
@@ -1942,7 +1942,7 @@ TEST_P(QuicHttpStreamTest, ServerPushCrossOriginFail) {
   // TODO(ckrasic) - could do this via constructing a PUSH_PROMISE
   // packet, but does it matter?
   push_promise_[":authority"] = "www.notexample.org";
-  promise_url_ = SpdyUtils::GetPromisedUrlFromHeaderBlock(push_promise_);
+  promise_url_ = SpdyUtils::GetUrlFromHeaderBlock(push_promise_);
 
   ReceivePromise(promise_id_);
   // The promise will have been rejected because the cert doesn't

net/quic/core/quic_client_promised_info_test.cc

@@ -84,7 +84,7 @@ class QuicClientPromisedInfoTest : public QuicTest {
     push_promise_[":method"] = "GET";
     push_promise_[":scheme"] = "https";
 
-    promise_url_ = SpdyUtils::GetPromisedUrlFromHeaderBlock(push_promise_);
+    promise_url_ = SpdyUtils::GetUrlFromHeaderBlock(push_promise_);
 
     client_request_ = push_promise_.Clone();
     promise_id_ =

net/quic/core/quic_client_push_promise_index.cc

@@ -33,7 +33,7 @@ QuicAsyncStatus QuicClientPushPromiseIndex::Try(
     const SpdyHeaderBlock& request,
     QuicClientPushPromiseIndex::Delegate* delegate,
     TryHandle** handle) {
-  string url(SpdyUtils::GetPromisedUrlFromHeaderBlock(request));
+  string url(SpdyUtils::GetUrlFromHeaderBlock(request));
   QuicPromisedByUrlMap::iterator it = promised_by_url_.find(url);
   if (it != promised_by_url_.end()) {
     QuicClientPromisedInfo* promised = it->second;

net/quic/core/quic_client_push_promise_index_test.cc

@@ -63,7 +63,7 @@ class QuicClientPushPromiseIndexTest : public QuicTest {
     request_[":version"] = "HTTP/1.1";
     request_[":method"] = "GET";
     request_[":scheme"] = "https";
-    url_ = SpdyUtils::GetPromisedUrlFromHeaderBlock(request_);
+    url_ = SpdyUtils::GetUrlFromHeaderBlock(request_);
   }
 
   MockQuicConnectionHelper helper_;

net/quic/core/quic_spdy_client_session_base.cc

@@ -99,7 +99,7 @@ bool QuicSpdyClientSessionBase::HandlePromised(QuicStreamId /* associated_id */,
     return false;
   }
 
-  const string url = SpdyUtils::GetPromisedUrlFromHeaderBlock(headers);
+  const string url = SpdyUtils::GetUrlFromHeaderBlock(headers);
   QuicClientPromisedInfo* old_promised = GetPromisedByUrl(url);
   if (old_promised) {
     QUIC_DVLOG(1) << "Promise for stream " << promised_id

net/quic/core/spdy_utils.cc

@@ -128,69 +128,39 @@ bool SpdyUtils::CopyAndValidateTrailers(const QuicHeaderList& header_list,
 }
 
 // static
-string SpdyUtils::GetPromisedUrlFromHeaderBlock(
-    const SpdyHeaderBlock& headers) {
-  // RFC 7540, Section 8.1.2.3: All HTTP/2 requests MUST include exactly
-  // one valid value for the ":method", ":scheme", and ":path" pseudo-header
-  // fields, unless it is a CONNECT request.
-
-  // RFC 7540, Section  8.2.1:  The header fields in PUSH_PROMISE and any
-  // subsequent CONTINUATION frames MUST be a valid and complete set of request
-  // header fields (Section 8.1.2.3).  The server MUST include a method in the
-  // ":method" pseudo-header field that is safe and cacheable.
-  //
-  // RFC 7231, Section  4.2.1: Of the request methods defined by this
-  // specification, the GET, HEAD, OPTIONS, and TRACE methods are defined to be
-  // safe.
-  //
-  // RFC 7231, Section  4.2.1: ... this specification defines GET, HEAD, and
-  // POST as cacheable, ...
-  //
-  // So the only methods allowed in a PUSH_PROMISE are GET and HEAD.
-  SpdyHeaderBlock::const_iterator it = headers.find(":method");
-  if (it == headers.end() || (it->second != "GET" && it->second != "HEAD")) {
-    return string();
+string SpdyUtils::GetUrlFromHeaderBlock(const SpdyHeaderBlock& headers) {
+  SpdyHeaderBlock::const_iterator it = headers.find(":scheme");
+  if (it == headers.end()) {
+    return "";
   }
+  std::string url = it->second.as_string();
 
-  it = headers.find(":scheme");
-  if (it == headers.end() || it->second.empty()) {
-    return string();
-  }
-  QuicStringPiece scheme = it->second;
+  url.append("://");
 
-  // RFC 7540, Section 8.2: The server MUST include a value in the
-  // ":authority" pseudo-header field for which the server is authoritative
-  // (see Section 10.1).
   it = headers.find(":authority");
-  if (it == headers.end() || it->second.empty()) {
-    return string();
+  if (it == headers.end()) {
+    return "";
   }
-  QuicStringPiece authority = it->second;
+  url.append(it->second.as_string());
 
-  // RFC 7540, Section 8.1.2.3 requires that the ":path" pseudo-header MUST
-  // NOT be empty for "http" or "https" URIs;
-  //
-  // However, to ensure the scheme is consistently canonicalized, that check
-  // is deferred to implementations in QuicUrlUtils::GetPushPromiseUrl().
   it = headers.find(":path");
   if (it == headers.end()) {
-    return string();
+    return "";
   }
-  QuicStringPiece path = it->second;
-
-  return QuicUrlUtils::GetPushPromiseUrl(scheme, authority, path);
+  url.append(it->second.as_string());
+  return url;
 }
 
 // static
 string SpdyUtils::GetHostNameFromHeaderBlock(const SpdyHeaderBlock& headers) {
   // TODO(fayang): Consider just checking out the value of the ":authority" key
   // in headers.
-  return QuicUrlUtils::HostName(GetPromisedUrlFromHeaderBlock(headers));
+  return QuicUrlUtils::HostName(GetUrlFromHeaderBlock(headers));
 }
 
 // static
 bool SpdyUtils::UrlIsValid(const SpdyHeaderBlock& headers) {
-  string url(GetPromisedUrlFromHeaderBlock(headers));
+  string url(GetUrlFromHeaderBlock(headers));
   return !url.empty() && QuicUrlUtils::IsValidUrl(url);
 }
 

net/quic/core/spdy_utils.h

@@ -35,17 +35,14 @@ class QUIC_EXPORT_PRIVATE SpdyUtils {
                                       size_t* final_byte_offset,
                                       SpdyHeaderBlock* trailers);
 
-  // Returns a canonicalized URL composed from the :scheme, :authority, and
-  // :path headers of a PUSH_PROMISE. Returns empty string if the headers do not
-  // conform to HTTP/2 spec or if the ":method" header contains a forbidden
-  // method for PUSH_PROMISE.
-  static std::string GetPromisedUrlFromHeaderBlock(
-      const SpdyHeaderBlock& headers);
+  // Returns URL composed from scheme, authority, and path header
+  // values, or empty string if any of those fields are missing.
+  static std::string GetUrlFromHeaderBlock(const SpdyHeaderBlock& headers);
 
   // Returns hostname, or empty string if missing.
   static std::string GetHostNameFromHeaderBlock(const SpdyHeaderBlock& headers);
 
-  // Returns true if result of |GetPromisedUrlFromHeaderBlock()| is non-empty
+  // Returns true if result of |GetUrlFromHeaderBlock()| is non-empty
   // and is a well-formed URL.
   static bool UrlIsValid(const SpdyHeaderBlock& headers);
 

net/quic/core/spdy_utils_test.cc

@@ -300,136 +300,28 @@ TEST_F(CopyAndValidateTrailers, DuplicateCookies) {
           Pair("key", "value")));
 }
 
-using GetPromisedUrlFromHeaderBlock = QuicTest;
+using GetUrlFromHeaderBlock = QuicTest;
 
-TEST_F(GetPromisedUrlFromHeaderBlock, Basic) {
+TEST_F(GetUrlFromHeaderBlock, Basic) {
   SpdyHeaderBlock headers;
-  headers[":method"] = "GET";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
+  EXPECT_EQ(SpdyUtils::GetUrlFromHeaderBlock(headers), "");
   headers[":scheme"] = "https";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
+  EXPECT_EQ(SpdyUtils::GetUrlFromHeaderBlock(headers), "");
   headers[":authority"] = "www.google.com";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
+  EXPECT_EQ(SpdyUtils::GetUrlFromHeaderBlock(headers), "");
   headers[":path"] = "/index.html";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers),
+  EXPECT_EQ(SpdyUtils::GetUrlFromHeaderBlock(headers),
             "https://www.google.com/index.html");
   headers["key1"] = "value1";
   headers["key2"] = "value2";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers),
+  EXPECT_EQ(SpdyUtils::GetUrlFromHeaderBlock(headers),
             "https://www.google.com/index.html");
 }
 
-TEST_F(GetPromisedUrlFromHeaderBlock, Connect) {
-  SpdyHeaderBlock headers;
-  headers[":method"] = "CONNECT";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
-  headers[":authority"] = "www.google.com";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
-  headers[":scheme"] = "https";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
-  headers[":path"] = "https";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
-}
-
-TEST_F(GetPromisedUrlFromHeaderBlock, UnsupportedFileScheme) {
-  SpdyHeaderBlock headers;
-  headers[":method"] = "GET";
-  headers[":scheme"] = "file";
-  headers[":authority"] = "localhost";
-  headers[":path"] = "/etc/password";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
-  headers[":authority"] = "";
-  headers[":path"] = "/C:/Windows/System32/Config/";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
-}
-
-TEST_F(GetPromisedUrlFromHeaderBlock, EmptySchemeAndInvalidAuthority) {
-  SpdyHeaderBlock headers;
-  headers[":method"] = "GET";
-  headers[":scheme"] = "";
-  headers[":authority"] = "https://www.google.com";
-  headers[":path"] = "/";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
-}
-
-TEST_F(GetPromisedUrlFromHeaderBlock, SchemeWithAuthority) {
-  SpdyHeaderBlock headers;
-  headers[":method"] = "GET";
-  headers[":scheme"] = "https://www.google.com";
-  headers[":authority"] = "www.google.com";
-  headers[":path"] = "/";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
-}
-
-TEST_F(GetPromisedUrlFromHeaderBlock, InvalidScheme) {
-  SpdyHeaderBlock headers;
-  headers[":method"] = "GET";
-  headers[":scheme"] = "https://";
-  headers[":authority"] = "www.google.com";
-  headers[":path"] = "/";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
-}
-
-TEST_F(GetPromisedUrlFromHeaderBlock, EmptyAuthority) {
-  SpdyHeaderBlock headers;
-  headers[":method"] = "GET";
-  headers[":scheme"] = "https";
-  headers[":authority"] = "";
-  headers[":path"] = "/";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
-}
-
-TEST_F(GetPromisedUrlFromHeaderBlock, EmptyAuthorityAndInvalidPath) {
-  SpdyHeaderBlock headers;
-  headers[":method"] = "GET";
-  headers[":scheme"] = "https";
-  headers[":authority"] = "";
-  headers[":path"] = "www.google.com/";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
-}
-
-TEST_F(GetPromisedUrlFromHeaderBlock, AuthorityWithPath) {
-  SpdyHeaderBlock headers;
-  headers[":method"] = "GET";
-  headers[":scheme"] = "https";
-  headers[":authority"] = "www.google.com/";
-  headers[":path"] = "/";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
-}
-
-TEST_F(GetPromisedUrlFromHeaderBlock, EmptyPath) {
-  SpdyHeaderBlock headers;
-  headers[":method"] = "GET";
-  headers[":scheme"] = "https";
-  headers[":authority"] = "www.google.com";
-  headers[":path"] = "";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
-}
-
-TEST_F(GetPromisedUrlFromHeaderBlock, InvalidPath) {
-  SpdyHeaderBlock headers;
-  headers[":method"] = "GET";
-  headers[":scheme"] = "https";
-  headers[":authority"] = "www.google";
-  headers[":path"] = ".com/";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers), "");
-}
-
-TEST_F(GetPromisedUrlFromHeaderBlock, Canonicalize) {
-  SpdyHeaderBlock headers;
-  headers[":method"] = "GET";
-  headers[":scheme"] = "hTtPs";
-  headers[":authority"] = "Www.gOo-Gle.Com:000003278";
-  headers[":path"] = "/pAth/To/reSOurce";
-  EXPECT_EQ(SpdyUtils::GetPromisedUrlFromHeaderBlock(headers),
-            "https://www.goo-gle.com:3278/pAth/To/reSOurce");
-}
-
 using GetHostNameFromHeaderBlock = QuicTest;
 
 TEST_F(GetHostNameFromHeaderBlock, NormalUsage) {
   SpdyHeaderBlock headers;
-  headers[":method"] = "GET";
   EXPECT_EQ(SpdyUtils::GetHostNameFromHeaderBlock(headers), "");
   headers[":scheme"] = "https";
   EXPECT_EQ(SpdyUtils::GetHostNameFromHeaderBlock(headers), "");

net/quic/platform/api/quic_url_utils.cc

@@ -18,11 +18,4 @@ bool QuicUrlUtils::IsValidUrl(QuicStringPiece url) {
   return QuicUrlUtilsImpl::IsValidUrl(url);
 }
 
-// static
-string QuicUrlUtils::GetPushPromiseUrl(QuicStringPiece scheme,
-                                       QuicStringPiece authority,
-                                       QuicStringPiece path) {
-  return QuicUrlUtilsImpl::GetPushPromiseUrl(scheme, authority, path);
-}
-
 }  // namespace net

net/quic/platform/api/quic_url_utils.h

@@ -22,13 +22,6 @@ class QUIC_EXPORT_PRIVATE QuicUrlUtils {
   // (e.g. greater than 65535).
   static bool IsValidUrl(QuicStringPiece url);
 
-  // Returns a canonical, valid URL for a PUSH_PROMISE with the specified
-  // ":scheme", ":authority", and ":path" header fields, or an empty
-  // string if the resulting URL is not valid or supported.
-  static std::string GetPushPromiseUrl(QuicStringPiece scheme,
-                                       QuicStringPiece authority,
-                                       QuicStringPiece path);
-
  private:
   DISALLOW_COPY_AND_ASSIGN(QuicUrlUtils);
 };

net/quic/platform/api/quic_url_utils_test.cc

-// Copyright (c) 2018 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/quic/platform/api/quic_url_utils.h"
-
-#include <cstdint>
-
-#include "net/quic/platform/api/quic_arraysize.h"
-#include "net/quic/platform/api/quic_test.h"
-
-using std::string;
-
-namespace net {
-namespace test {
-namespace {
-
-class QuicUrlUtilsTest : public QuicTest {};
-
-TEST_F(QuicUrlUtilsTest, GetPushPromiseUrl) {
-  // Test acception/rejection of various input combinations.
-  // |input_headers| is an array of pairs. The first value of each pair is a
-  // string that will be used as one of the inputs of GetPushPromiseUrl(). The
-  // second value of each pair is a bitfield where the lowest 3 bits indicate
-  // for which headers that string is valid (in a PUSH_PROMISE). For example,
-  // the string "http" would be valid for both the ":scheme" and ":authority"
-  // headers, so the bitfield paired with it is set to SCHEME | AUTH.
-  const unsigned char SCHEME = (1u << 0);
-  const unsigned char AUTH = (1u << 1);
-  const unsigned char PATH = (1u << 2);
-  const std::pair<const char*, unsigned char> input_headers[] = {
-      {"http", SCHEME | AUTH},
-      {"https", SCHEME | AUTH},
-      {"hTtP", SCHEME | AUTH},
-      {"HTTPS", SCHEME | AUTH},
-      {"www.google.com", AUTH},
-      {"90af90e0", AUTH},
-      {"12foo%20-bar:00001233", AUTH},
-      {"GOO\u200b\u2060\ufeffgoo", AUTH},
-      {"192.168.0.5", AUTH},
-      {"[::ffff:192.168.0.1.]", AUTH},
-      {"http:", AUTH},
-      {"bife l", AUTH},
-      {"/", PATH},
-      {"/foo/bar/baz", PATH},
-      {"/%20-2DVdkj.cie/foe_.iif/", PATH},
-      {"http://", 0},
-      {":443", 0},
-      {":80/eddd", 0},
-      {"google.com:-0", 0},
-      {"google.com:65536", 0},
-      {"http://google.com", 0},
-      {"http://google.com:39", 0},
-      {"//google.com/foo", 0},
-      {".com/", 0},
-      {"http://www.google.com/", 0},
-      {"http://foo:439", 0},
-      {"[::ffff:192.168", 0},
-      {"]/", 0},
-      {"//", 0}};
-  for (size_t i = 0; i < QUIC_ARRAYSIZE(input_headers); ++i) {
-    bool should_accept = (input_headers[i].second & SCHEME);
-    for (size_t j = 0; j < QUIC_ARRAYSIZE(input_headers); ++j) {
-      bool should_accept_2 = should_accept && (input_headers[j].second & AUTH);
-      for (size_t k = 0; k < QUIC_ARRAYSIZE(input_headers); ++k) {
-        // |should_accept_3| indicates whether or not GetPushPromiseUrl() is
-        // expected to accept this input combination.
-        bool should_accept_3 =
-            should_accept_2 && (input_headers[k].second & PATH);
-
-        std::string url = QuicUrlUtilsImpl::GetPushPromiseUrl(
-            input_headers[i].first, input_headers[j].first,
-            input_headers[k].first);
-
-        ::testing::AssertionResult result = ::testing::AssertionSuccess();
-        if (url.empty() == should_accept_3) {
-          result = ::testing::AssertionFailure()
-                   << "GetPushPromiseUrl() accepted/rejected the inputs when "
-                      "it shouldn't have."
-                   << std::endl
-                   << "     scheme: " << input_headers[i].first << std::endl
-                   << "  authority: " << input_headers[j].first << std::endl
-                   << "       path: " << input_headers[k].first << std::endl
-                   << "Output: " << url << std::endl;
-        }
-        ASSERT_TRUE(result);
-      }
-    }
-  }
-
-  // Test canonicalization of various valid inputs.
-  EXPECT_EQ("http://www.google.com/",
-            QuicUrlUtilsImpl::GetPushPromiseUrl("http", "www.google.com", "/"));
-  EXPECT_EQ("https://www.goo-gle.com/fOOo/baRR",
-            QuicUrlUtilsImpl::GetPushPromiseUrl("hTtPs", "wWw.gOo-gLE.cOm",
-                                                "/fOOo/baRR"));
-  EXPECT_EQ(
-      "https://foo%20bar/foo/bar/baz",
-      QuicUrlUtilsImpl::GetPushPromiseUrl("https", "foo bar", "/foo/bar/baz"));
-  EXPECT_EQ("http://foo.com:70/e/", QuicUrlUtilsImpl::GetPushPromiseUrl(
-                                        "http", "foo.com:0000070", "/e/"));
-  EXPECT_EQ("http://192.168.0.1:70/e/",
-            QuicUrlUtilsImpl::GetPushPromiseUrl("http", "0300.0250.00.01:0070",
-                                                "/e/"));
-  EXPECT_EQ("http://192.168.0.1/e/",
-            QuicUrlUtilsImpl::GetPushPromiseUrl("http", "0xC0a80001", "/e/"));
-  EXPECT_EQ("http://[::c0a8:1]/", QuicUrlUtilsImpl::GetPushPromiseUrl(
-                                      "http", "[::192.168.0.1]", "/"));
-  EXPECT_EQ("https://[::ffff:c0a8:1]/",
-            QuicUrlUtilsImpl::GetPushPromiseUrl(
-                "https", "[::ffff:0xC0.0Xa8.0x0.0x1]", "/"));
-}
-
-};  // namespace
-};  // namespace test
-};  // namespace net
\ No newline at end of file

net/quic/platform/impl/quic_url_utils_impl.cc

@@ -6,10 +6,12 @@
 
 #include "url/gurl.h"
 
+using std::string;
+
 namespace net {
 
 // static
-std::string QuicUrlUtilsImpl::HostName(QuicStringPiece url) {
+string QuicUrlUtilsImpl::HostName(QuicStringPiece url) {
   return GURL(url).host();
 }
 
@@ -18,138 +20,4 @@ bool QuicUrlUtilsImpl::IsValidUrl(QuicStringPiece url) {
   return GURL(url).is_valid();
 }
 
-// static
-std::string QuicUrlUtilsImpl::GetPushPromiseUrl(QuicStringPiece scheme,
-                                                QuicStringPiece authority,
-                                                QuicStringPiece path) {
-  // RFC 7540, Section 8.1.2.3: The ":path" pseudo-header field includes the
-  // path and query parts of the target URI (the "path-absolute" production
-  // and optionally a '?' character followed by the "query" production (see
-  // Sections 3.3 and 3.4 of RFC3986). A request in asterisk form includes the
-  // value '*' for the ":path" pseudo-header field.
-  //
-  // This pseudo-header field MUST NOT be empty for "http" or "https" URIs;
-  // "http" or "https" URIs that do not contain a path MUST include a value of
-  // '/'. The exception to this rule is an OPTIONS request for an "http" or
-  // "https" URI that does not include a path component; these MUST include a
-  // ":path" pseudo-header with a value of '*' (see RFC7230, Section 5.3.4).
-  //
-  // In addition to the above restriction from RFC 7540, note that RFC3986
-  // defines the "path-absolute" construction as starting with "/" but not "//".
-  //
-  // RFC 7540, Section  8.2.1:  The header fields in PUSH_PROMISE and any
-  // subsequent CONTINUATION frames MUST be a valid and complete set of request
-  // header fields (Section 8.1.2.3).  The server MUST include a method in the
-  // ":method" pseudo-header field that is safe and cacheable.
-  //
-  // RFC 7231, Section  4.2.1:
-  // ... this specification defines GET, HEAD, and POST as cacheable, ...
-  //
-  // Since the OPTIONS method is not cacheable, it cannot be the method of a
-  // PUSH_PROMISE. Therefore, the exception mentioned in RFC 7540, Section
-  // 8.1.2.3 about OPTIONS requests does not apply here (i.e. ":path" cannot be
-  // "*").
-  if (path.empty() || path[0] != '/' || (path.size() >= 2 && path[1] == '/')) {
-    return std::string();
-  }
-
-  // Validate the scheme; this is to ensure a scheme of "foo://bar" is not
-  // parsed as a URL of "foo://bar://baz" when combined with a host of "baz".
-  std::string canonical_scheme;
-  url::StdStringCanonOutput canon_output(&canonical_scheme);
-  url::Component canon_component;
-  url::Component scheme_component(0, scheme.size());
-
-  if (!url::CanonicalizeScheme(scheme.data(), scheme_component, &canon_output,
-                               &canon_component) ||
-      !canon_component.is_nonempty() || canon_component.begin != 0) {
-    return std::string();
-  }
-  canonical_scheme.resize(canon_component.len + 1);
-
-  // Validate the authority; this is to ensure an authority such as
-  // "host/path" is not accepted, as when combined with a scheme like
-  // "http://", could result in a URL of "http://host/path".
-  url::Component auth_component(0, authority.size());
-  url::Component username_component;
-  url::Component password_component;
-  url::Component host_component;
-  url::Component port_component;
-
-  url::ParseAuthority(authority.data(), auth_component, &username_component,
-                      &password_component, &host_component, &port_component);
-
-  // RFC 7540, Section 8.1.2.3: The authority MUST NOT include the deprecated
-  // "userinfo" subcomponent for "http" or "https" schemed URIs.
-  //
-  // Note: Although |canonical_scheme| has not yet been checked for that, as
-  // it is performed later in processing, only "http" and "https" schemed
-  // URIs are supported for PUSH.
-  if (username_component.is_valid() || password_component.is_valid()) {
-    return std::string();
-  }
-
-  // Failed parsing or no host present. ParseAuthority() will ensure that
-  // host_component + port_component cover the entire string, if
-  // username_component and password_component are not present.
-  if (!host_component.is_nonempty()) {
-    return std::string();
-  }
-
-  // Validate the port (if present; it's optional).
-  int parsed_port_number = url::PORT_INVALID;
-  if (port_component.is_nonempty()) {
-    parsed_port_number = url::ParsePort(authority.data(), port_component);
-    if (parsed_port_number < 0 && parsed_port_number != url::PORT_UNSPECIFIED) {
-      return std::string();
-    }
-  }
-
-  // Validate the host by attempting to canoncalize it. Invalid characters
-  // will result in a canonicalization failure (e.g. '/')
-  std::string canon_host;
-  canon_output = url::StdStringCanonOutput(&canon_host);
-  canon_component.reset();
-  if (!url::CanonicalizeHost(authority.data(), host_component, &canon_output,
-                             &canon_component) ||
-      !canon_component.is_nonempty() || canon_component.begin != 0) {
-    return std::string();
-  }
-
-  // At this point, "authority" has been validated to either be of the form
-  // 'host:port' or 'host', with 'host' being a valid domain or IP address,
-  // and 'port' (if present), being a valid port. Attempt to construct a
-  // URL of just the (scheme, host, port), which should be safe and will not
-  // result in ambiguous parsing.
-  //
-  // This also enforces that all PUSHed URLs are either HTTP or HTTPS-schemed
-  // URIs, consistent with the other restrictions enforced above.
-  //
-  // Note: url::CanonicalizeScheme() will have added the ':' to
-  // |canonical_scheme|.
-  GURL origin_url(canonical_scheme + "//" + authority.as_string());
-  if (!origin_url.is_valid() || !origin_url.SchemeIsHTTPOrHTTPS() ||
-      // The following checks are merely defense in depth.
-      origin_url.SchemeIsSuborigin() || origin_url.has_username() ||
-      origin_url.has_password() ||
-      (origin_url.has_path() && origin_url.path_piece() != "/") ||
-      origin_url.has_query() || origin_url.has_ref()) {
-    return std::string();
-  }
-
-  // Attempt to parse the path.
-  std::string spec = origin_url.GetWithEmptyPath().spec();
-  spec.pop_back();  // Remove the '/', as ":path" must contain it.
-  spec.append(path.as_string());
-
-  // Attempt to parse the full URL, with the path as well. Ensure there is no
-  // fragment to the query.
-  GURL full_url(spec);
-  if (!full_url.is_valid() || full_url.has_ref()) {
-    return std::string();
-  }
-
-  return full_url.spec();
-}
-
 }  // namespace net

net/quic/platform/impl/quic_url_utils_impl.h

@@ -22,13 +22,6 @@ class QUIC_EXPORT_PRIVATE QuicUrlUtilsImpl {
   // (e.g. greater than 65535).
   static bool IsValidUrl(QuicStringPiece url);
 
-  // Returns a canonical, valid URL for a PUSH_PROMISE with the specified
-  // ":scheme", ":authority", and ":path" header fields, or an empty
-  // string if the resulting URL is not valid or supported.
-  static std::string GetPushPromiseUrl(QuicStringPiece scheme,
-                                       QuicStringPiece authority,
-                                       QuicStringPiece path);
-
  private:
   DISALLOW_COPY_AND_ASSIGN(QuicUrlUtilsImpl);
 };

net/tools/quic/quic_spdy_client_session_test.cc

@@ -96,7 +96,7 @@ class QuicSpdyClientSessionTest : public QuicTestWithParam<ParsedQuicVersion> {
     push_promise_[":version"] = "HTTP/1.1";
     push_promise_[":method"] = "GET";
     push_promise_[":scheme"] = "https";
-    promise_url_ = SpdyUtils::GetPromisedUrlFromHeaderBlock(push_promise_);
+    promise_url_ = SpdyUtils::GetUrlFromHeaderBlock(push_promise_);
     promised_stream_id_ =
         QuicSpdySessionPeer::GetNthServerInitiatedStreamId(*session_, 0);
     associated_stream_id_ =
@@ -502,7 +502,7 @@ TEST_P(QuicSpdyClientSessionTest, ReceivingPromiseEnhanceYourCalm) {
     session_->HandlePromised(associated_stream_id_, id, push_promise_);
 
     // Verify that the promise is in the unclaimed streams map.
-    string promise_url(SpdyUtils::GetPromisedUrlFromHeaderBlock(push_promise_));
+    string promise_url(SpdyUtils::GetUrlFromHeaderBlock(push_promise_));
     EXPECT_NE(session_->GetPromisedByUrl(promise_url), nullptr);
     EXPECT_NE(session_->GetPromisedById(id), nullptr);
   }
@@ -521,7 +521,7 @@ TEST_P(QuicSpdyClientSessionTest, ReceivingPromiseEnhanceYourCalm) {
   session_->HandlePromised(associated_stream_id_, id, push_promise_);
 
   // Verify that the promise was not created.
-  string promise_url(SpdyUtils::GetPromisedUrlFromHeaderBlock(push_promise_));
+  string promise_url(SpdyUtils::GetUrlFromHeaderBlock(push_promise_));
   EXPECT_EQ(session_->GetPromisedById(id), nullptr);
   EXPECT_EQ(session_->GetPromisedByUrl(promise_url), nullptr);
 }