[url] Avoid scanning for whitespace twice during ResolveRelative

url/url_util.cc

 // Copyright 2013 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 "url/url_util.h"
 
 #include <stddef.h>
 #include <string.h>
 #include <vector>
 
 #include "base/debug/leak_annotations.h"
 #include "base/logging.h"
 #include "base/strings/string_util.h"
 #include "url/url_canon_internal.h"
 #include "url/url_file.h"
 #include "url/url_util_internal.h"
 
 namespace url {
 
 namespace {
 
+// Pass this enum through for methods which would like to know if whitespace
+// removal is necessary.
+enum WhitespaceRemovalPolicy {
+  REMOVE_WHITESPACE,
+  DONT_REMOVE_WHITESPACE,

[Mike West, 2016/12/01 09:39:36]

Nit: I'd prefer `DO_NOT` to `DONT`. The lack of a quote just bothers me
aesthetically. :)

[Charlie Harrison, 2016/12/01 13:27:37]

Done.

+};
+
 const int kNumStandardURLSchemes = 10;
 const SchemeWithType kStandardURLSchemes[kNumStandardURLSchemes] = {
     {kHttpScheme, SCHEME_WITH_PORT},
     {kHttpsScheme, SCHEME_WITH_PORT},
     // Yes, file URLs can have a hostname, so file URLs should be handled as
     // "standard". File URLs never have a port as specified by the SchemeType
     // field.
     {kFileScheme, SCHEME_WITHOUT_PORT},
     {kFtpScheme, SCHEME_WITH_PORT},
     {kGopherScheme, SCHEME_WITH_PORT},
@@ skipping 115 matching lines @@
     // No scheme.
     if (found_scheme)
       *found_scheme = Component();
     return false;
   }
   if (found_scheme)
     *found_scheme = our_scheme;
   return DoCompareSchemeComponent(spec, our_scheme, compare);
 }
 
-template<typename CHAR>
-bool DoCanonicalize(const CHAR* in_spec,
-                    int in_spec_len,
+template <typename CHAR>
+bool DoCanonicalize(const CHAR* spec,
+                    int spec_len,
                     bool trim_path_end,
+                    WhitespaceRemovalPolicy whitespace_policy,
                     CharsetConverter* charset_converter,
                     CanonOutput* output,
                     Parsed* output_parsed) {
-  // Remove any whitespace from the middle of the relative URL, possibly
-  // copying to the new buffer.
+  // Remove any whitespace from the middle of the relative URL if necessary.
+  // Possibly this will result in copying to the new buffer.
   RawCanonOutputT<CHAR> whitespace_buffer;
-  int spec_len;
-  const CHAR* spec = RemoveURLWhitespace(in_spec, in_spec_len,
-                                         &whitespace_buffer, &spec_len);
+  if (whitespace_policy == REMOVE_WHITESPACE)
+    spec = RemoveURLWhitespace(spec, spec_len, &whitespace_buffer, &spec_len);
 
   Parsed parsed_input;
 #ifdef WIN32
   // For Windows, we allow things that look like absolute Windows paths to be
   // fixed up magically to file URLs. This is done for IE compatibility. For
   // example, this will change "c:/foo" into a file URL rather than treating
   // it as a URL with the protocol "c". It also works for UNC ("\\foo\bar.txt").
   // There is similar logic in url_canon_relative.cc for
   //
   // For Max & Unix, we don't do this (the equivalent would be "/foo/bar" which
@@ skipping 100 matching lines @@
     if (base_parsed_authority.host.is_nonempty()) {
       RawCanonOutputT<char> temporary_output;
       bool did_resolve_succeed =
           ResolveRelativeURL(base_spec, base_parsed_authority, false, relative,
                              relative_component, charset_converter,
                              &temporary_output, output_parsed);
       // The output_parsed is incorrect at this point (because it was built
       // based on base_parsed_authority instead of base_parsed) and needs to be
       // re-created.
       DoCanonicalize(temporary_output.data(), temporary_output.length(), true,
-                     charset_converter, output, output_parsed);
+                     REMOVE_WHITESPACE, charset_converter, output,
+                     output_parsed);
       return did_resolve_succeed;
     }
   } else if (is_relative) {
     // Relative, resolve and canonicalize.
     bool file_base_scheme = base_parsed.scheme.is_nonempty() &&
         DoCompareSchemeComponent(base_spec, base_parsed.scheme, kFileScheme);
     return ResolveRelativeURL(base_spec, base_parsed, file_base_scheme, relative,
                               relative_component, charset_converter, output,
                               output_parsed);
   }
 
   // Not relative, canonicalize the input.
