Do "path normalization" when creating a URI.

sdk/lib/core/uri.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of dart.core;
 
 /**
  * A parsed URI, such as a URL.
  *
  * **See also:**
@@ skipping 363 matching lines @@
         char = uri.codeUnitAt(index);
         if (char == _QUESTION || char == _NUMBER_SIGN) {
           break;
         }
         char = EOI;
       }
       state = NOT_IN_PATH;
     }
 
     assert(state == NOT_IN_PATH);
-    bool isFile = (scheme == "file");
-    bool ensureLeadingSlash = host != null;
-    path = _makePath(uri, pathStart, index, null, ensureLeadingSlash, isFile);
+    bool hasAuthority = host != null;

[floitsch, 2015/07/17 13:03:20]

nit: (host != null)

[Lasse Reichstein Nielsen, 2015/07/17 18:40:22]

Done.

+    path = _makePath(uri, pathStart, index, null, scheme, hasAuthority);
 
     if (char == _QUESTION) {
       int numberSignIndex = -1;
       for (int i = index + 1; i < end; i++) {
         if (uri.codeUnitAt(i) == _NUMBER_SIGN) {
           numberSignIndex = i;
           break;
         }
       }
       if (numberSignIndex < 0) {
@@ skipping 107 matching lines @@
     // Special case this constructor for backwards compatibility.
     if (query == "") query = null;
     query = _makeQuery(query, 0, _stringOrNullLength(query), queryParameters);
     fragment = _makeFragment(fragment, 0, _stringOrNullLength(fragment));
     port = _makePort(port, scheme);
     bool isFile = (scheme == "file");
     if (host == null &&
         (userInfo.isNotEmpty || port != null || isFile)) {
       host = "";
     }
-    bool ensureLeadingSlash = host != null;
+    bool hasAuthority = host != null;

[floitsch, 2015/07/17 13:03:20]

ditto.

     path = _makePath(path, 0, _stringOrNullLength(path), pathSegments,
-                     ensureLeadingSlash, isFile);
+                     scheme, hasAuthority);
+    if (scheme.isEmpty && host == null && !path.startsWith('/')) {

[floitsch, 2015/07/17 13:03:20]

ditto.

[Lasse Reichstein Nielsen, 2015/07/17 18:40:23]

Not here, no.
Reading expressions between && is not improved by extra wrapping. The && is a
very visible separator already, and adding parentheses only makes it harder to
read because then you have to find the matching one before you can separate the
expression into parts.

+      path = _normalizeRelativePath(path);
+    } else {
+      path = _removeDotSegments(path);
+    }
     return new Uri._internal(scheme, userInfo, host, port,
                              path, query, fragment);
   }
 
   /**
    * Creates a new `http` URI from authority, path and query.
    *
    * Examples:
    *
    * ```
@@ skipping 419 matching lines @@
       }
     }
     if (host != null) {
       host = _makeHost(host, 0, host.length, false);
     } else if (this.hasAuthority) {
       host = this.host;
     } else if (userInfo.isNotEmpty || port != null || isFile) {
       host = "";
     }
 
-    bool ensureLeadingSlash = (host != null);
+    bool hasAuthority = host != null;

[floitsch, 2015/07/17 13:03:20]

we seem to disagree :)
I like the parenthesis.

[Lasse Reichstein Nielsen, 2015/07/17 18:40:23]

We do disagree. 
Parentheses are added to change or highlight the associativity of operators.
If you add them when they are not necessary, you require the reader to match up
the parentheses before parsing their content. 
In some settings, that's ok because the associativity isn't very clear (nobody
remembers '|' vs '<') so you add them as emphasis and explanation.
In other cases, there should be no doubt about the meaning, and adding
parenthses then only encumer the reader who might think they are added for a
reason that he can't see.

I wouldn't add parentheses around "var sum = something + other;", so I didn't
add parentheses around "host != null". The one reason for doing it is that the
operator contains a '=' sign which may confuse reading slighly.

I'll add it here, and in similar cases, because it's simple enough that I don't
think it matters either way, but in general I don't think adding parntheses is
without cost, and the cost need to be outweighed by benefits.

     if (path != null || pathSegments != null) {
       path = _makePath(path, 0, _stringOrNullLength(path), pathSegments,
-                       ensureLeadingSlash, isFile);
+                       scheme, hasAuthority);
     } else {
       path = this.path;
-      if ((isFile || (ensureLeadingSlash && !path.isEmpty)) &&
+      if ((isFile || (hasAuthority && !path.isEmpty)) &&
           !path.startsWith('/')) {
-        path = "/$path";
+        path = "/" + path;
       }
     }
 
     if (query != null || queryParameters != null) {
       query = _makeQuery(query, 0, _stringOrNullLength(query), queryParameters);
     } else if (this.hasQuery) {
       query = this.query;
     }
 
     if (fragment != null) {
@@ skipping 43 matching lines @@
    * calls that would mutate it.
    */
   Map<String, String> get queryParameters {
     if (_queryParameters == null) {
       _queryParameters = new UnmodifiableMapView(splitQueryString(query));
     }
     return _queryParameters;
   }
 
   /**
    * Returns an URI where the path has been normalized.

[floitsch, 2015/07/17 13:03:20]

a URI

[Lasse Reichstein Nielsen, 2015/07/17 18:40:22]

Done.

    *
    * A normalized path does not contain `.` segments or non-leading `..`
    * segments.
-   * Only a relative path may contain leading `..` segments,
+   * Only a relative path with no scheme or authority may contain
+   * leading `..` segments,
    * a path that starts with `/` will also drop any leading `..` segments.
    *
-   * This uses the same normalization strategy as [resolveUri], as specified by
-   * RFC 3986.
+   * This uses the same normalization strategy as `new Uri().resolve(this)`.
    *
    * Does not change any part of the URI except the path.
    */
   Uri normalizePath() {
-    String path = _removeDotSegments(_path);
+    String path = _normalizePath(_path, scheme, hasAuthority);
     if (identical(path, _path)) return this;
     return this.replace(path: path);
   }
 
   static int _makePort(int port, String scheme) {
     // Perform scheme specific normalization.
     if (port != null && port == _defaultPort(scheme)) return null;
     return port;
   }
 
