Make HttpClient not send fragments as part of request.

Make HttpServer ignore fragments sent in requests (which are invalid HTTP request syntax).

Add `removeFragment` method to Uri.

R=sgjesse@google.com

Review URL: https://codereview.chromium.org//1281973004 .

CHANGELOG.md

@@ -1,3 +1,14 @@
+## 1.13.0
+
+* `dart:core`
+  * `Uri` added `removeFragment` method.
+
+* `dart:io`
+  * `HttpClient` no longer sends URI fragments in the requeust. This is not
+    allowed by the HTTP protocol.
+    The `HttpServer` still gracefully receives fragments, but discards them
+    before delivering the request.
+
 ## 1.12.0
 
 ### Core library changes

sdk/lib/core/uri.dart

@@ -985,6 +985,16 @@ class Uri {
         scheme, userInfo, host, port, path, query, fragment);
   }
 
+  /**
+   * Returns a `Uri` that differs from this only in not having a fragment.
+   *
+   * If this `Uri` does not have a fragment, it is itself returned.
+   */
+  Uri removeFragment() {
+    if (!this.hasFragment) return this;
+    return new Uri._internal(scheme, userInfo, host, port, path, query, null);
+  }
+
   /**
    * Returns the URI path split into its segments. Each of the
    * segments in the returned list have been decoded. If the path is

sdk/lib/io/http.dart

@@ -958,7 +958,7 @@ abstract class HttpRequest implements Stream<List<int>> {
    * The URI for the request.
    *
    * This provides access to the
-   * path, query string, and fragment identifier for the request.
+   * path and query string for the request.
    */
   Uri get uri;
 
@@ -1338,7 +1338,8 @@ abstract class HttpClient {
    *
    * The HTTP method to use is specified in [method], the server is
    * specified using [host] and [port], and the path (including
-   * possible fragment and query) is specified using [path].
+   * a possible query) is specified using [path].
+   * The path may also contain a URI fragment, which will be ignored.
    *
    * The `Host` header for the request will be set to the value
    * [host]:[port]. This can be overridden through the
@@ -1377,7 +1378,7 @@ abstract class HttpClient {
    * Opens a HTTP connection using the GET method.
    *
    * The server is specified using [host] and [port], and the path
-   * (including possible fragment and query) is specified using
+   * (including a possible query) is specified using
    * [path].
    *
    * See [open] for details.
@@ -1397,7 +1398,7 @@ abstract class HttpClient {
    * Opens a HTTP connection using the POST method.
    *
    * The server is specified using [host] and [port], and the path
-   * (including possible fragment and query) is specified using
+   * (including a possible query) is specified using
    * [path].
    *
    * See [open] for details.
@@ -1417,8 +1418,7 @@ abstract class HttpClient {
    * Opens a HTTP connection using the PUT method.
    *
    * The server is specified using [host] and [port], and the path
-   * (including possible fragment and query) is specified using
-   * [path].
+   * (including a possible query) is specified using [path].
    *
    * See [open] for details.
    */
@@ -1437,8 +1437,7 @@ abstract class HttpClient {
    * Opens a HTTP connection using the DELETE method.
    *
    * The server is specified using [host] and [port], and the path
-   * (including possible fragment and query) is specified using
-   * [path].
+   * (including s possible query) is specified using [path].
    *
    * See [open] for details.
    */
@@ -1457,8 +1456,7 @@ abstract class HttpClient {
    * Opens a HTTP connection using the PATCH method.
    *
    * The server is specified using [host] and [port], and the path
-   * (including possible fragment and query) is specified using
-   * [path].
+   * (including a possible query) is specified using [path].
    *
    * See [open] for details.
    */
@@ -1477,8 +1475,7 @@ abstract class HttpClient {
    * Opens a HTTP connection using the HEAD method.
    *
    * The server is specified using [host] and [port], and the path
-   * (including possible fragment and query) is specified using
-   * [path].
+   * (including a possible query) is specified using [path].
    *
    * See [open] for details.
    */

sdk/lib/io/http_impl.dart

@@ -791,13 +791,9 @@ class _HttpClientRequest extends _HttpOutboundMessage<HttpClientResponse>
     // Generate the request URI starting from the path component.
     String uriStartingFromPath() {
       String result = uri.path;
-      if (result.length == 0) result = "/";
-      if (uri.query != "") {
-        if (uri.fragment != "") {
-          result = "${result}?${uri.query}#${uri.fragment}";
-        } else {
-          result = "${result}?${uri.query}";
-        }
+      if (result.isEmpty) result = "/";
+      if (uri.hasQuery) {
+        result = "${result}?${uri.query}";
       }
       return result;
     }
@@ -814,7 +810,7 @@ class _HttpClientRequest extends _HttpOutboundMessage<HttpClientResponse>
         if (_httpClientConnection._proxyTunnel) {
           return uriStartingFromPath();
         } else {
-          return uri.toString();
+          return uri.removeFragment().toString();
         }
       }
     }
@@ -1702,9 +1698,26 @@ class _HttpClient implements HttpClient {
                                  String host,
                                  int port,
                                  String path) {
-    Uri uri = new Uri(scheme: "http", host: host, port: port).resolve(path);
-    // TODO(sgjesse): The path set here can contain both query and
-    // fragment. They should be cracked and set correctly.
+    const int hashMark = 0x23;
+    const int questionMark = 0x3f;
+    int fragmentStart = path.length;
+    int queryStart = path.length;
+    for (int i = path.length - 1; i >= 0; i--) {
+      var char = path.codeUnitAt(i);
+      if (char == hashMark) {
+        fragmentStart = i;
+        queryStart = i;
+      } else if (char == questionMark) {
+        queryStart = i;
+      }
+    }
+    String query = null;
+    if (queryStart < fragmentStart) {
+      query = path.substring(queryStart + 1, fragmentStart);
+      path = path.substring(0, queryStart);
+    }
+    Uri uri = new Uri(scheme: "http", host: host, port: port,
+                      path: path, query: query);
     return _openUrl(method, uri);
   }
 
@@ -1772,6 +1785,9 @@ class _HttpClient implements HttpClient {
   set findProxy(String f(Uri uri)) => _findProxy = f;
 
   Future<HttpClientRequest> _openUrl(String method, Uri uri) {
+    // Ignore any fragments on the request URI.
+    uri = uri.removeFragment();
+
     if (method == null) {
       throw new ArgumentError(method);
     }

tests/standalone/io/http_requested_uri_test.dart

@@ -7,19 +7,20 @@ import "package:async_helper/async_helper.dart";
 import "dart:async";
 import "dart:io";
 
-const PATH = '/path?a=b#c';
+const sendPath = '/path?a=b#c';
+const expectedPath = '/path?a=b';
 
 void test(String expected, Map headers) {
   asyncStart();
   HttpServer.bind("localhost", 0).then((server) {
     expected = expected.replaceAll('%PORT', server.port.toString());
     server.listen((request) {
-      Expect.equals("$expected$PATH",
+      Expect.equals("$expected$expectedPath",
                     request.requestedUri.toString());
       request.response.close();
     });
     HttpClient client = new HttpClient();
-    client.get("localhost", server.port, PATH)
+    client.get("localhost", server.port, sendPath)
       .then((request) {
         for (var v in headers.keys) {
           if (headers[v] != null) {