Fix a number of issues with the Range header handling for serving files

pkg/http_server/test/utils.dart

Index: pkg/http_server/test/utils.dart
diff --git a/pkg/http_server/test/utils.dart b/pkg/http_server/test/utils.dart
index c0b3c5ca49e154a1d138b8f0178f230f31401838..cefa1b43728aef408a2f7ab304454cd278cbbc52 100644
--- a/pkg/http_server/test/utils.dart
+++ b/pkg/http_server/test/utils.dart
@@ -42,12 +42,14 @@ void _testVirtualDir(String name, bool useMocks, Future func(Directory dir)) {
 }
 
 Future<int> getStatusCodeForVirtDir(VirtualDirectory virtualDir,
-                           String path,
-                          {String host,
-                           bool secure: false,
-                           DateTime ifModifiedSince,
-                           bool rawPath: false,
-                           bool followRedirects: true}) {
+                                    String path,
+                                    {String host,
+                                     bool secure: false,
+                                     DateTime ifModifiedSince,
+                                     bool rawPath: false,
+                                     bool followRedirects: true,
+                                     int from,
+                                     int to}) {
 
   // if this is a mock test, then run the mock code path
   if(_isMockTestExpando[currentTestCase]) {
@@ -55,6 +57,7 @@ Future<int> getStatusCodeForVirtDir(VirtualDirectory virtualDir,
 
     var request = new MockHttpRequest(uri, followRedirects: followRedirects,
         ifModifiedSince: ifModifiedSince);
+    _addRangeHeader(request, from, to);
 
     return _withMockRequest(virtualDir, request)
         .then((response) {
@@ -67,7 +70,7 @@ Future<int> getStatusCodeForVirtDir(VirtualDirectory virtualDir,
   return _withServer(virtualDir, (port) {
     return getStatusCode(port, path, host: host, secure: secure,
           ifModifiedSince: ifModifiedSince, rawPath: rawPath,
-          followRedirects: followRedirects);
+          followRedirects: followRedirects, from: from, to: to);
   });
 }
 
@@ -77,7 +80,9 @@ Future<int> getStatusCode(int port,
                            bool secure: false,
                            DateTime ifModifiedSince,
                            bool rawPath: false,
-                           bool followRedirects: true}) {
+                           bool followRedirects: true,
+                           int from,
+                           int to}) {
   var uri = _getUri(port, path, secure: secure, rawPath: rawPath);
 
   return new HttpClient().getUrl(uri)
@@ -87,19 +92,22 @@ Future<int> getStatusCode(int port,
         if (ifModifiedSince != null) {
           request.headers.ifModifiedSince = ifModifiedSince;
         }
+        _addRangeHeader(request, from, to);
         return request.close();
       })
       .then((response) => response.drain().then(
           (_) => response.statusCode));
 }
 
-Future<HttpHeaders> getHeaders(VirtualDirectory virDir, String path) {
+Future<HttpHeaders> getHeaders(
+    VirtualDirectory virDir, String path, {int from, int to}) {
 
   // if this is a mock test, then run the mock code path
   if(_isMockTestExpando[currentTestCase]) {
     var uri = _getUri(0, path);
 
     var request = new MockHttpRequest(uri);
+    _addRangeHeader(request, from, to);
 
     return _withMockRequest(virDir, request)
         .then((response) {
@@ -110,7 +118,7 @@ Future<HttpHeaders> getHeaders(VirtualDirectory virDir, String path) {
   assert(_isMockTestExpando[currentTestCase] == false);
 
   return _withServer(virDir, (port) {
-      return _getHeaders(port, path);
+      return _getHeaders(port, path, from, to);
     });
 }
 
@@ -135,6 +143,29 @@ Future<String> getAsString(VirtualDirectory virtualDir, String path) {
     });
 }
 
+Future<String> getAsBytes(
+    VirtualDirectory virtualDir, String path, {int from, int to}) {
+
+  // if this is a mock test, then run the mock code path
+  if(_isMockTestExpando[currentTestCase]) {

[kustermann, 2014/11/14 13:25:27]

space after 'if'

[Søren Gjesse, 2014/11/14 15:27:29]

Done.

+    var uri = _getUri(0, path);
+
+    var request = new MockHttpRequest(uri);
+    _addRangeHeader(request, from, to);
+
+    return _withMockRequest(virtualDir, request)
+        .then((response) {
+          return response.mockContentBinary;
+        });
+  };
+
+  assert(_isMockTestExpando[currentTestCase] == false);

[kustermann, 2014/11/14 13:25:27]

This assert is not doing anything, If you go into the if(), then it will return
before coming here, right?

Is it just for documentation purposes?

[Søren Gjesse, 2014/11/14 15:27:30]

Honestly don't know. It is in all these methods.

Will remove all in a separate CL.

+
+  return _withServer(virtualDir, (int port) {
+      return _getAsBytes(port, path, from, to);

[kustermann, 2014/11/14 13:25:27]

_getAsBytes is returning Future<List<int>> but the type signature of the above
function is

Future<String> getAsBytes()

[Søren Gjesse, 2014/11/14 15:27:29]

Done.

+    });
+}
+
 Future<MockHttpResponse> _withMockRequest(VirtualDirectory virDir,
     MockHttpRequest request) {
     return virDir.serveRequest(request).then((value) {
@@ -167,10 +198,13 @@ Future _withServer(VirtualDirectory virDir, Future func(int port)) {
       .whenComplete(() => server.close());
 }
 
-Future<HttpHeaders> _getHeaders(int port, String path) =>
+Future<HttpHeaders> _getHeaders(int port, String path, int from, int to) =>
     new HttpClient()

[kustermann, 2014/11/14 13:25:27]

Who is closing HttpClient here?

[Søren Gjesse, 2014/11/14 15:27:29]

Nobody. Added close.

       .get('localhost', port, path)
-      .then((request) => request.close())
+      .then((request) {
+        _addRangeHeader(request, from, to);
+        return request.close();
+      })
       .then((response) => response.drain().then(
           (_) => response.headers));
 
@@ -180,6 +214,15 @@ Future<String> _getAsString(int port, String path) =>
       .then((request) => request.close())
       .then((response) => UTF8.decodeStream(response));
 
+Future<List<int>> _getAsBytes(int port, String path, int from, int to) =>
+    new HttpClient()

[kustermann, 2014/11/14 13:25:27]

ditto

[Søren Gjesse, 2014/11/14 15:27:29]

Ditto.

+      .get('localhost', port, path)
+      .then((request) {
+        _addRangeHeader(request, from, to);
+        return request.close();
+      })
+      .then((response) => response.fold([], (p, e) => p..addAll(e)));
+
 Uri _getUri(int port,
             String path,
            {bool secure: false,
@@ -196,6 +239,14 @@ Uri _getUri(int port,
   }
 }
 
+void _addRangeHeader(HttpRequest request, int from, int to) {
+  var fromStr = from != null ? '$from' : '';
+  var toStr = to != null ? '$to' : '';
+  if (fromStr.isNotEmpty || toStr.isNotEmpty) {
+    request.headers.set(HttpHeaders.RANGE, 'bytes=$fromStr-$toStr');
+  }
+}
+
 const CERTIFICATE = "localhost_cert";