@@ skipping 119 matching lines @@
    * Validates scheme characters and does case-normalization.
    *
    * Schemes are converted to lower case. They cannot contain escapes.
    */
   static String _makeScheme(String scheme, int start, int end) {
     if (start == end) return "";
     final int firstCodeUnit = scheme.codeUnitAt(start);
     if (!_isAlphabeticCharacter(firstCodeUnit)) {
       _fail(scheme, start, "Scheme not starting with alphabetic character");
     }
-    bool allLowercase = firstCodeUnit >= _LOWER_CASE_A;
+    int minChar = firstCodeUnit;
     for (int i = start; i < end; i++) {
       final int codeUnit = scheme.codeUnitAt(i);
       if (!_isSchemeCharacter(codeUnit)) {
         _fail(scheme, i, "Illegal scheme character");
       }
-      if (codeUnit < _LOWER_CASE_A || codeUnit > _LOWER_CASE_Z) {
-        allLowercase = false;
-      }
+      if (codeUnit < minChar) minChar = codeUnit;
     }
+    // All valid scheme characters are greater than or equal to _UPPER_CASE_A.
+    // If they are above _UPPER_CASE_Z, the scheme is already lower-case.

[floitsch, 2015/07/17 13:03:20]

maybe add assert(_UPPER_CASE_Z < _LOWER_CASE_A);

(for documentation purposes)

[Lasse Reichstein Nielsen, 2015/07/17 18:40:23]

I'll revert this, I had missed that there are valid non-letter characters before
'A' as well.

     scheme = scheme.substring(start, end);
+    bool allLowercase = minChar > _UPPER_CASE_Z;

[floitsch, 2015/07/17 13:03:20]

why not minChar >= _LOWER_CASE_A ?

[Lasse Reichstein Nielsen, 2015/07/17 18:40:23]

Because we allow two non-letter characters, one of which is between 'Z' and 'a'.
Sadly, some are also below 'A', so this doesn't work.

     if (!allLowercase) scheme = scheme.toLowerCase();
     return scheme;
   }
 
   static String _makeUserInfo(String userInfo, int start, int end) {
     if (userInfo == null) return "";
     return _normalize(userInfo, start, end, _userinfoTable);
   }
 
   static String _makePath(String path, int start, int end,
                           Iterable<String> pathSegments,
-                          bool ensureLeadingSlash,
-                          bool isFile) {
+                          String scheme,
+                          bool hasAuthority) {
+    bool isFile = (scheme == "file");
+    bool ensureLeadingSlash = isFile || hasAuthority;
     if (path == null && pathSegments == null) return isFile ? "/" : "";
     if (path != null && pathSegments != null) {
       throw new ArgumentError('Both path and pathSegments specified');
     }
     var result;
     if (path != null) {
       result = _normalize(path, start, end, _pathCharOrSlashTable);
     } else {
       result = pathSegments.map((s) => _uriEncode(_pathCharTable, s)).join("/");
     }
     if (result.isEmpty) {
       if (isFile) return "/";
-    } else if ((isFile || ensureLeadingSlash) &&
-               result.codeUnitAt(0) != _SLASH) {
-      return "/$result";
+    } else if (ensureLeadingSlash && !result.startsWith('/')) {
+      result = "/" + result;
     }
+    result = _normalizePath(result, scheme, hasAuthority);
     return result;
   }
 
