commit d012b2cf3a76162fdadb8a1eb74507239ea45f26
author Jason Daly <lyceel@gmail.com> Tue Jun 06 13:17:51 2017 -0700
committer Lasse R.H. Nielsen <lrn@google.com> Tue Jun 06 22:17:51 2017 +0200
tree ecc0f7413e2889d3a795ea982eb73a989cf1811c
parent 3b1ee7b6abf6ce80877421067360ce7ed64eccb0

Fix bug in `readAsBytes` via http when content-length is set. (#24)

The `readAsBytes` method returns twice as much data as expected,
with the first half all zeroes. This comes from pre-allocating
the buffer, then appending to it (similar to issue 21, but with
`readAsBytes` instead of `readAsString`).

Updated the tests to also catch this case.

BUG: https://github.com/dart-lang/resource/issues/23

lib/src/io_io.dart

diff --git a/lib/src/io_io.dart b/lib/src/io_io.dart
index 0450651..03cc92e 100644
--- a/lib/src/io_io.dart
+++ b/lib/src/io_io.dart
@@ -4,13 +4,15 @@
 
 import "dart:async" show Future, Stream;
 import "dart:convert" show Encoding, LATIN1, UTF8;
-import "dart:io" show File,
-                      HttpStatus,
-                      HttpClient,
-                      HttpClientResponse,
-                      HttpClientRequest,
-                      HttpException,
-                      HttpHeaders;
+import "dart:io"
+    show
+        File,
+        HttpStatus,
+        HttpClient,
+        HttpClientResponse,
+        HttpClientRequest,
+        HttpException,
+        HttpHeaders;
 
 import "package:typed_data/typed_buffers.dart" show Uint8Buffer;
 
@@ -43,7 +45,8 @@
     _throwIfFailed(response, uri);
     int length = response.contentLength;
     if (length < 0) length = 0;
-    var buffer = new Uint8Buffer(length);
+    // Create empty buffer with capacity matching contentLength.
+    var buffer = new Uint8Buffer(length)..length = 0;
     await for (var bytes in response) {
       buffer.addAll(bytes);
     }

test/loader_http_test.dart

diff --git a/test/loader_http_test.dart b/test/loader_http_test.dart
index 155613e..f86e417 100644
--- a/test/loader_http_test.dart
+++ b/test/loader_http_test.dart
@@ -31,10 +31,12 @@
       request.response.headers.contentType =
           new ContentType("text", "plain", charset: encoding.name);
       if (request.uri.query.contains("length")) {
-        request.response.headers.contentLength = content.length;
+        request.response.headers.contentLength =
+            encoding.encode(content).length;
       }
-      request.response..write(content)
-                      ..close();
+      request.response
+        ..write(content)
+        ..close();
     }).catchError(print);
   });
 
@@ -53,26 +55,40 @@
   test("Latin-1 encoding w/ length", () async {
     // Regression test for issue #21.
     var loader = ResourceLoader.defaultLoader;
-    var newUri = uri.replace(query: "length");  // Request length set.
+    var newUri = uri.replace(query: "length"); // Request length set.
     String string = await loader.readAsString(newUri, encoding: LATIN1);
     expect(string, content);
   });
 
   test("UTF-8 encoding", () async {
     var loader = ResourceLoader.defaultLoader;
+    var newUri = uri.replace(query: "length"); // Request length set.
+    String string = await loader.readAsString(newUri, encoding: UTF8);
+    expect(string, content);
+  });
+
+  test("UTF-8 encoding w/ length", () async {
+    var loader = ResourceLoader.defaultLoader;
     String string = await loader.readAsString(uri, encoding: UTF8);
     expect(string, content);
   });
 
   test("bytes", () async {
     var loader = ResourceLoader.defaultLoader;
-    List<int> bytes = await loader.readAsBytes(uri);  // Sender uses Latin-1.
+    List<int> bytes = await loader.readAsBytes(uri); // Sender uses Latin-1.
+    expect(bytes, content.codeUnits);
+  });
+
+  test("bytes w/ length", () async {
+    var loader = ResourceLoader.defaultLoader;
+    var newUri = uri.replace(query: "length"); // Request length set.
+    List<int> bytes = await loader.readAsBytes(newUri); // Sender uses Latin-1.
     expect(bytes, content.codeUnits);
   });
 
   test("byte stream", () async {
     var loader = ResourceLoader.defaultLoader;
-    var bytes = loader.openRead(uri);  // Sender uses Latin-1.
+    var bytes = loader.openRead(uri); // Sender uses Latin-1.
     var buffer = [];
     await bytes.forEach(buffer.addAll);
     expect(buffer, content.codeUnits);
@@ -80,20 +96,20 @@
 
   test("not found - String", () async {
     var loader = ResourceLoader.defaultLoader;
-    var badUri = uri.resolve("file.not");  // .not makes server fail.
-    expect(loader.readAsString(badUri), throws);
+    var badUri = uri.resolve("file.not"); // .not makes server fail.
+    expect(loader.readAsString(badUri), throwsException);
   });
 
   test("not found - bytes", () async {
     var loader = ResourceLoader.defaultLoader;
-    var badUri = uri.resolve("file.not");  // .not makes server fail.
-    expect(loader.readAsBytes(badUri), throws);
+    var badUri = uri.resolve("file.not"); // .not makes server fail.
+    expect(loader.readAsBytes(badUri), throwsException);
   });
 
   test("not found - byte stream", () async {
     var loader = ResourceLoader.defaultLoader;
-    var badUri = uri.resolve("file.not");  // .not makes server fail.
-    expect(loader.openRead(badUri).length, throws);
+    var badUri = uri.resolve("file.not"); // .not makes server fail.
+    expect(loader.openRead(badUri).length, throwsException);
   });
 
   tearDown(() {
@@ -102,7 +118,6 @@
   });
 }
 
-
 Encoding parseAcceptCharset(List<String> headers) {
   var encoding = LATIN1;
   if (headers != null) {