Modify ParseUri to validate uris.

Parse uri verifies that if a port is provided, it has a valid value.
We accept empty paths.  We also validate that there is a valid scheme
value is provided if it is found.  These requirements are now validated
via unit test.

Bug: 844521
Change-Id: I4fb28a5bba8e6bea9ac9a0bb2e6b4de21351d892
Reviewed-on: https://chromium-review.googlesource.com/1066949
Commit-Queue: Sean Kau <skau@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560869}

chromeos/BUILD.gn

@@ -763,6 +763,7 @@ test("chromeos_unittests") {
     "printing/ppd_cache_unittest.cc",
     "printing/ppd_line_reader_unittest.cc",
     "printing/ppd_provider_unittest.cc",
+    "printing/printer_configuration_unittest.cc",
     "printing/printer_translator_unittest.cc",
     "process_proxy/process_output_watcher_unittest.cc",
     "process_proxy/process_proxy_unittest.cc",

chromeos/printing/printer_configuration.cc

@@ -18,23 +18,45 @@
 
 namespace chromeos {
 
+// Returns true if the scheme is both valid and non-empty.
+bool IsSchemeValid(const url::Parsed& parsed) {
+  return parsed.scheme.is_valid() && parsed.scheme.is_nonempty();
+}
+
+// Returns true if |parsed| contains a valid port. A valid port is one that
+// either contains a valid value or is completely missing.
+bool IsPortValid(const url::Parsed& parsed) {
+  // A length of -1 indicates that the port is missing.
+  return parsed.port.len == -1 ||
+         (parsed.port.is_valid() && parsed.port.is_nonempty());
+}
+
+// Returns |printer_uri| broken into components if it represents a valid uri. A
+// valid uri contains a scheme, host, and a valid or missing port number.
+// Optionally, the uri contains a path.
 base::Optional<UriComponents> ParseUri(const std::string& printer_uri) {
   const char* uri_ptr = printer_uri.c_str();
   url::Parsed parsed;
   url::ParseStandardURL(uri_ptr, printer_uri.length(), &parsed);
-  if (!parsed.scheme.is_valid() || !parsed.host.is_valid() ||
-      !parsed.path.is_valid()) {
+  if (!IsSchemeValid(parsed) || !parsed.host.is_valid() ||
+      !IsPortValid(parsed)) {
+    LOG(WARNING) << "Could not parse printer uri";
     return {};
   }
   base::StringPiece scheme(&uri_ptr[parsed.scheme.begin], parsed.scheme.len);
   base::StringPiece host(&uri_ptr[parsed.host.begin], parsed.host.len);
-  base::StringPiece path(&uri_ptr[parsed.path.begin], parsed.path.len);
+  base::StringPiece path =
+      parsed.path.is_valid()
+          ? base::StringPiece(&uri_ptr[parsed.path.begin], parsed.path.len)
+          : "";
 
-  bool encrypted = scheme != kIppScheme;
   int port = ParsePort(uri_ptr, parsed.port);
+  if (port == url::SpecialPort::PORT_INVALID) {
+    LOG(WARNING) << "Port is invalid";
+    return {};
+  }
   // Port not specified.
-  if (port == url::SpecialPort::PORT_UNSPECIFIED ||
-      port == url::SpecialPort::PORT_INVALID) {
+  if (port == url::SpecialPort::PORT_UNSPECIFIED) {
     if (scheme == kIppScheme) {
       port = kIppPort;
     } else if (scheme == kIppsScheme) {
@@ -42,6 +64,7 @@ base::Optional<UriComponents> ParseUri(const std::string& printer_uri) {
     }
   }
 
+  bool encrypted = scheme != kIppScheme;
   return base::Optional<UriComponents>(base::in_place, encrypted,
                                        scheme.as_string(), host.as_string(),
                                        port, path.as_string());

chromeos/printing/printer_configuration_unittest.cc

+// Copyright 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 "chromeos/printing/printer_configuration.h"
+#include "chromeos/printing/uri_components.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+TEST(PrinterConfigurationTest, EmptyScheme) {
+  auto result = chromeos::ParseUri("://hostname.com/");
+  EXPECT_FALSE(result.has_value());
+}
+
+TEST(PrinterConfigurationTest, JustScheme) {
+  auto result = chromeos::ParseUri("ipps://");
+  EXPECT_FALSE(result.has_value());
+}
+
+TEST(PrinterConfigurationTest, InvalidUriDanglingPort) {
+  auto result = chromeos::ParseUri("ipp://192.168.1.1:");
+  EXPECT_FALSE(result.has_value());
+}
+
+TEST(PrinterConfigurationTest, InvalidUriEmptyPort) {
+  auto result = chromeos::ParseUri("ipp://192.168.1.1:/printer");
+  EXPECT_FALSE(result.has_value());
+}
+
+TEST(PrinterConfigurationTest, InvalidPort) {
+  auto result = chromeos::ParseUri("ipp://1.2.3.4:abcd");
+  EXPECT_FALSE(result.has_value());
+}
+
+TEST(PrinterConfigurationTest, MissingScheme) {
+  auto result = chromeos::ParseUri("/10.2.12.3/");
+  EXPECT_FALSE(result.has_value());
+}
+
+TEST(PrinterConfigurationTest, ParseUriIp) {
+  auto result = chromeos::ParseUri("ipp://192.168.1.5");
+
+  ASSERT_TRUE(result.has_value());
+  EXPECT_EQ(result->scheme(), "ipp");
+  EXPECT_EQ(result->host(), "192.168.1.5");
+  EXPECT_TRUE(result->path().empty());
+}
+
+TEST(PrinterConfigurationTest, ParseUriPort) {
+  auto result = chromeos::ParseUri("ipp://1.2.3.4:4444");
+
+  ASSERT_TRUE(result.has_value());
+  EXPECT_EQ(result->port(), 4444);
+}
+
+TEST(PrinterConfigurationTest, ParseTrailingSlash) {
+  auto result = chromeos::ParseUri("ipp://1.2.3.4/");
+  ASSERT_TRUE(result.has_value());
+  EXPECT_EQ(result->path(), "/");
+  EXPECT_EQ(result->host(), "1.2.3.4");
+}
+
+TEST(PrinterConfigurationTest, ParseUriHostNameAndPort) {
+  auto result = chromeos::ParseUri("ipp://chromium.org:8");
+  ASSERT_TRUE(result.has_value());
+  EXPECT_EQ(result->port(), 8);
+  EXPECT_EQ(result->host(), "chromium.org");
+}
+
+TEST(PrinterConfigurationTest, ParseUriPathNoPort) {
+  auto result = chromeos::ParseUri("ipps://chromium.org/printers/printprint");
+  ASSERT_TRUE(result.has_value());
+  EXPECT_EQ(result->host(), "chromium.org");
+  EXPECT_EQ(result->path(), "/printers/printprint");
+}
+
+TEST(PrinterConfigurationTest, ParseUriSubdomainQueueAndPort) {
+  auto result =
+      chromeos::ParseUri("ipp://codesearch.chromium.org:1234/ipp/print");
+  ASSERT_TRUE(result.has_value());
+  EXPECT_EQ(result->host(), "codesearch.chromium.org");
+  EXPECT_EQ(result->port(), 1234);
+  EXPECT_EQ(result->path(), "/ipp/print");
+}
+
+}  // namespace