URI: Always add port in normalized form by default

This patch switches the default parameter in Uri::GetNormalized(bool)
method from false to true. As a result, simple call to
Uri::GetNormalized() will return normalized form including port number.
In the previous default (for parameter equals false), the port number
is omitted when equals to the default scheme's port number.

BUG=none
TEST=unittests on my workstation

Change-Id: Iee55e2f4eaa811c8b056e6b93d130eba922cc60b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2550587
Commit-Queue: Piotr Pawliczek <pawliczek@chromium.org>
Reviewed-by: Sean Kau <skau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829958}

chrome/browser/chromeos/printing/history/print_job_info_proto_conversions_unittest.cc

@@ -20,7 +20,7 @@ constexpr int kHeight = 420000;
 constexpr char kVendorId[] = "iso_a3_297x420mm";
 
 constexpr char kName[] = "name";
-constexpr char kUri[] = "ipp://192.168.1.5";
+constexpr char kUri[] = "ipp://192.168.1.5:631";
 
 constexpr char kTitle[] = "title";
 constexpr char kId[] = "id";

chrome/browser/chromeos/printing/print_management/print_job_info_mojom_conversions_unittest.cc

@@ -24,7 +24,7 @@ namespace mojom = printing::printing_manager::mojom;
 namespace {
 
 constexpr char kName[] = "name";
-constexpr char kUri[] = "ipp://192.168.1.5";
+constexpr char kUri[] = "ipp://192.168.1.5:631";
 constexpr char kTitle[] = "title";
 constexpr char kId[] = "id";
 constexpr char kPrinterId[] = "printerId";

chrome/browser/chromeos/printing/printer_configurer.cc

@@ -226,7 +226,7 @@ std::string PrinterConfigurer::SetupFingerprint(const Printer& printer) {
   base::MD5Context ctx;
   base::MD5Init(&ctx);
   base::MD5Update(&ctx, printer.id());
-  base::MD5Update(&ctx, printer.uri().GetNormalized());
+  base::MD5Update(&ctx, printer.uri().GetNormalized(false));
   base::MD5Update(&ctx, printer.ppd_reference().user_supplied_ppd_url);
   base::MD5Update(&ctx, printer.ppd_reference().effective_make_and_model);
   char autoconf = printer.ppd_reference().autoconf ? 1 : 0;

chrome/browser/chromeos/printing/server_printers_provider_unittest.cc

@@ -57,7 +57,7 @@ Printer Printer1() {
 Printer Printer2() {
   Printer printer("server-5da95e01216b1fe0ee1de25dc8d0a6e8");
   printer.set_display_name("Color Laser - Name");
-  std::string server("ipps://print-server.intranet.example.com");
+  std::string server("ipps://print-server.intranet.example.com:443");
   printer.set_print_server_uri(server);
   Uri url(
       "ipps://print-server.intranet.example.com/printers/Color Laser - Name");
@@ -151,12 +151,6 @@ TEST_F(ServerPrintersProviderTest, GetPrinters) {
   OnServersChanged(true, print_servers);
   task_environment_.RunUntilIdle();
 
-  auto first = server_printers_provider_->GetPrinters().front();
-
-  LOG(INFO) << first.printer.uri().GetNormalized()
-            << " server:" << first.printer.print_server_uri()
-            << " name:" << first.printer.display_name();
-
   EXPECT_THAT(server_printers_provider_->GetPrinters(),
               UnorderedElementsAre(PrinterMatcher(Printer1()),
                                    PrinterMatcher(Printer2())));

chrome/browser/chromeos/printing/specifics_translation_unittest.cc

@@ -19,7 +19,7 @@ constexpr char kDescription[] = "The green one";
 constexpr char kManufacturer[] = "Manufacturer";
 constexpr char kModel[] = "MODEL";
 constexpr char kMakeAndModel[] = "Manufacturer MODEL";
-constexpr char kUri[] = "ipps://notaprinter.chromium.org/ipp/print";
+constexpr char kUri[] = "ipps://notaprinter.chromium.org:123/ipp/print";
 constexpr char kUuid[] = "UUIDUUIDUUID";
 const base::Time kUpdateTime = base::Time::FromInternalValue(22114455660000);
 
@@ -49,7 +49,7 @@ TEST(SpecificsTranslationTest, SpecificsToPrinter) {
   EXPECT_EQ(kDisplayName, result->display_name());
   EXPECT_EQ(kDescription, result->description());
   EXPECT_EQ(kMakeAndModel, result->make_and_model());
-  EXPECT_EQ(kUri, result->uri().GetNormalized());
+  EXPECT_EQ(kUri, result->uri().GetNormalized(false));
   EXPECT_EQ(kUuid, result->uuid());
 
   EXPECT_EQ(kEffectiveMakeAndModel,
@@ -125,7 +125,7 @@ TEST(SpecificsTranslationTest, SpecificsToPrinterRoundTrip) {
   EXPECT_EQ(kManufacturer, result->manufacturer());
   EXPECT_EQ(kModel, result->model());
   EXPECT_EQ(kMakeAndModel, result->make_and_model());
-  EXPECT_EQ(kUri, result->uri().GetNormalized());
+  EXPECT_EQ(kUri, result->uri().GetNormalized(false));
   EXPECT_EQ(kUuid, result->uuid());
 
   EXPECT_TRUE(result->ppd_reference().effective_make_and_model.empty());

chrome/browser/chromeos/printing/synced_printers_manager_unittest.cc

@@ -124,7 +124,7 @@ TEST_F(SyncedPrintersManagerTest, UpdatePrinter) {
 
   auto printers = manager_->GetSavedPrinters();
   ASSERT_EQ(1U, printers.size());
-  EXPECT_EQ(kTestUri, printers[0].uri().GetNormalized());
+  EXPECT_EQ(kTestUri, printers[0].uri().GetNormalized(false));
 
   ExpectObservedPrinterIdsAre(observer.saved_printers(), {kTestPrinterId});
 }

chromeos/printing/uri.h

@@ -284,7 +284,7 @@ class CHROMEOS_EXPORT Uri {
   // If the Port is specified (GetPort() != -1) and |always_print_port| is set
   // to true, a Port number is always included in the returned URI (even when
   // it equals to a Scheme's default port number).
-  std::string GetNormalized(bool always_print_port = false) const;
+  std::string GetNormalized(bool always_print_port = true) const;
 
   // Returns true <=> whole URL has no UTF-8 characters.
   bool IsASCII() const;

chromeos/printing/uri_unittest.cc

@@ -55,7 +55,7 @@ void TestBuilder(const UriComponents& components,
   uri.SetUserinfo(components.userinfo);
   ASSERT_EQ(uri.GetLastParsingError().status, Uri::ParserStatus::kNoErrors);
   // Check URI.
-  EXPECT_EQ(uri.GetNormalized(), normalized_uri);
+  EXPECT_EQ(uri.GetNormalized(false), normalized_uri);
 }
 
 // Verifies that |input_uri| set as parameter in Uri constructor is parsed
@@ -79,7 +79,7 @@ void TestNormalization(const std::string& input_uri,
                        const std::string& normalized_uri) {
   Uri uri(input_uri);
   ASSERT_EQ(uri.GetLastParsingError().status, Uri::ParserStatus::kNoErrors);
-  EXPECT_EQ(uri.GetNormalized(), normalized_uri);
+  EXPECT_EQ(uri.GetNormalized(false), normalized_uri);
 }
 
 TEST(UriTest, DefaultConstructor) {

chromeos/printing/uri_unittest_consistency.cc

@@ -121,32 +121,36 @@ TEST_P(UriConsistencyTest, QueryBuilding) {
 // Build normalized URI from encoded components and make sure that it is
 // equal to the value returned by GetNormalized().
 TEST_P(UriConsistencyTest, UriBuilding) {
-  std::string expected_uri = uri_.GetScheme() + ":";
+  std::string scheme = uri_.GetScheme() + ":";
 
   // Build a part of URI called Authority (Userinfo@Host:Port).
   std::string authority_encoded;
   if (!uri_.GetUserinfoEncoded().empty())
     authority_encoded = uri_.GetUserinfoEncoded() + "@";
   authority_encoded += uri_.GetHostEncoded();
-  if (uri_.GetPort() != -1 &&
-      uri_.GetPort() != Uri::GetDefaultPort(uri_.GetScheme())) {
-    authority_encoded += ":" + base::NumberToString(uri_.GetPort());
+  std::string authority_with_port_encoded = authority_encoded;
+  if (uri_.GetPort() != -1) {
+    if (uri_.GetPort() != Uri::GetDefaultPort(uri_.GetScheme()))
+      authority_encoded += ":" + base::NumberToString(uri_.GetPort());
+    authority_with_port_encoded += ":" + base::NumberToString(uri_.GetPort());
   }
-  // If Authority is not empty, add it to |expected_uri|.
+  // If Authority is not empty, prefix it with "//".
   if (!authority_encoded.empty())
-    expected_uri += "//" + authority_encoded;
-
-  // Add Path and Query.
-  expected_uri += uri_.GetPathEncodedAsString();
-  const std::string expected_query = uri_.GetQueryEncodedAsString();
-  if (!expected_query.empty())
-    expected_uri += "?" + expected_query;
-
-  // Add Fragment to |expected_uri|.
+    authority_encoded = "//" + authority_encoded;
+  if (!authority_with_port_encoded.empty())
+    authority_with_port_encoded = "//" + authority_with_port_encoded;
+
+  // Build Path, Query and Fragment.
+  std::string path_query_fragment_encoded = uri_.GetPathEncodedAsString();
+  if (!uri_.GetQueryEncodedAsString().empty())
+    path_query_fragment_encoded += "?" + uri_.GetQueryEncodedAsString();
   if (!uri_.GetFragmentEncoded().empty())
-    expected_uri += "#" + uri_.GetFragmentEncoded();
+    path_query_fragment_encoded += "#" + uri_.GetFragmentEncoded();
 
-  EXPECT_EQ(uri_.GetNormalized(), expected_uri);
+  EXPECT_EQ(uri_.GetNormalized(false),
+            scheme + authority_encoded + path_query_fragment_encoded);
+  EXPECT_EQ(uri_.GetNormalized(true),
+            scheme + authority_with_port_encoded + path_query_fragment_encoded);
 }
 
 // Checks if the normalization algorithm is consistent.
@@ -154,7 +158,8 @@ TEST_P(UriConsistencyTest, Normalization) {
   // Normalization of normalized uri must not change it.
   Uri uri2(uri_.GetNormalized());
   EXPECT_EQ(uri2.GetLastParsingError().status, Uri::ParserStatus::kNoErrors);
-  EXPECT_EQ(uri_.GetNormalized(), uri2.GetNormalized());
+  EXPECT_EQ(uri_.GetNormalized(true), uri2.GetNormalized(true));
+  EXPECT_EQ(uri_.GetNormalized(false), uri2.GetNormalized(false));
 
   // Normalization of normalized Path must not change it.
   uri2.SetPathEncoded(uri_.GetPathEncodedAsString());