Simple URI: Bugfix for the Uri class

The detected problem was related to reporting position
in the input data where the last parser error occurred.

BUG=chromium:821497
TEST=on my workstation

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

chromeos/printing/uri.cc

@@ -245,7 +245,6 @@ std::string Uri::GetScheme() const {
 }
 
 bool Uri::SetScheme(const std::string& val) {
-  pim_->ResetParserError();
   return pim_->ParseScheme(val.begin(), val.end());
 }
 
@@ -254,7 +253,6 @@ int Uri::GetPort() const {
 }
 
 bool Uri::SetPort(int val) {
-  pim_->ResetParserError();
   return pim_->SavePort(val);
 }
 
@@ -323,54 +321,42 @@ std::string Uri::GetFragmentEncoded() const {
 }
 
 bool Uri::SetUserinfo(const std::string& val) {
-  pim_->ResetParserError();
   return pim_->SaveUserinfo<false>(val);
 }
 bool Uri::SetHost(const std::string& val) {
-  pim_->ResetParserError();
   return pim_->SaveHost<false>(val);
 }
 bool Uri::SetPath(const std::vector<std::string>& val) {
-  pim_->ResetParserError();
   return pim_->SavePath<false>(val);
 }
 bool Uri::SetQuery(
     const std::vector<std::pair<std::string, std::string>>& val) {
-  pim_->ResetParserError();
   return pim_->SaveQuery<false>(val);
 }
 bool Uri::SetFragment(const std::string& val) {
-  pim_->ResetParserError();
   return pim_->SaveFragment<false>(val);
 }
 
 bool Uri::SetUserinfoEncoded(const std::string& val) {
-  pim_->ResetParserError();
   return pim_->SaveUserinfo<true>(val);
 }
 bool Uri::SetHostEncoded(const std::string& val) {
-  pim_->ResetParserError();
   return pim_->SaveHost<true>(val);
 }
 bool Uri::SetPathEncoded(const std::vector<std::string>& val) {
-  pim_->ResetParserError();
   return pim_->SavePath<true>(val);
 }
 bool Uri::SetPathEncoded(const std::string& val) {
-  pim_->ResetParserError();
   return pim_->ParsePath(val.begin(), val.end());
 }
 bool Uri::SetQueryEncoded(
     const std::vector<std::pair<std::string, std::string>>& val) {
-  pim_->ResetParserError();
   return pim_->SaveQuery<true>(val);
 }
 bool Uri::SetQueryEncoded(const std::string& val) {
-  pim_->ResetParserError();
   return pim_->ParseQuery(val.begin(), val.end());
 }
 bool Uri::SetFragmentEncoded(const std::string& val) {
-  pim_->ResetParserError();
   return pim_->SaveFragment<true>(val);
 }
 

chromeos/printing/uri_impl.cc

@@ -171,11 +171,14 @@ bool Uri::Pim::ParseString(const Iter& begin,
       out->append(std::move(utf8_character));
     }
   }