+  /// Performs path normalization (remove dot segments) on a path.
+  ///
+  /// If the URI has neither scheme nor authority, it's considered a
+  /// "pure path" and normalization won't remove leading ".." segments.
+  /// Otherwise it follows the RFC 3986 "remove dot segments" algorithm.
+  static String _normalizePath(String path, String scheme, bool hasAuthority) {
+    if (scheme.isEmpty && !hasAuthority && !path.startsWith('/')) {
+      return _normalizeRelativePath(path);
+    }
+    return _removeDotSegments(path);
+  }
+
   static String _makeQuery(String query, int start, int end,
                            Map<String, String> queryParameters) {
     if (query == null && queryParameters == null) return null;
     if (query != null && queryParameters != null) {
       throw new ArgumentError('Both query and queryParameters specified');
     }
     if (query != null) return _normalize(query, start, end, _queryCharTable);
 
     var result = new StringBuffer();
     var first = true;
@@ skipping 188 matching lines @@
   static bool _isGeneralDelimiter(int ch) {
     return ch <= _RIGHT_BRACKET &&
         ((_genDelimitersTable[ch >> 4] & (1 << (ch & 0x0f))) != 0);
   }
 
   /**
    * Returns whether the URI is absolute.
    */
   bool get isAbsolute => scheme != "" && fragment == "";
 
-  String _merge(String base, String reference) {
-    if (base.isEmpty) return "/$reference";
+  String _mergePaths(String base, String reference) {
     // Optimize for the case: absolute base, reference beginning with "../".
     int backCount = 0;
     int refStart = 0;
     // Count number of "../" at beginning of reference.
     while (reference.startsWith("../", refStart)) {
       refStart += 3;
       backCount++;
     }
 
     // Drop last segment - everything after last '/' of base.
@@ skipping 12 matching lines @@
           (delta == 2 || base.codeUnitAt(newEnd + 2) == _DOT)) {
         break;
       }
       baseEnd = newEnd;
       backCount--;
     }
     return base.replaceRange(baseEnd + 1, null,
                              reference.substring(refStart - 3 * backCount));
   }
 
