Use std::string_view in ParseFileSystemURL.

Bug: 325408566
Change-Id: Idf694657ef8be3a98ce297e6316cfcf0df7170df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5374208
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1283621}

components/url_formatter/url_fixer.cc

@@ -483,7 +483,7 @@ std::string SegmentURLInternal(std::string* text, url::Parsed* parts) {
 
   if (scheme == url::kFileSystemScheme) {
     // Have the GURL parser do the heavy lifting for us.
-    url::ParseFileSystemURL(text->data(), text_length, parts);
+    *parts = url::ParseFileSystemURL(*text);
     return scheme;
   }
 

url/third_party/mozilla/url_parse.cc

@@ -496,79 +496,68 @@ void DoParseNonSpecialURL(const CHAR* spec,
   DoParseAfterNonSpecialScheme(spec, spec_len, after_scheme, parsed);
 }
 
-template <typename CHAR>
-void DoParseFileSystemURL(const CHAR* spec, int spec_len, Parsed* parsed) {
-  DCHECK(spec_len >= 0);
-
-  // Get the unused parts of the URL out of the way.
-  parsed->username.reset();
-  parsed->password.reset();
-  parsed->host.reset();
-  parsed->port.reset();
-  parsed->path.reset();          // May use this; reset for convenience.
-  parsed->ref.reset();           // May use this; reset for convenience.
-  parsed->query.reset();         // May use this; reset for convenience.
-  parsed->clear_inner_parsed();  // May use this; reset for convenience.
-  parsed->has_opaque_path = false;
-
+template <typename CharT>
+Parsed DoParseFileSystemURL(std::basic_string_view<CharT> url) {
   // Strip leading & trailing spaces and control characters.
   int begin = 0;
-  TrimURL(spec, &begin, &spec_len);
+  int url_len = base::checked_cast<int>(url.size());
+  TrimURL(url.data(), &begin, &url_len);
 
   // Handle empty specs or ones that contain only whitespace or control chars.
-  if (begin == spec_len) {
-    parsed->scheme.reset();
-    return;
+  if (begin == url_len) {
+    return {};
   }
 
   int inner_start = -1;
-
   // Extract the scheme.  We also handle the case where there is no scheme.
-  if (DoExtractScheme(std::basic_string_view(&spec[begin], spec_len - begin),
-                      &parsed->scheme)) {
+  Parsed parsed;
+  if (DoExtractScheme(url.substr(begin, url_len - begin), &parsed.scheme)) {
     // Offset the results since we gave ExtractScheme a substring.
-    parsed->scheme.begin += begin;
+    parsed.scheme.begin += begin;
 
-    if (parsed->scheme.end() == spec_len - 1)
-      return;
+    if (parsed.scheme.end() == url_len - 1) {
+      return {};
+    }
 
-    inner_start = parsed->scheme.end() + 1;
+    inner_start = parsed.scheme.end() + 1;
   } else {
     // No scheme found; that's not valid for filesystem URLs.
-    parsed->scheme.reset();
-    return;
+    return {};
   }
 
   Component inner_scheme;
-  const CHAR* inner_spec = &spec[inner_start];
-  int inner_spec_len = spec_len - inner_start;
-
-  if (DoExtractScheme(std::basic_string_view(inner_spec, inner_spec_len),
-                      &inner_scheme)) {
+  std::basic_string_view inner_url =
+      url.substr(inner_start, url_len - inner_start);
+  if (DoExtractScheme(inner_url, &inner_scheme)) {
     // Offset the results since we gave ExtractScheme a substring.
     inner_scheme.begin += inner_start;
 
-    if (inner_scheme.end() == spec_len - 1)
-      return;
+    if (inner_scheme.end() == url_len - 1) {
+      return parsed;
+    }
   } else {
     // No scheme found; that's not valid for filesystem URLs.
     // The best we can do is return "filesystem://".
-    return;
+    return parsed;
   }
 
   Parsed inner_parsed;
 
-  if (CompareSchemeComponent(spec, inner_scheme, kFileScheme)) {
-    // File URLs are special.
-    ParseFileURL(inner_spec, inner_spec_len, &inner_parsed);
-  } else if (CompareSchemeComponent(spec, inner_scheme, kFileSystemScheme)) {
+  if (CompareSchemeComponent(url.data(), inner_scheme, kFileScheme)) {
+    // File URLs are special. The static cast is safe because we calculated the
+    // size above as the difference of two ints.
+    ParseFileURL(inner_url.data(), static_cast<int>(inner_url.size()),
+                 &inner_parsed);
+  } else if (CompareSchemeComponent(url.data(), inner_scheme,
+                                    kFileSystemScheme)) {
     // Filesystem URLs don't nest.
-    return;
-  } else if (IsStandard(spec, inner_scheme)) {
+    return parsed;
+  } else if (IsStandard(url.data(), inner_scheme)) {
     // All "normal" URLs.
-    DoParseStandardURL(inner_spec, inner_spec_len, &inner_parsed);
+    DoParseStandardURL(inner_url.data(), static_cast<int>(inner_url.size()),
+                       &inner_parsed);
   } else {
-    return;
+    return parsed;
   }
 
   // All members of inner_parsed need to be offset by inner_start.
@@ -585,15 +574,15 @@ void DoParseFileSystemURL(const CHAR* spec, int spec_len, Parsed* parsed) {
   inner_parsed.path.begin += inner_start;
 
   // Query and ref move from inner_parsed to parsed.
-  parsed->query = inner_parsed.query;
+  parsed.query = inner_parsed.query;
   inner_parsed.query.reset();
-  parsed->ref = inner_parsed.ref;
+  parsed.ref = inner_parsed.ref;
   inner_parsed.ref.reset();
 
-  parsed->set_inner_parsed(inner_parsed);
+  parsed.set_inner_parsed(inner_parsed);
   if (!inner_parsed.scheme.is_valid() || !inner_parsed.path.is_valid() ||
       inner_parsed.inner_parsed()) {
-    return;
+    return parsed;
   }
 
   // The path in inner_parsed should start with a slash, then have a filesystem
@@ -601,18 +590,18 @@ void DoParseFileSystemURL(const CHAR* spec, int spec_len, Parsed* parsed) {
   // second should be what it keeps; the rest goes to parsed.  If the path ends
   // before the second slash, it's still pretty clear what the user meant, so
   // we'll let that through.
-  if (!IsSlashOrBackslash(spec[inner_parsed.path.begin])) {
-    return;
+  if (!IsSlashOrBackslash(url[inner_parsed.path.begin])) {
+    return parsed;
   }
   int inner_path_end = inner_parsed.path.begin + 1;  // skip the leading slash
-  while (inner_path_end < spec_len &&
-         !IsSlashOrBackslash(spec[inner_path_end])) {
+  while (inner_path_end < url_len && !IsSlashOrBackslash(url[inner_path_end])) {
     ++inner_path_end;
   }
-  parsed->path.begin = inner_path_end;
+  parsed.path.begin = inner_path_end;
   int new_inner_path_length = inner_path_end - inner_parsed.path.begin;
-  parsed->path.len = inner_parsed.path.len - new_inner_path_length;
-  parsed->inner_parsed()->path.len = new_inner_path_length;
+  parsed.path.len = inner_parsed.path.len - new_inner_path_length;
+  parsed.inner_parsed()->path.len = new_inner_path_length;
+  return parsed;
 }
 
 // Initializes a path URL which is merely a scheme followed by a path. Examples
@@ -1118,12 +1107,12 @@ void ParsePathURL(const char16_t* url,
   DoParsePathURL(url, url_len, trim_path_end, parsed);
 }
 
-void ParseFileSystemURL(const char* url, int url_len, Parsed* parsed) {
-  DoParseFileSystemURL(url, url_len, parsed);
+Parsed ParseFileSystemURL(std::string_view url) {
+  return DoParseFileSystemURL(url);
 }
 
-void ParseFileSystemURL(const char16_t* url, int url_len, Parsed* parsed) {
-  DoParseFileSystemURL(url, url_len, parsed);
+Parsed ParseFileSystemURL(std::u16string_view url) {
+  return DoParseFileSystemURL(url);
 }
 
 Parsed ParseMailtoURL(std::string_view url) {

url/third_party/mozilla/url_parse.h

@@ -318,10 +318,8 @@ COMPONENT_EXPORT(URL)
 void ParseFileURL(const char16_t* url, int url_len, Parsed* parsed);
 
 // Filesystem URLs are structured differently than other URLs.
-COMPONENT_EXPORT(URL)
-void ParseFileSystemURL(const char* url, int url_len, Parsed* parsed);
-COMPONENT_EXPORT(URL)
-void ParseFileSystemURL(const char16_t* url, int url_len, Parsed* parsed);
+COMPONENT_EXPORT(URL) Parsed ParseFileSystemURL(std::string_view url);
+COMPONENT_EXPORT(URL) Parsed ParseFileSystemURL(std::u16string_view url);
 
 // MailtoURL is for mailto: urls. They are made up scheme,path,query
 COMPONENT_EXPORT(URL) Parsed ParseMailtoURL(std::string_view url);

url/url_canon_unittest.cc

@@ -2103,9 +2103,7 @@ TEST(URLCanonTest, ReplaceFileSystemURL) {
 
   for (const auto& replace_case : replace_cases) {
     const ReplaceCase& cur = replace_case;
-    int base_len = static_cast<int>(strlen(cur.base));
-    Parsed parsed;
-    ParseFileSystemURL(cur.base, base_len, &parsed);
+    Parsed parsed = ParseFileSystemURL(cur.base);
 
     Replacements<char> r;
     typedef Replacements<char> R;  // Clean up syntax.
@@ -2367,9 +2365,7 @@ TEST(URLCanonTest, CanonicalizeFileSystemURL) {
   };
 
   for (const auto& i : cases) {
-    int url_len = static_cast<int>(strlen(i.input));
-    Parsed parsed;
-    ParseFileSystemURL(i.input, url_len, &parsed);
+    Parsed parsed = ParseFileSystemURL(i.input);
 
     Parsed out_parsed;
     std::string out_str;

url/url_parse_unittest.cc

@@ -635,10 +635,9 @@ static FileSystemURLParseCase filesystem_cases[] = {
 TEST(URLParser, FileSystemURL) {
   // Declared outside for loop to try to catch cases in init() where we forget
   // to reset something that is reset by the constructor.
-  Parsed parsed;
   for (const auto& filesystem_case : filesystem_cases) {
     const char* url = filesystem_case.input;
-    ParseFileSystemURL(url, static_cast<int>(strlen(url)), &parsed);
+    Parsed parsed = ParseFileSystemURL(url);
 
     EXPECT_TRUE(ComponentMatches(url, "filesystem", parsed.scheme));
     EXPECT_EQ(!filesystem_case.inner_scheme, !parsed.inner_parsed());

url/url_util.cc

@@ -273,9 +273,9 @@ bool DoCanonicalize(const CHAR* spec,
                                   charset_converter, output, output_parsed);
   } else if (DoCompareSchemeComponent(spec, scheme, url::kFileSystemScheme)) {
     // Filesystem URLs are special.
-    ParseFileSystemURL(spec, spec_len, &parsed_input);
-    success = CanonicalizeFileSystemURL(spec, parsed_input, charset_converter,
-                                        output, output_parsed);
+    success = CanonicalizeFileSystemURL(
+        spec, ParseFileSystemURL(std::basic_string_view(spec, spec_len)),
+        charset_converter, output, output_parsed);
 
   } else if (DoIsStandard(spec, scheme, &scheme_type)) {
     // All "normal" URLs.