-  return DoCanonicalize(relative, relative_length, true, charset_converter,
-                        output, output_parsed);
+  return DoCanonicalize(relative, relative_length, true, DONT_REMOVE_WHITESPACE,
+                        charset_converter, output, output_parsed);
 }
 
 template<typename CHAR>
 bool DoReplaceComponents(const char* spec,
                          int spec_len,
                          const Parsed& parsed,
                          const Replacements<CHAR>& replacements,
                          CharsetConverter* charset_converter,
                          CanonOutput* output,
                          Parsed* out_parsed) {
@@ skipping 26 matching lines @@
     if (spec_len - spec_after_colon > 0) {
       scheme_replaced.Append(&spec[spec_after_colon],
                              spec_len - spec_after_colon);
     }
 
     // We now need to completely re-parse the resulting string since its meaning
     // may have changed with the different scheme.
     RawCanonOutput<128> recanonicalized;
     Parsed recanonicalized_parsed;
     DoCanonicalize(scheme_replaced.data(), scheme_replaced.length(), true,
-                   charset_converter,
-                   &recanonicalized, &recanonicalized_parsed);
+                   REMOVE_WHITESPACE, charset_converter, &recanonicalized,
+                   &recanonicalized_parsed);
 
     // Recurse using the version with the scheme already replaced. This will now
     // use the replacement rules for the new scheme.
     //
     // Warning: this code assumes that ReplaceComponents will re-check all
     // components for validity. This is because we can't fail if DoCanonicalize
     // failed above since theoretically the thing making it fail could be
     // getting replaced here. If ReplaceComponents didn't re-check everything,
     // we wouldn't know if something *not* getting replaced is a problem.
     // If the scheme-specific replacers are made more intelligent so they don't
@@ skipping 165 matching lines @@
 
   return true;
 }
 
 bool Canonicalize(const char* spec,
                   int spec_len,
                   bool trim_path_end,
                   CharsetConverter* charset_converter,
                   CanonOutput* output,
                   Parsed* output_parsed) {
-  return DoCanonicalize(spec, spec_len, trim_path_end, charset_converter,
-                        output, output_parsed);
+  return DoCanonicalize(spec, spec_len, trim_path_end, REMOVE_WHITESPACE,
+                        charset_converter, output, output_parsed);
 }
 
 bool Canonicalize(const base::char16* spec,
                   int spec_len,
                   bool trim_path_end,
                   CharsetConverter* charset_converter,
                   CanonOutput* output,
                   Parsed* output_parsed) {
-  return DoCanonicalize(spec, spec_len, trim_path_end, charset_converter,
-                        output, output_parsed);
+  return DoCanonicalize(spec, spec_len, trim_path_end, REMOVE_WHITESPACE,
+                        charset_converter, output, output_parsed);
 }
 
 bool ResolveRelative(const char* base_spec,
                      int base_spec_len,
                      const Parsed& base_parsed,
                      const char* relative,
                      int relative_length,
                      CharsetConverter* charset_converter,
                      CanonOutput* output,
                      Parsed* output_parsed) {
@@ skipping 103 matching lines @@
   return DoCompareSchemeComponent(spec, component, compare_to);
 }
 
 bool CompareSchemeComponent(const base::char16* spec,
                             const Component& component,
                             const char* compare_to) {
   return DoCompareSchemeComponent(spec, component, compare_to);
 }
 
 }  // namespace url