Add encodimg argument to Uri.encodeQueryString

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 f645d2c003eab2830697bf5bb8c07b63d6704473..debd845ca2eaa1e4ccc8ce3c8c07fa1e8dc56bc2 100644
--- a/sdk/lib/core/uri.dart
+++ b/sdk/lib/core/uri.dart
@@ -681,7 +681,7 @@ class Uri {
 
     while (index < length) {
 
-      // Normalize percent encoding to uppercase and don't encode
+      // Normalize percent-encoding to uppercase and don't encode
       // unreserved characters.
       if (component.codeUnitAt(index) == _PERCENT) {
         if (length < index + 2) {
@@ -1114,11 +1114,15 @@ class Uri {
    * for encoding the posting of a HTML form as a query string
    * component.
    *
-   * Spaces will be replaced with plus and all characters except for
+   * Spaces are replaced with plus and all characters except for
    * uppercase and lowercase letters, decimal digits and the
-   * characters `-._~`. Note that the set of characters encoded is a
-   * superset of what HTML 4.01 says as it refers to RFC 1738 for
-   * reserved characters.
+   * characters `-._~` are percent encoded. Note that the set of

[Lasse Reichstein Nielsen, 2013/11/29 12:34:10]

"percent-encoded"

[Søren Gjesse, 2013/12/02 09:00:55]

Done.

+   * characters which are percent-encoded is a superset of what HTML
+   * 4.01 says as it refers to RFC 1738 for reserved characters.
+   *
+   * For all the characters which are percent-encoded the [encoding]
+   * is used to find the actual bytes used to represent each
+   * character. The default encoding is UTF-8.

[Lasse Reichstein Nielsen, 2013/11/29 12:34:10]

Comment is no longer correct - it's not just the unreserved characters that are
encoded, but everything. So, replace the parts above by something like:

 Encode the string [component] according to the HTML 4.01 rules
 for encoding the posting of a HTML form as a query string	 
 component.

 The component is first encoded to bytes using [encoding]. 
 The default is to use [UTF8] encoding, which preserves all 
 the characters that don't need encoding.

 Then the resulting bytes are "percent-encoded". This transforms
 spaces (U+0020) to a plus sign ('+') and all bytes that are not
 the ASCII decimal digits, letters or one of '-._~' are written as 
 a percent sign '%' followed by the two-digit hexadecimal
 representation of the byte.

 Note that the set of characters which are percent-encoded is a 
 superset of what HTML 4.01 requires, since it refers to RFC 1738
 for reserved characters.
 

[Søren Gjesse, 2013/12/02 09:00:55]

Thanks for these suggestions. Comment updated accordingly.

    *
    * When manually encoding query components remember to encode each
    * part separately before building the query string.
@@ -1130,8 +1134,10 @@ class Uri {
    * See http://www.w3.org/TR/html401/interact/forms.html#h-17.13.4.2 for more
    * details.
    */
-  static String encodeQueryComponent(String component) {
-    return _uriEncode(_unreservedTable, component, spaceToPlus: true);
+  static String encodeQueryComponent(String component,
+                                     {Encoding encoding: UTF8}) {
+    return _uriEncode(
+        _unreservedTable, component, encoding: encoding, spaceToPlus: true);
   }
 
   /**
@@ -1353,6 +1359,7 @@ class Uri {
   }
 
   // Frequently used character codes.
+  static const int _SPACE = 0x20;

[Lasse Reichstein Nielsen, 2013/11/29 12:34:10]

Sigh. We really, really should have character code constants in the language!

[Søren Gjesse, 2013/12/02 09:00:55]

Is there a bug to star?

Is it time to add the "chars" package?

   static const int _DOUBLE_QUOTE = 0x22;
   static const int _PERCENT = 0x25;
   static const int _ASTERISK = 0x2A;
@@ -1382,38 +1389,28 @@ class Uri {
    * that appear in [canonicalTable], and returns the escaped string.
    */
   static String _uriEncode(List<int> canonicalTable,
-                    String text,
-                    {bool spaceToPlus: false}) {
+                           String text,
+                           {Encoding encoding: UTF8,
+                            bool spaceToPlus: false}) {
     byteToHex(int v) {
       final String hex = '0123456789ABCDEF';
       return '%${hex[v >> 4]}${hex[v & 0x0f]}';
     }
 
+    // Encode the string into bytes then generate an ASCII only string
+    // by percent encoding selected bytes.
     StringBuffer result = new StringBuffer();
-    for (int i = 0; i < text.length; i++) {
-      int ch = text.codeUnitAt(i);
-      if (ch < 128 && ((canonicalTable[ch >> 4] & (1 << (ch & 0x0f))) != 0)) {
-        result.write(text[i]);
-      } else if (spaceToPlus && text[i] == " ") {
+    var bytes = encoding.encode(text);
+    bytes.forEach((byte) {

[Lasse Reichstein Nielsen, 2013/11/29 12:34:10]

I'd do 
 for (int i = 0; i < bytes.length; i++) {
   int byte = bytes[i];
   ...
 }
No need for an extra function call per byte.

[Søren Gjesse, 2013/12/02 09:00:55]

Done.

+      if (byte < 128 &&
+          ((canonicalTable[byte >> 4] & (1 << (byte & 0x0f))) != 0)) {
+        result.writeCharCode(byte);
+      } else if (spaceToPlus && byte == _SPACE) {
         result.write("+");

[Lasse Reichstein Nielsen, 2013/11/29 12:34:10]

  result.writeCharCode(_PLUS);  
Better to not mix String writes and code writes too much (I think, depends on
implementation, but I'd optimize for sequences of the same kind of writes).

[Søren Gjesse, 2013/12/02 09:00:55]

Done.

       } else {
-        if (ch >= 0xD800 && ch < 0xDC00) {
-          // Low surrogate. We expect a next char high surrogate.
-          ++i;
-          int nextCh = text.length == i ? 0 : text.codeUnitAt(i);
-          if (nextCh >= 0xDC00 && nextCh < 0xE000) {
-            // convert the pair to a U+10000 codepoint
-            ch = 0x10000 + ((ch - 0xD800) << 10) + (nextCh - 0xDC00);
-          } else {
-            throw new ArgumentError('Malformed URI');
-          }
-        }
-        // TODO(floitsch): don't allocate a new string.
-        for (int codepoint in UTF8.encode(new String.fromCharCode(ch))) {
-          result.write(byteToHex(codepoint));
-        }
+        result.write(byteToHex(byte));

[Lasse Reichstein Nielsen, 2013/11/29 12:34:10]

Pass 'result' to the converter and let it put the bytes into it, instead of
creating an intermediate string. I.e.
  writeByteAsHex(byte, result);

with 
  void writeByteAsHex(byte, buffer) {
    const String hex = "0123456789ABCDEF";
    buffer.writeCharCode(hex.codeUnitAt(byte >> 4));
    buffer.writeCharCode(hex.codeUnitAt(byte & 0x0f));
  }

[Søren Gjesse, 2013/12/02 09:00:55]

Done.

       }
-    }
+    });
     return result.toString();
   }
 
@@ -1455,32 +1452,25 @@ class Uri {
   static String _uriDecode(String text,
                            {bool plusToSpace: false,
                             Encoding encoding: UTF8}) {
-    StringBuffer result = new StringBuffer();
-    List<int> codepoints = new List<int>();
-    for (int i = 0; i < text.length;) {
-      int ch = text.codeUnitAt(i);
-      if (ch != _PERCENT) {
-        if (plusToSpace && ch == _PLUS) {
-          result.write(" ");
-        } else {
-          result.writeCharCode(ch);
+    List<int> bytes = new List<int>();

[Lasse Reichstein Nielsen, 2013/11/29 12:34:10]

Consider using a new Uint8List(text.length) as buffer.
It should be smaller than one smi per three characters, and even better if the
string isn't all percent-encodings.

Also consider checking first if there are any plusses, percent-signs or
non-ASCII chars, and only build the buffer if there is. If not, you can use
text.codeUnits directly as the bytes.

(but that's just optimizations).

[Søren Gjesse, 2013/12/02 09:00:55]

Added the test for simple strings. Not sure about Uint8List - aren't we still
trying to keep IE9 working?

+    for (int i = 0; i < text.length; i++) {
+      var codeUnit = text.codeUnitAt(i);
+      if (codeUnit > 128) {

[Lasse Reichstein Nielsen, 2013/11/29 12:34:10]

codeUnit >= 128  
or
codeUnit > 127

[Søren Gjesse, 2013/12/02 09:00:55]

Ups. Good catch.

+        throw new ArgumentError("Illegal percent encoding in URI");
+      }
+      if (codeUnit == _PERCENT) {
+        if (i + 3 > text.length) {
+          throw new ArgumentError('Truncated URI');
         }
-        i++;
+        bytes.add(_hexCharPairToByte(text, i + 1));
+        i += 2;
+      } else if (plusToSpace && codeUnit == _PLUS) {
+        bytes.add(_SPACE);
       } else {
-        codepoints.clear();
-        while (ch == _PERCENT) {
-          if (++i > text.length - 2) {
-            throw new ArgumentError('Truncated URI');
-          }
-          codepoints.add(_hexCharPairToByte(text, i));
-          i += 2;
-          if (i == text.length) break;
-          ch = text.codeUnitAt(i);
-        }
-        result.write(encoding.decode(codepoints));
+        bytes.add(codeUnit);
       }
     }
-    return result.toString();
+    return encoding.decode(bytes);
   }
 
   static bool _isAlphabeticCharacter(int codeUnit)