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

sdk/lib/core/uri.dart

Index: sdk/lib/core/uri.dart
diff --git a/sdk/lib/core/uri.dart b/sdk/lib/core/uri.dart
index e4a31aa575fd0f40d23012d7b8f1382cf17d279c..bccb4426b30560c468222d58f46dd46c1cd827cf 100644
--- a/sdk/lib/core/uri.dart
+++ b/sdk/lib/core/uri.dart
@@ -76,6 +76,7 @@ class Uri {
    * 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,
@@ -136,14 +137,16 @@ class Uri {
    * 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
@@ -157,7 +160,7 @@ class Uri {
                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));
@@ -1104,23 +1107,54 @@ class Uri {
    * 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.
+   * 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(splitQueryString(query));
+      _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 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 and the lists it contains are unmodifiable
+   * and will throw [UnsupportedError] on any calls that would mutate them.
+   */
+  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 _queryParameterLists;
+  }
+
+  /**
    * Returns a URI where the path has been normalized.
    *
    * A normalized path does not contain `.` segments or non-leading `..`
@@ -1350,10 +1384,28 @@ class Uri {
         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();
@@ -2156,6 +2208,46 @@ class Uri {
     });
   }
 
+  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).