Support the same parameter key more than once in Uri query parameters.

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 58 matching lines @@
 
   /**
    * Cache the computed return value of [pathSegements].
    */
   List<String> _pathSegments;
 
   /**
    * Cache the computed return value of [queryParameters].
    */
   Map<String, String> _queryParameters;
+  Map<String, List<String>> _queryParameterLists;

[Lasse Reichstein Nielsen, 2015/12/14 13:16:54]

I'm wondering if these can share something, but I don't think so.
It's still annoying to have all these extra fields.
Maybe we should use an expando instead?

[floitsch, 2015/12/14 17:24:54]

I don't really like expandos, but they would save 3 fields... I don't think
there are enough URIs in a program to justify it. Furthermore we would
implicitly drag in Expandos (increasing the size of snapshots...).

 
   /// Internal non-verifying constructor. Only call with validated arguments.
   Uri._internal(this.scheme,
                 this._userInfo,
                 this._host,
                 this._port,
                 this._path,
                 this._query,
                 this._fragment);
 
@@ skipping 40 matching lines @@
    * list of characters: `!$&'()*+,;=:@`. If the other components
    * calls for an absolute path a leading slash `/` is prepended if
    * not already there.
    *
    * The query component is set through either [query] or
    * [queryParameters]. When [query] is used the provided string should
    * be a valid URI query, but invalid characters other than general delimiters,
    * will be escaped if necessary.
    * When [queryParameters] is used the query is built from the
    * provided map. Each key and value in the map is percent-encoded
-   * and joined using equal and ampersand characters. The
-   * percent-encoding of the keys and values encodes all characters
-   * except for the unreserved characters.
+   * and joined using equal and ampersand characters.
+   * A value in the map must be either a string, or a an [Iterable] of strings,
+   * where the latter corresponds to multiple values for the same key.
+   * The percent-encoding of the keys and values encodes all characters
+   * except for the unreserved characters, and replaces spaces with `+`.
    * If `query` is the empty string, it is equivalent to omitting it.
    * To have an actual empty query part,
    * use an empty list for `queryParameters`.
-   * If both `query` and `queryParameters` are omitted or `null`, the
-   * URI will have no query part.
+   * If both `query` and `queryParameters` are omitted or `null`,
+   * the URI will have no query part.
    *
    * The fragment component is set through [fragment].
    * It should be a valid URI fragment, but invalid characters other than
    * general delimiters, will be escaped if necessary.
    * If `fragment` is omitted or `null`, the URI will have no fragment part.
    */
   factory Uri({String scheme : "",
                String userInfo : "",
                String host,
                int port,
                String path,
                Iterable<String> pathSegments,
                String query,
-               Map<String, String> queryParameters,
+               Map<String, dynamic> queryParameters,
                String fragment}) {
     scheme = _makeScheme(scheme, 0, _stringOrNullLength(scheme));
     userInfo = _makeUserInfo(userInfo, 0, _stringOrNullLength(userInfo));
     host = _makeHost(host, 0, _stringOrNullLength(host), false);
     // 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");
@@ skipping 926 matching lines @@
         : new List<String>.unmodifiable(
               pathToSplit.split("/").map(Uri.decodeComponent));
     _pathSegments = result;
     return result;
   }
 
   /**
    * Returns the URI query split into a map according to the rules
    * specified for FORM post in the [HTML 4.01 specification section
    * 17.13.4](http://www.w3.org/TR/REC-html40/interact/forms.html#h-17.13.4 "HTML 4.01 section 17.13.4").
+   * Each key and value in the returned map has been decoded.
+   * If there is no query the empty map is returned.
+   *
+   * Keys in the query string that have no value are mapped to the
+   * empty string.
+   * If a key occurs more than once in the query string, it is mapped to
+   * one of the values. The [queryParameterValues] getter can provide a
+   * map that maps to all the values.
+   *
+   * The returned map is unmodifiable and will throw [UnsupportedError] on any
+   * calls that would mutate it.
+   */
+  Map<String, String> get queryParameters {
+    if (_queryParameters == null) {
+      _queryParameters =
+          new UnmodifiableMapView<String, String>(splitQueryString(query));
+    }
+    return _queryParameters;
+  }
+
+  /**
+   * Returns the URI query split into a map according to the rules
+   * specified for FORM post in the [HTML 4.01 specification section
+   * 17.13.4](http://www.w3.org/TR/REC-html40/interact/forms.html#h-17.13.4 "HTML 4.01 section 17.13.4").
    * Each key and value in the returned map has been decoded. If there is no
    * query the empty map is returned.
    *
-   * Keys in the query string that have no value are mapped to the
-   * empty string.
+   * Keys are mapped to lists of their values. If a key occurs only once,
+   * its value is a singleton list. If a key occurs with no value, the
+   * empty string is used as the value for that occurrence.
    *
-   * The returned map is unmodifiable and will throw [UnsupportedError] on any
-   * calls that would mutate it.
+   * The returned map and the lists it contains are unmodifiable
+   * and will throw [UnsupportedError] on any calls that would mutate them.
    */
