Support URL fragment resolution againt non-hierarchical schemes

url/gurl.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.
 
 #ifdef WIN32
 #include <windows.h>
 #else
 #include <pthread.h>
 #endif
 
 #include <algorithm>
 #include <ostream>
 
 #include "url/gurl.h"
 
 #include "base/logging.h"
 #include "url/url_canon_stdstring.h"
 #include "url/url_util.h"
 
-namespace {
+// TODO(joth): Move to appropriate place in file, justing putting this here

[brettw, 2013/11/20 00:03:51]

Thanks, I think you can move this now.

[joth, 2013/11/21 00:08:45]

Done.

+// to make initial code review iterations easier to diff.
 
 // External template that can handle initialization of either character type.
 // The input spec is given, and the canonical version will be placed in
 // |*canonical|, along with the parsing of the canonical spec in |*parsed|.
 template<typename STR>
-bool InitCanonical(const STR& input_spec,
-                   std::string* canonical,
-                   url_parse::Parsed* parsed) {
+void GURL::InitCanonical(const STR& input_spec,
+                         bool trim_path_end) {

[brettw, 2013/11/20 00:03:51]

One line.

[joth, 2013/11/21 00:08:45]

Done.

   // Reserve enough room in the output for the input, plus some extra so that
   // we have room if we have to escape a few things without reallocating.
-  canonical->reserve(input_spec.size() + 32);
-  url_canon::StdStringCanonOutput output(canonical);
-  bool success = url_util::Canonicalize(
-      input_spec.data(), static_cast<int>(input_spec.length()),
-      NULL, &output, parsed);
+  spec_.reserve(input_spec.size() + 32);
+  url_canon::StdStringCanonOutput output(&spec_);
+  is_valid_ = url_util::Canonicalize(
+      input_spec.data(), static_cast<int>(input_spec.length()), trim_path_end,
+      NULL, &output, &parsed_);
 
   output.Complete();  // Must be done before using string.
-  return success;
+  if (is_valid_ && SchemeIsFileSystem()) {
+    inner_url_.reset(new GURL(spec_.data(), parsed_.Length(),
+                              *parsed_.inner_parsed(), true));
+  }
 }
 
+namespace {
+
 static std::string* empty_string = NULL;
 static GURL* empty_gurl = NULL;
 
 #ifdef WIN32
 
 // Returns a static reference to an empty string for returning a reference
 // when there is no underlying string.
 const std::string& EmptyStringForGURL() {
   // Avoid static object construction/destruction on startup/shutdown.
   if (!empty_string) {
@@ skipping 36 matching lines @@
     : spec_(other.spec_),
       is_valid_(other.is_valid_),
       parsed_(other.parsed_) {
   if (other.inner_url_)
     inner_url_.reset(new GURL(*other.inner_url_));
   // Valid filesystem urls should always have an inner_url_.
   DCHECK(!is_valid_ || !SchemeIsFileSystem() || inner_url_);
 }
 
 GURL::GURL(const std::string& url_string) {
-  is_valid_ = InitCanonical(url_string, &spec_, &parsed_);
-  if (is_valid_ && SchemeIsFileSystem()) {
-    inner_url_.reset(
-        new GURL(spec_.data(), parsed_.Length(),
-                 *parsed_.inner_parsed(), true));
-  }
+  InitCanonical(url_string, true);
 }
 
 GURL::GURL(const base::string16& url_string) {
-  is_valid_ = InitCanonical(url_string, &spec_, &parsed_);
-  if (is_valid_ && SchemeIsFileSystem()) {
-    inner_url_.reset(
-        new GURL(spec_.data(), parsed_.Length(),
-                 *parsed_.inner_parsed(), true));
-  }
+  InitCanonical(url_string, true);
+}
+
+GURL::GURL(const std::string& url_string, RetainWhiteSpaceSelector) {
+  InitCanonical(url_string, false);
 }
 
 GURL::GURL(const char* canonical_spec, size_t canonical_spec_len,
            const url_parse::Parsed& parsed, bool is_valid)
     : spec_(canonical_spec, canonical_spec_len),
       is_valid_(is_valid),
       parsed_(parsed) {
   InitializeFromCanonicalSpec();
 }
 
@@ skipping 17 matching lines @@
   // what we would have produced. Skip checking for invalid URLs have no meaning
   // and we can't always canonicalize then reproducabely.
   if (is_valid_) {
     url_parse::Component scheme;
     if (!url_util::FindAndCompareScheme(spec_.data(), spec_.length(),
                                         "filesystem", &scheme) ||
         scheme.begin == parsed_.scheme.begin) {
       // We can't do this check on the inner_url of a filesystem URL, as
       // canonical_spec actually points to the start of the outer URL, so we'd
       // end up with infinite recursion in this constructor.
-      GURL test_url(spec_);
+      GURL test_url(spec_, RETAIN_TRAILING_PATH_WHITEPACE);

[joth, 2013/11/21 00:08:45]

I added an extra comment here too:

     // We need to retain trailing whitespace on path URLs, as the |parsed_|
     // spec we originally received may legitimately contain trailing white-
     // space on the path or  components e.g. if the #ref has been
     // removed from a "foo:hello #ref" URL (see http://crbug.com/291747).

 
       DCHECK(test_url.is_valid_ == is_valid_);
       DCHECK(test_url.spec_ == spec_);
 
       DCHECK(test_url.parsed_.scheme == parsed_.scheme);
       DCHECK(test_url.parsed_.username == parsed_.username);
       DCHECK(test_url.parsed_.password == parsed_.password);
       DCHECK(test_url.parsed_.host == parsed_.host);
       DCHECK(test_url.parsed_.port == parsed_.port);
       DCHECK(test_url.parsed_.path == parsed_.path);
@@ skipping 349 matching lines @@
 void GURL::Swap(GURL* other) {
   spec_.swap(other->spec_);
   std::swap(is_valid_, other->is_valid_);
   std::swap(parsed_, other->parsed_);
   inner_url_.swap(other->inner_url_);
 }
 
 std::ostream& operator<<(std::ostream& out, const GURL& url) {
   return out << url.possibly_invalid_spec();
 }