chromeos::Uri: Add method SetPort(const std::string&)

Adds a method that allows for setting a new port value directly from
a string. Also add missing validation for the old SetPort(int) method.

BUG=none
TEST=on my laptop, a dedicated unit test was added

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

chromeos/printing/uri.cc

@@ -256,6 +256,10 @@ bool Uri::SetPort(int val) {
   return pim_->SavePort(val);
 }
 
+bool Uri::SetPort(const std::string& port) {
+  return pim_->ParsePort(port.begin(), port.end());
+}
+
 std::string Uri::GetUserinfo() const {
   return pim_->userinfo();
 }

chromeos/printing/uri.h

@@ -307,6 +307,9 @@ class CHROMEOS_EXPORT Uri {
   // Returns false when |port| is invalid. In this case, the current port is
   // not modified.
   bool SetPort(int port);
+  // A version of the method above for a string parameter. Empty string means
+  // "not-specified" and has the same effect as passing -1 to the method above.
+  bool SetPort(const std::string& port);
 
   // These methods return values of components. There is no %-escaped sequences
   // and returned string may contain UTF-8 characters.

chromeos/printing/uri_impl.cc

@@ -73,26 +73,6 @@ bool ParseCharacter(const Iter& end, Iter* current, char* out) {
   return true;
 }
 
-// Tries to parse the input string |begin|-|end| as a Port number.
-// For a number from range [0,65535] it returns this number.
-// For an empty string it returns -1 (not specified).
-// For all other inputs it returns -2 (invalid Port number).
-// The input requirement: |begin| <= |end|.
-int ParsePort(const Iter& begin, const Iter& end) {
-  if (begin == end)
-    return kPortUnspecified;
-  int number = 0;
-  for (Iter it = begin; it < end; ++it) {
-    if (!base::IsAsciiDigit(*it))
-      return kPortInvalid;
-    number *= 10;
-    number += *it - '0';
-    if (number > kPortMaxNumber)
-      return kPortInvalid;
-  }
-  return number;
-}
-
 // Helper struct for the function below.
 class Comparator {
  public:
@@ -201,7 +181,7 @@ bool Uri::Pim::SavePort(int value) {
   parser_error_.status = ParserStatus::kNoErrors;
   parser_error_.parsed_strings = 0;
   parser_error_.parsed_chars = 0;
-  if (value == kPortInvalid) {
+  if (value < -1 || value > 65535) {
     parser_error_.status = ParserStatus::kInvalidPortNumber;
     return false;
   }
@@ -343,7 +323,7 @@ bool Uri::Pim::ParseAuthority(const Iter& begin, const Iter& end) {
   // Parse and save Port.
   if (it2 != end) {
     ++it2;  // omit the ':' character
-    if (it2 < end && !SavePort(ParsePort(it2, end))) {
+    if (!ParsePort(it2, end)) {
       parser_error_.parsed_chars += it2 - begin;
       return false;
     }
@@ -351,6 +331,21 @@ bool Uri::Pim::ParseAuthority(const Iter& begin, const Iter& end) {
   return true;
 }
 
+bool Uri::Pim::ParsePort(const Iter& begin, const Iter& end) {
+  if (begin == end)
+    return SavePort(kPortUnspecified);
+  int number = 0;
+  for (Iter it = begin; it < end; ++it) {
+    if (!base::IsAsciiDigit(*it))
+      return SavePort(kPortInvalid);
+    number *= 10;
+    number += *it - '0';
+    if (number > kPortMaxNumber)
+      return SavePort(kPortInvalid);
+  }
+  return SavePort(number);
+}
+
 bool Uri::Pim::ParsePath(const Iter& begin, const Iter& end) {
   // This holds Path's segments.
   std::vector<std::string> path;

chromeos/printing/uri_impl.h

@@ -38,9 +38,11 @@ class Uri::Pim {
   //    default port number, the Port is set to this default value
   //  * Authority: this is Userinfo + Host + Port, see description in uri.h for
   //    the grammar
+  //  * Port: an empty string means -1, see the method SavePort(int) below
   //  * Path: the input string must be empty or starts from '/'
   bool ParseScheme(const Iter& begin, const Iter& end);
   bool ParseAuthority(const Iter& begin, const Iter& end);
+  bool ParsePort(const Iter& begin, const Iter& end);
   bool ParsePath(const Iter& begin, const Iter& end);
   bool ParseQuery(const Iter& begin, const Iter& end);
   bool ParseFragment(const Iter& begin, const Iter& end);

chromeos/printing/uri_unittest.cc

@@ -141,6 +141,25 @@ TEST(UriTest, EncodingInHostComponent) {
             "example._!_%40_%23_$_%25_%5E_._!_%40_%23_$_%25_%5E_");
 }
 
+TEST(UriTest, SetPortFromString) {
+  Uri uri1;
+  Uri uri2;
+
+  EXPECT_TRUE(uri1.SetPort(1234));
+  EXPECT_TRUE(uri2.SetPort("1234"));
+  EXPECT_EQ(uri1, uri2);
+
+  // -1 and empty string mean "unspecified port".
+  EXPECT_TRUE(uri1.SetPort(-1));
+  EXPECT_TRUE(uri2.SetPort(""));
+  EXPECT_EQ(uri1, uri2);
+
+  EXPECT_FALSE(uri2.SetPort("65536"));
+  EXPECT_FALSE(uri2.SetPort("-2"));
+  EXPECT_FALSE(uri2.SetPort(" 2133"));
+  EXPECT_FALSE(uri2.SetPort("0x123"));
+}
+
 TEST(UriTest, UriWithAllPrintableASCII) {
   Uri uri;
   std::string host = kPrintableASCII;