+  ++(parser_error_.parsed_strings);
   return true;
 }
 
 template <bool encoded>
 bool Uri::Pim::SaveUserinfo(const std::string& val) {
+  parser_error_.status = ParserStatus::kNoErrors;
+  parser_error_.parsed_strings = 0;
   std::string out;
   if (!ParseString<encoded>(val.begin(), val.end(), &out))
     return false;
@@ -185,6 +188,8 @@ bool Uri::Pim::SaveUserinfo(const std::string& val) {
 
 template <bool encoded>
 bool Uri::Pim::SaveHost(const std::string& val) {
+  parser_error_.status = ParserStatus::kNoErrors;
+  parser_error_.parsed_strings = 0;
   std::string out;
   if (!ParseString<encoded, true>(val.begin(), val.end(), &out))
     return false;
@@ -193,8 +198,10 @@ bool Uri::Pim::SaveHost(const std::string& val) {
 }
 
 bool Uri::Pim::SavePort(int value) {
+  parser_error_.status = ParserStatus::kNoErrors;
+  parser_error_.parsed_strings = 0;
+  parser_error_.parsed_chars = 0;
   if (value == kPortInvalid) {
-    parser_error_.parsed_chars = 0;
     parser_error_.status = ParserStatus::kInvalidPortNumber;
     return false;
   }
@@ -207,6 +214,9 @@ bool Uri::Pim::SavePort(int value) {
 
 template <bool encoded>
 bool Uri::Pim::SavePath(const std::vector<std::string>& val) {
+  parser_error_.status = ParserStatus::kNoErrors;
+  parser_error_.parsed_strings = 0;
+  parser_error_.parsed_chars = 0;
   std::vector<std::string> out;
   out.reserve(val.size());
   for (size_t i = 0; i < val.size(); ++i) {
@@ -220,12 +230,13 @@ bool Uri::Pim::SavePath(const std::vector<std::string>& val) {
     } else if (segment == ".." && !out.empty() && out.back() != "..") {
       out.pop_back();
     } else if (segment.empty()) {
+      --parser_error_.parsed_strings;  // it was already counted
+      parser_error_.parsed_chars = 0;
       parser_error_.status = ParserStatus::kEmptySegmentInPath;
       return false;
     } else {
       out.push_back(std::move(segment));
     }
-    ++parser_error_.parsed_strings;
   }
   path_ = std::move(out);
   return true;
@@ -234,6 +245,9 @@ bool Uri::Pim::SavePath(const std::vector<std::string>& val) {
 template <bool encoded>
 bool Uri::Pim::SaveQuery(
     const std::vector<std::pair<std::string, std::string>>& val) {
+  parser_error_.status = ParserStatus::kNoErrors;
+  parser_error_.parsed_strings = 0;
+  parser_error_.parsed_chars = 0;
   std::vector<std::pair<std::string, std::string>> out(val.size());
   for (size_t i = 0; i < out.size(); ++i) {
     // Process parameter name.
@@ -242,23 +256,25 @@ bool Uri::Pim::SaveQuery(
     if (!ParseString<encoded>(it1, it2, &out[i].first, true))
       return false;
     if (out[i].first.empty()) {
+      --parser_error_.parsed_strings;  // it was already counted
+      parser_error_.parsed_chars = 0;
       parser_error_.status = ParserStatus::kEmptyParameterNameInQuery;
       return false;
     }
-    ++parser_error_.parsed_strings;
     // Process parameter value.
     it1 = val[i].second.begin();
     it2 = val[i].second.end();
     if (!ParseString<encoded>(it1, it2, &out[i].second, true))
       return false;
-    ++parser_error_.parsed_strings;
   }
   query_ = std::move(out);
   return true;
 }
 
 template <bool encoded>
 bool Uri::Pim::SaveFragment(const std::string& val) {
+  parser_error_.status = ParserStatus::kNoErrors;
+  parser_error_.parsed_strings = 0;
   std::string out;
   if (!ParseString<encoded>(val.begin(), val.end(), &out))
     return false;
@@ -267,6 +283,9 @@ bool Uri::Pim::SaveFragment(const std::string& val) {
 }
 
 bool Uri::Pim::ParseScheme(const Iter& begin, const Iter& end) {
+  parser_error_.status = ParserStatus::kNoErrors;
+  parser_error_.parsed_strings = 0;
+  parser_error_.parsed_chars = 0;
   // Special case for an empty string on the input.
   if (begin == end) {
     scheme_.clear();
@@ -392,6 +411,7 @@ bool Uri::Pim::ParseQuery(const Iter& begin, const Iter& end) {
 }
 
 bool Uri::Pim::ParseFragment(const Iter& begin, const Iter& end) {
+  parser_error_.parsed_strings = 0;
   std::string out;
   if (!ParseString<true>(begin, end, &out))
     return false;
@@ -400,6 +420,9 @@ bool Uri::Pim::ParseFragment(const Iter& begin, const Iter& end) {
 }
 
 bool Uri::Pim::ParseUri(const Iter& begin, const Iter end) {
+  parser_error_.status = ParserStatus::kNoErrors;
+  parser_error_.parsed_strings = 0;
+  parser_error_.parsed_chars = 0;
   Iter it1 = begin;
   // The Scheme component ends at the first colon (":").
   {

chromeos/printing/uri_impl.h

@@ -29,13 +29,6 @@ class Uri::Pim {
   Pim(const Pim&);
   ~Pim();
 
-  // Resets the internal field |parser_error|.
-  void ResetParserError() {
-    parser_error_.parsed_chars = 0;
-    parser_error_.parsed_strings = 0;
-    parser_error_.status = ParserStatus::kNoErrors;
-  }
-
   // These methods parse and normalize the corresponding component(s) from the
   // input string |begin|-|end|. Each component is saved only if successfully
   // parsed and verified. In case of an error, the field |parser_error| is set
@@ -60,7 +53,8 @@ class Uri::Pim {
 
   // This method fails (and return false) <=> |port| is smaller than -1 or
   // larger than 65535. If |port| == -1 and the current Scheme has a default
-  // port, the default port is set as a new Port number.
+  // port, the default port is set as a new Port number. The field
+  // |parser_error| is set accordingly.
   bool SavePort(int port);
 
   // These methods save values of corresponding components. The template

chromeos/printing/uri_unittest.cc

@@ -320,5 +320,38 @@ TEST(UriTest, ParserErrorEmptySegmentInPath) {
   EXPECT_EQ(pe2.parsed_strings, 0u);
 }
 
+TEST(UriTest, ParserErrorInPath) {
+  // Non-printable character (0xBA) inside the path.
+  Uri uri(
+      "  HTTP://example.org/aa/\xba_d/cc"
+      "?name1&name2=param2&\xba_d=character#here\xba ");
+  const Uri::ParserError pe = uri.GetLastParsingError();
+  EXPECT_EQ(pe.status, Uri::ParserStatus::kDisallowedASCIICharacter);
+  EXPECT_EQ(pe.parsed_chars, 24u);
+  EXPECT_EQ(pe.parsed_strings, 0u);
+}
+
+TEST(UriTest, ParserErrorInQuery) {
+  // Non-printable character (0xBA) inside the query.
+  Uri uri(
+      "  HTTP://example.org/aa/bb/cc"
+      "?name1&name2=param2&\xba_d=character#here\xba ");
+  const Uri::ParserError pe = uri.GetLastParsingError();
+  EXPECT_EQ(pe.status, Uri::ParserStatus::kDisallowedASCIICharacter);
+  EXPECT_EQ(pe.parsed_chars, 49u);
+  EXPECT_EQ(pe.parsed_strings, 0u);
+}
+
+TEST(UriTest, ParserErrorInFragment) {
+  // Non-printable character (0xBA) inside the fragment.
+  Uri uri(
+      "  HTTP://example.org/aa/bb/cc"
+      "?name1&name2=param2&good=character#here\xba ");
+  const Uri::ParserError pe = uri.GetLastParsingError();
+  EXPECT_EQ(pe.status, Uri::ParserStatus::kDisallowedASCIICharacter);
+  EXPECT_EQ(pe.parsed_chars, 68u);
+  EXPECT_EQ(pe.parsed_strings, 0u);
+}
+
 }  // namespace
 }  // namespace chromeos