-  Map<String, String> get queryParameters {
-    if (_queryParameters == null) {
-      _queryParameters = new UnmodifiableMapView(splitQueryString(query));
+  Map<String, List<String>> get queryParameterLists {

[Lasse Reichstein Nielsen, 2015/12/14 13:16:54]

I'm not particularly happy with the name. Suggestions appreciated.

[floitsch, 2015/12/14 17:24:54]

multiQueryParameters?

[Søren Gjesse, 2015/12/15 07:27:42]

+1

[Lasse Reichstein Nielsen, 2015/12/15 08:11:28]

I don't think that works very well. It sounds like there are multiple queries,
not multiple parameters of the query.

So, 
  queryMultiParameters?
  queryParametersAll?

[floitsch, 2015/12/15 09:17:52]

alternatively we could go the long way:

queryParametersWithDuplicates

[Lasse Reichstein Nielsen, 2015/12/16 13:06:21]

Longer could do it, but "with duplicates" doesn't tell me what it does. It needs
to be read in the context of "queryParameters" and you need to know that that
one has no duplicates. 

It's hard to find a name because the best name is already taken.

[Lasse Reichstein Nielsen, 2016/01/12 13:34:12]

Going with queryParametersAll - which isn't perfect, but it's broken in a
well-established way.

+    if (_queryParameterLists == null) {
+      Map queryParameterLists = _splitQueryStringAll(query);
+      for (var key in queryParameterLists.keys) {
+        queryParameterLists[key] =
+            new List<String>.unmodifiable(queryParameterLists[key]);
+      }
+      _queryParameterLists = new Map<String, List<String>>.unmodifiable(
+        queryParameterLists);
     }
-    return _queryParameters;
+    return _queryParameterLists;
   }
 
   /**
    * Returns a URI where the path has been normalized.
    *
    * A normalized path does not contain `.` segments or non-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.
@@ skipping 212 matching lines @@
     }
     if (query != null) return _normalize(query, start, end, _queryCharTable);
 
     var result = new StringBuffer();
     var first = true;
     queryParameters.forEach((key, value) {
       if (!first) {
         result.write("&");
       }
       first = false;
-      result.write(Uri.encodeQueryComponent(key));
-      if (value != null && !value.isEmpty) {
-        result.write("=");
-        result.write(Uri.encodeQueryComponent(value));
+      if (value == null) {
+        result.write(Uri.encodeQueryComponent(key));
+      } else if (value is String) {
+        result.write(Uri.encodeQueryComponent(key));
+        if (value.isNotEmpty) {
+          result.write("=");
+          result.write(Uri.encodeQueryComponent(value));
+        }
+      } else {
+        Iterable values = value;
+        bool first = true;
+        for (String value in values) {
+          if (!first) {
+            result.write("&");
+          }
+          first = false;
+          result.write(Uri.encodeQueryComponent(key));
+          if (value != null && value.isNotEmpty) {
+            result.write("=");
+            result.write(Uri.encodeQueryComponent(value));
+          }
+        }

[Lasse Reichstein Nielsen, 2015/12/14 13:16:54]

Maybe I should extract some of this into a helper function.

[Lasse Reichstein Nielsen, 2016/01/12 13:34:11]

Done.

       }
     });
     return result.toString();
   }
 
   static String _makeFragment(String fragment, int start, int end) {
     if (fragment == null) return null;
     return _normalize(fragment, start, end, _queryCharTable);
   }
 
@@ skipping 782 matching lines @@
       } else if (index != 0) {
         var key = element.substring(0, index);
         var value = element.substring(index + 1);
         map[Uri.decodeQueryComponent(key, encoding: encoding)] =
             decodeQueryComponent(value, encoding: encoding);
       }
       return map;
     });
   }
 
+  static List _createList() => [];
+
+  static Map _splitQueryStringAll(

[Lasse Reichstein Nielsen, 2015/12/14 13:16:54]

This one is private, while splitQueryString is public.
I'm not sure why it's public.

+      String query, {Encoding encoding: UTF8}) {
+    Map result = {};
+    int i = 0;
+    int start = 0;
+    int equalsIndex = -1;
+    void parsePair(int start, int equalsIndex, int end) {
+      String key;
+      String value;
+      if (start == end) return;
+      if (equalsIndex < 0) {
+        key =  _uriDecode(query, start, end, encoding, true);
+        value = "";
+      } else {
+        key =
+            _uriDecode(query, start, equalsIndex, encoding, true);
+        value =
+            _uriDecode(query, equalsIndex + 1, end, encoding, true);
+      }
+      result.putIfAbsent(key, _createList).add(value);
+    }
+    const int _equals = 0x3d;
+    const int _ampersand = 0x26;
+    while (i < query.length) {
+      int char = query.codeUnitAt(i);
+      if (char == _equals) {
+        if (equalsIndex < 0) equalsIndex = i;
+      } else if (char == _ampersand) {
+        parsePair(start, equalsIndex, i);
+        start = i + 1;
+        equalsIndex = -1;
+      }
+      i++;
+    }
+    parsePair(start, equalsIndex, i);
+    return result;
+  }
+
   /**
    * Parse the [host] as an IP version 4 (IPv4) address, returning the address
    * as a list of 4 bytes in network byte order (big endian).
    *
    * Throws a [FormatException] if [host] is not a valid IPv4 address
    * representation.
    */
   static List<int> parseIPv4Address(String host) {
     void error(String msg) {
       throw new FormatException('Illegal IPv4 address, $msg');
@@ skipping 1088 matching lines @@
   // All non-escape RFC-2396 uric characters.
   //
   //  uric        =  reserved | unreserved | escaped
   //  reserved    =  ";" | "/" | "?" | ":" | "@" | "&" | "=" | "+" | "$" | ","
   //  unreserved  =  alphanum | mark
   //  mark        =  "-" | "_" | "." | "!" | "~" | "*" | "'" | "(" | ")"
   //
   // This is the same characters as in a URI query (which is URI pchar plus '?')
   static const _uricTable = Uri._queryCharTable;
 }