-  bool _hasDotSegments(String path) {
+  /// Make a guess at whether a path contains a `..` or `.` segments.

[floitsch, 2015/07/17 13:03:20]

segment.

[Lasse Reichstein Nielsen, 2015/07/17 18:40:22]

Done.

+  ///
+  /// This is a primitive test that can cause false positives.
+  /// It's only used to avoid a more expensive operation in the case where
+  /// it's not necessary.
+  static bool _hasDotSegments(String path) {

[floitsch, 2015/07/17 13:03:20]

_maybeHasDotSegments ?

[Lasse Reichstein Nielsen, 2015/07/17 18:40:23]

_mayContainDotSegments

     if (path.length > 0 && path.codeUnitAt(0) == _DOT) return true;
     int index = path.indexOf("/.");
     return index != -1;
   }
 
-  String _removeDotSegments(String path) {
+  /// Removes '.' and '..' segments from a path.
+  ///
+  /// Follows the RFC 2986 "remove dot segments" algorithm.
+  /// This algorithm is only used on paths of URIs with a scheme,
+  /// and it treats the path as if it is absolute (leading '..' are removed).
+  static String _removeDotSegments(String path) {
     if (!_hasDotSegments(path)) return path;
     List<String> output = [];
     bool appendSlash = false;
     for (String segment in path.split("/")) {
       appendSlash = false;
       if (segment == "..") {
-        if (!output.isEmpty &&
-            ((output.length != 1) || (output[0] != ""))) output.removeLast();
+        if (output.isNotEmpty) {
+          output.removeLast();
+          if (output.isEmpty) {
+            output.add("");
+          }
+        }
         appendSlash = true;
       } else if ("." == segment) {
         appendSlash = true;
       } else {
         output.add(segment);
       }
     }
     if (appendSlash) output.add("");
     return output.join("/");
   }
 
+  /// Removes all `.` segments and any non-leading `..` segments.
+  ///
+  /// Removing the ".." from a "bar/foo/.." sequence results in "bar/"
+  /// (trailing "/"). If the entire path is removed (because it contains as
+  /// many ".." segments as real segments), the result is "./".
+  /// This is different from an empty string, which represents "no path",
+  /// when you resolve it against a base URI with a path with a non-empty
+  /// final segment.
+  static String _normalizeRelativePath(String path) {
+    assert(!path.startsWith('/'));  // Only get called for relative paths.
+    if (!_hasDotSegments(path)) return path;  // Ensures path non-empty below.

[floitsch, 2015/07/17 13:03:20]

If _hasDotSegments return false for the empty string it must say so in the
documentation.

?:
// Also weeds out empty paths.

[Lasse Reichstein Nielsen, 2015/07/17 18:40:23]

Well, it's pretty certain that an empty path won't contain any dot segments, so
not something I would document on that function. That's why I document it here.
I'll just add an assert.

+    List<String> output = [];
+    bool appendSlash = false;
+    for (String segment in path.split("/")) {
+      appendSlash = false;
+      if (".." == segment) {
+        if (!output.isEmpty && output.last != "..") {
+          output.removeLast();
+          appendSlash = true;
+        } else {
+          output.add("..");
+        }
+      } else if ("." == segment) {
+        appendSlash = true;
+      } else {
+        output.add(segment);
+      }
+    }
+    if (output.isEmpty || output.length == 1 && output[0].isEmpty) return "./";

[floitsch, 2015/07/17 13:03:20]

parenthesis

[Lasse Reichstein Nielsen, 2015/07/17 18:40:22]

Ack, with mixed || and &&, some parentheses can improve reading.

+    if (appendSlash || output.last == '..') output.add("");
+    return output.join("/");
+  }
+
   /**
    * Resolve [reference] as an URI relative to `this`.
    *
    * First turn [reference] into a URI using [Uri.parse]. Then resolve the
    * resulting URI relative to `this`.
    *
    * Returns the resolved URI.
    *
    * See [resolveUri] for details.
    */
   Uri resolve(String reference) {
     return resolveUri(Uri.parse(reference));
   }
 
   /**
    * Resolve [reference] as an URI relative to `this`.
    *
    * Returns the resolved URI.
    *
-   * The algorithm for resolving a reference is described in
-   * [RFC-3986 Section 5]
+   * The algorithm "Transform Reference" for resolving a reference is
+   * described in [RFC-3986 Section 5]
    * (http://tools.ietf.org/html/rfc3986#section-5 "RFC-1123").
+   *
+   * Updated to handle the case where the base URI is just a relative path -
+   * that is: when it has no scheme or authority and the path does not start
+   * with a slash.
+   * In that case, the paths are combined without removing leading "..", and
+   * an empty path is not converted to "/".
    */
   Uri resolveUri(Uri reference) {
     // From RFC 3986.
     String targetScheme;
     String targetUserInfo = "";
     String targetHost;
     int targetPort;
     String targetPath;
     String targetQuery;
     if (reference.scheme.isNotEmpty) {
@@ skipping 10 matching lines @@
     } else {
       targetScheme = this.scheme;
       if (reference.hasAuthority) {
         targetUserInfo = reference.userInfo;
         targetHost = reference.host;
         targetPort = _makePort(reference.hasPort ? reference.port : null,
                                targetScheme);
         targetPath = _removeDotSegments(reference.path);
         if (reference.hasQuery) targetQuery = reference.query;
       } else {
+        targetUserInfo = this._userInfo;
+        targetHost = this._host;
+        targetPort = this._port;
         if (reference.path == "") {
           targetPath = this._path;
           if (reference.hasQuery) {
             targetQuery = reference.query;
           } else {
             targetQuery = this._query;
           }
         } else {
-          if (reference.path.startsWith("/")) {
+          if (reference.hasAbsolutePath) {
             targetPath = _removeDotSegments(reference.path);
           } else {
-            targetPath = _removeDotSegments(_merge(this._path, reference.path));
+            // This is the RFC 3986 behavior for merging.
+            if (this.hasEmptyPath) {
+              if (!this.hasScheme && !this.hasAuthority) {
+                // Keep the path relative if no scheme or authority.
+                targetPath = reference.path;
+              } else {
+                // Add path normalization on top of RFC algorithm.
+                targetPath = _removeDotSegments("/" + reference.path);
+              }
+            } else {
+              var mergedPath = _mergePaths(this._path, reference.path);
+              if (this.hasScheme || this.hasAuthority || this.hasAbsolutePath) {
+                targetPath = _removeDotSegments(mergedPath);
+              } else {
+                // Non-RFC 3986 beavior. If both base and reference are relative

[floitsch, 2015/07/17 13:03:20]

behavior.

Also state if this is violating the RFC, or if it is additional behavior that
isn't covered by the RFC.

[Lasse Reichstein Nielsen, 2015/07/17 18:40:23]

Not covered, will document.

+                // path, allow the merged path to start with "..".
+                targetPath = _normalizeRelativePath(mergedPath);
+              }
+            }
           }
           if (reference.hasQuery) targetQuery = reference.query;
         }
-        targetUserInfo = this._userInfo;
-        targetHost = this._host;
-        targetPort = this._port;
       }
     }
     String fragment = reference.hasFragment ? reference.fragment : null;
     return new Uri._internal(targetScheme,
                              targetUserInfo,
                              targetHost,
                              targetPort,
                              targetPath,
                              targetQuery,
                              fragment);
   }
 
   /**
+   * Returns whether the URI has a [scheme] component.
+   */
+  bool get hasScheme => scheme.isNotEmpty;
+
+  /**
    * Returns whether the URI has an [authority] component.
    */
   bool get hasAuthority => _host != null;
 
   /**
    * Returns whether the URI has an explicit port.
    *
    * If the port number is the default port number
    * (zero for unrecognized schemes, with http (80) and https (443) being
    * recognized),
    * then the port is made implicit and omitted from the URI.
    */
   bool get hasPort => _port != null;
 
   /**
    * Returns whether the URI has a query part.
    */
   bool get hasQuery => _query != null;
 
   /**
    * Returns whether the URI has a fragment part.
    */
   bool get hasFragment => _fragment != null;
 
   /**
+   * Returns whether the URI has an empty path.
+   */
+  bool get hasEmptyPath => _path.isEmpty;
+
+  /**
+   * Returns whether the URI has an absolute path (starting with '/').
+   */
+  bool get hasAbsolutePath => _path.startsWith('/');
+
+  /**
    * Returns the origin of the URI in the form scheme://host:port for the
    * schemes http and https.
    *
    * It is an error if the scheme is not "http" or "https".
    *
    * See: http://www.w3.org/TR/2011/WD-html5-20110405/origin-0.html#origin
    */
   String get origin {
     if (scheme == "" || _host == null || _host == "") {
       throw new StateError("Cannot use origin without a scheme: $this");
@@ skipping 860 matching lines @@
       0xafff,   // 0x30 - 0x3f  1111111111110101
                 //              @ABCDEFGHIJKLMNO
       0xffff,   // 0x40 - 0x4f  1111111111111111
                 //              PQRSTUVWXYZ    _
       0x87ff,   // 0x50 - 0x5f  1111111111100001
                 //               abcdefghijklmno
       0xfffe,   // 0x60 - 0x6f  0111111111111111
                 //              pqrstuvwxyz   ~
       0x47ff];  // 0x70 - 0x7f  1111111111100010
 }