From 83a94f9e4030cf608ffb53592b937bbe7e7a4307 Mon Sep 17 00:00:00 2001 From: Don Date: Thu, 21 Sep 2017 13:43:51 -0700 Subject: [PATCH 01/20] Update MultipartFile --- lib/http.dart | 1 + lib/src/multipart_file.dart | 56 +++++++++++++++++-------------------- 2 files changed, 27 insertions(+), 30 deletions(-) diff --git a/lib/http.dart b/lib/http.dart index 2728e1994b..4736abf2da 100644 --- a/lib/http.dart +++ b/lib/http.dart @@ -16,6 +16,7 @@ export 'src/exception.dart'; export 'src/handler.dart'; export 'src/io_client.dart'; export 'src/middleware.dart'; +export 'src/multipart_file.dart'; export 'src/pipeline.dart'; export 'src/request.dart'; export 'src/response.dart'; diff --git a/lib/src/multipart_file.dart b/lib/src/multipart_file.dart index da4bface78..0736886bae 100644 --- a/lib/src/multipart_file.dart +++ b/lib/src/multipart_file.dart @@ -7,20 +7,23 @@ import 'dart:convert'; import 'dart:io'; import 'package:async/async.dart'; +import 'package:collection/collection.dart'; import 'package:http_parser/http_parser.dart'; import 'package:path/path.dart' as path; -import 'byte_stream.dart'; import 'utils.dart'; /// A file to be uploaded as part of a [MultipartRequest]. This doesn't need to /// correspond to a physical file. class MultipartFile { + /// The stream that will emit the file's contents. + Stream> _stream; + /// The name of the form field for the file. final String field; /// The size of the file in bytes. This must be known in advance, even if this - /// file is created from a [ByteStream]. + /// file is created from a [Stream]. final int length; /// The basename of the file. May be null. @@ -29,13 +32,6 @@ class MultipartFile { /// The content-type of the file. Defaults to `application/octet-stream`. final MediaType contentType; - /// The stream that will emit the file's contents. - final ByteStream _stream; - - /// Whether [finalize] has been called. - bool get isFinalized => _isFinalized; - bool _isFinalized = false; - /// Creates a new [MultipartFile] from a chunked [Stream] of bytes. The length /// of the file in bytes must be known in advance. If it's not, read the data /// from the stream and use [MultipartFile.fromBytes] instead. @@ -44,9 +40,10 @@ class MultipartFile { /// future may be inferred from [filename]. MultipartFile(this.field, Stream> stream, this.length, {this.filename, MediaType contentType}) - : this._stream = toByteStream(stream), - this.contentType = contentType != null ? contentType : - new MediaType("application", "octet-stream"); + : this._stream = stream, + this.contentType = contentType != null + ? contentType + : new MediaType("application", "octet-stream"); /// Creates a new [MultipartFile] from a byte array. /// @@ -54,10 +51,9 @@ class MultipartFile { /// future may be inferred from [filename]. factory MultipartFile.fromBytes(String field, List value, {String filename, MediaType contentType}) { - var stream = new ByteStream.fromBytes(value); + var stream = new Stream.fromIterable([DelegatingList.typed(value)]); return new MultipartFile(field, stream, value.length, - filename: filename, - contentType: contentType); + filename: filename, contentType: contentType); } /// Creates a new [MultipartFile] from a string. @@ -68,14 +64,13 @@ class MultipartFile { /// the future may be inferred from [filename]. factory MultipartFile.fromString(String field, String value, {String filename, MediaType contentType}) { - contentType = contentType == null ? new MediaType("text", "plain") - : contentType; + contentType = + contentType == null ? new MediaType("text", "plain") : contentType; var encoding = encodingForCharset(contentType.parameters['charset'], UTF8); contentType = contentType.change(parameters: {'charset': encoding.name}); return new MultipartFile.fromBytes(field, encoding.encode(value), - filename: filename, - contentType: contentType); + filename: filename, contentType: contentType); } // TODO(nweiz): Infer the content-type from the filename. @@ -92,20 +87,21 @@ class MultipartFile { if (filename == null) filename = path.basename(filePath); var file = new File(filePath); var length = await file.length(); - var stream = new ByteStream(DelegatingStream.typed(file.openRead())); + var stream = DelegatingStream.typed(file.openRead()); return new MultipartFile(field, stream, length, - filename: filename, - contentType: contentType); + filename: filename, contentType: contentType); } - // Finalizes the file in preparation for it being sent as part of a - // [MultipartRequest]. This returns a [ByteStream] that should emit the body - // of the file. The stream may be closed to indicate an empty file. - ByteStream finalize() { - if (isFinalized) { - throw new StateError("Can't finalize a finalized MultipartFile."); + /// Returns a [Stream] representing the file. + /// + /// Can only be called once. + Stream> read() { + if (_stream == null) { + throw new StateError("The 'read' method can only be called once on a " + "http.MultipartFile object."); } - _isFinalized = true; - return _stream; + var stream = _stream; + _stream = null; + return stream; } } From a94906307c555fd0b2ebb4f1433eaf23303d4d74 Mon Sep 17 00:00:00 2001 From: Don Date: Thu, 21 Sep 2017 13:44:23 -0700 Subject: [PATCH 02/20] Update MultipartRequest --- lib/src/boundary_characters.dart | 19 +++ lib/src/multipart_request.dart | 233 ++++++++++++++----------------- lib/src/request.dart | 15 ++ 3 files changed, 141 insertions(+), 126 deletions(-) diff --git a/lib/src/boundary_characters.dart b/lib/src/boundary_characters.dart index cc5742a30a..249acbd047 100644 --- a/lib/src/boundary_characters.dart +++ b/lib/src/boundary_characters.dart @@ -2,6 +2,8 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'dart:math'; + /// All character codes that are valid in multipart boundaries. This is the /// intersection of the characters allowed in the `bcharsnospace` production /// defined in [RFC 2046][] and those allowed in the `token` production @@ -16,3 +18,20 @@ const List BOUNDARY_CHARACTERS = const [ 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122 ]; + +/// The total length of the multipart boundaries used when building the +/// request body. According to http://tools.ietf.org/html/rfc1341.html, this +/// can't be longer than 70. +const int BOUNDARY_LENGTH = 70; + +final Random _random = new Random(); + +/// Returns a randomly-generated multipart boundary string +String boundaryString() { + var prefix = "dart-http-boundary-"; + var list = new List.generate(BOUNDARY_LENGTH - prefix.length, + (index) => + BOUNDARY_CHARACTERS[_random.nextInt(BOUNDARY_CHARACTERS.length)], + growable: false); + return "$prefix${new String.fromCharCodes(list)}"; +} diff --git a/lib/src/multipart_request.dart b/lib/src/multipart_request.dart index 8132f80924..07712910d8 100644 --- a/lib/src/multipart_request.dart +++ b/lib/src/multipart_request.dart @@ -4,162 +4,143 @@ import 'dart:async'; import 'dart:convert'; -import 'dart:math'; -import 'base_request.dart'; -import 'boundary_characters.dart'; -import 'byte_stream.dart'; +import 'body.dart'; import 'multipart_file.dart'; import 'utils.dart'; final _newlineRegExp = new RegExp(r"\r\n|\r|\n"); -/// A `multipart/form-data` request. Such a request has both string [fields], -/// which function as normal form fields, and (potentially streamed) binary -/// [files]. -/// -/// This request automatically sets the Content-Type header to -/// `multipart/form-data`. This value will override any value set by the user. -/// -/// var uri = Uri.parse("http://pub.dartlang.org/packages/create"); -/// var request = new http.MultipartRequest("POST", url); -/// request.fields['user'] = 'nweiz@google.com'; -/// request.files.add(new http.MultipartFile.fromFile( -/// 'package', -/// new File('build/package.tar.gz'), -/// contentType: new MediaType('application', 'x-tar')); -/// request.send().then((response) { -/// if (response.statusCode == 200) print("Uploaded!"); -/// }); -class MultipartRequest extends BaseRequest { - /// The total length of the multipart boundaries used when building the - /// request body. According to http://tools.ietf.org/html/rfc1341.html, this - /// can't be longer than 70. - static const int _BOUNDARY_LENGTH = 70; - - static final Random _random = new Random(); - - /// The form fields to send for this request. - final Map fields; - - /// The private version of [files]. - final List _files; - - /// Creates a new [MultipartRequest]. - MultipartRequest(String method, Uri url) - : fields = {}, - _files = [], - super(method, url); - - /// The list of files to upload for this request. - List get files => _files; - - /// The total length of the request body, in bytes. This is calculated from - /// [fields] and [files] and cannot be set manually. - int get contentLength { - var length = 0; - - fields.forEach((name, value) { - length += "--".length + _BOUNDARY_LENGTH + "\r\n".length + - UTF8.encode(_headerForField(name, value)).length + - UTF8.encode(value).length + "\r\n".length; - }); - - for (var file in _files) { - length += "--".length + _BOUNDARY_LENGTH + "\r\n".length + - UTF8.encode(_headerForFile(file)).length + - file.length + "\r\n".length; - } - - return length + "--".length + _BOUNDARY_LENGTH + "--\r\n".length; - } - - void set contentLength(int value) { - throw new UnsupportedError("Cannot set the contentLength property of " - "multipart requests."); - } - - /// Freezes all mutable fields and returns a single-subscription [ByteStream] - /// that will emit the request body. - ByteStream finalize() { - // TODO(nweiz): freeze fields and files - var boundary = _boundaryString(); - headers['content-type'] = 'multipart/form-data; boundary=$boundary'; - super.finalize(); - +class MultipartBody implements Body { + /// The contents of the message body. + /// + /// This will be `null` after [read] is called. + Stream> _stream; + + /// The length of the stream returned by [read]. + /// + /// This is calculated from the fields and files passed into the body. + final int contentLength; + + /// Multipart forms do not have an encoding. + Encoding get encoding => null; + + /// Creates a [MultipartBody] from the given [fields] and [files]. + /// + /// The [boundary] is used to + factory MultipartBody( + Map fields, + List files, + String boundary, + ) { var controller = new StreamController>(sync: true); + var contentLength = 0; void writeAscii(String string) { - controller.add(UTF8.encode(string)); + controller.add(string.codeUnits); + contentLength += string.length; + } + void writeUtf8(String string) { + var encoded = UTF8.encode(string); + controller.add(encoded); + contentLength += encoded.length; + } + void writeLine() { + controller.add([13, 10]); // \r\n + contentLength += 2; } - writeUtf8(String string) => controller.add(UTF8.encode(string)); - writeLine() => controller.add([13, 10]); // \r\n - + // Write the fields to the stream fields.forEach((name, value) { writeAscii('--$boundary\r\n'); - writeAscii(_headerForField(name, value)); + writeUtf8(_headerForField(name, value)); writeUtf8(value); writeLine(); }); - Future.forEach(_files, (file) { - writeAscii('--$boundary\r\n'); - writeAscii(_headerForFile(file)); - return writeStreamToSink(file.finalize(), controller) - .then((_) => writeLine()); + // Iterate over the files to get the length + var fileCount = files.length; + var fileHeaders = new List>(fileCount); + + for (var i = 0; i < fileCount; ++i) { + var file = files[i]; + var header = []; + header.addAll('--$boundary\r\n'.codeUnits); + header.addAll(UTF8.encode(_headerForFile(file))); + contentLength += header.length + file.length + 2; + fileHeaders[i] = header; + } + + // Ending characters + var ending = '--$boundary--\r\n'.codeUnits; + contentLength += ending.length; + + // Write the files to the stream + // + // Future.forEach will ensure that the actions happen in sequence so i is + // used to get the fileHeaders + var i = 0; + + Future.forEach(files, (file) { + assert(files[i] == file); + controller.add(fileHeaders[i++]); + return writeStreamToSink(file.read(), controller).then((_) => controller.add([13, 10])); }).then((_) { // TODO(nweiz): pass any errors propagated through this future on to // the stream. See issue 3657. - writeAscii('--$boundary--\r\n'); + controller.add(ending); controller.close(); }); - return new ByteStream(controller.stream); - } - - /// Returns the header string for a field. The return value is guaranteed to - /// contain only ASCII characters. - String _headerForField(String name, String value) { - var header = - 'content-disposition: form-data; name="${_browserEncode(name)}"'; - if (!isPlainAscii(value)) { - header = '$header\r\n' - 'content-type: text/plain; charset=utf-8\r\n' - 'content-transfer-encoding: binary'; - } - return '$header\r\n\r\n'; + return new MultipartBody._(controller.stream, contentLength); } - /// Returns the header string for a file. The return value is guaranteed to - /// contain only ASCII characters. - String _headerForFile(MultipartFile file) { - var header = 'content-type: ${file.contentType}\r\n' - 'content-disposition: form-data; name="${_browserEncode(file.field)}"'; + MultipartBody._(this._stream, this.contentLength); - if (file.filename != null) { - header = '$header; filename="${_browserEncode(file.filename)}"'; + /// Returns a [Stream] representing the body. + /// + /// Can only be called once. + Stream> read() { + if (_stream == null) { + throw new StateError("The 'read' method can only be called once on a " + "http.Request/http.Response object."); } - return '$header\r\n\r\n'; + var stream = _stream; + _stream = null; + return stream; } +} - /// Encode [value] in the same way browsers do. - String _browserEncode(String value) { - // http://tools.ietf.org/html/rfc2388 mandates some complex encodings for - // field names and file names, but in practice user agents seem not to - // follow this at all. Instead, they URL-encode `\r`, `\n`, and `\r\n` as - // `\r\n`; URL-encode `"`; and do nothing else (even for `%` or non-ASCII - // characters). We follow their behavior. - return value.replaceAll(_newlineRegExp, "%0D%0A").replaceAll('"', "%22"); +/// Returns the header string for a field. +String _headerForField(String name, String value) { + var header = + 'content-disposition: form-data; name="${_browserEncode(name)}"'; + if (!isPlainAscii(value)) { + header = '$header\r\n' + 'content-type: text/plain; charset=utf-8\r\n' + 'content-transfer-encoding: binary'; } + return '$header\r\n\r\n'; +} - /// Returns a randomly-generated multipart boundary string - String _boundaryString() { - var prefix = "dart-http-boundary-"; - var list = new List.generate(_BOUNDARY_LENGTH - prefix.length, - (index) => - BOUNDARY_CHARACTERS[_random.nextInt(BOUNDARY_CHARACTERS.length)], - growable: false); - return "$prefix${new String.fromCharCodes(list)}"; +/// Returns the header string for a file. The return value is guaranteed to +/// contain only ASCII characters. +String _headerForFile(MultipartFile file) { + var header = 'content-type: ${file.contentType}\r\n' + 'content-disposition: form-data; name="${_browserEncode(file.field)}"'; + + if (file.filename != null) { + header = '$header; filename="${_browserEncode(file.filename)}"'; } + return '$header\r\n\r\n'; +} + +/// Encode [value] in the same way browsers do. +String _browserEncode(String value) { + // http://tools.ietf.org/html/rfc2388 mandates some complex encodings for + // field names and file names, but in practice user agents seem not to + // follow this at all. Instead, they URL-encode `\r`, `\n`, and `\r\n` as + // `\r\n`; URL-encode `"`; and do nothing else (even for `%` or non-ASCII + // characters). We follow their behavior. + return value.replaceAll(_newlineRegExp, "%0D%0A").replaceAll('"', "%22"); } diff --git a/lib/src/request.dart b/lib/src/request.dart index f49c5b9f16..89b5d33bbb 100644 --- a/lib/src/request.dart +++ b/lib/src/request.dart @@ -4,7 +4,10 @@ import 'dart:convert'; +import 'boundary_characters.dart'; import 'message.dart'; +import 'multipart_file.dart'; +import 'multipart_request.dart'; import 'utils.dart'; /// Represents an HTTP request to be sent to a server. @@ -130,6 +133,18 @@ class Request extends Message { {Map headers, Map context}) : this('DELETE', url, headers: headers, context: context); + factory Request.multipart(url, {Map headers, Map context, Map fields, List files,}) { + fields ??= {}; + files ??= []; + + var boundary = boundaryString(); + + headers ??= {}; + headers['content-type'] = 'multipart/form-data; boundary=$boundary'; + + return new Request._('POST', url, new MultipartBody(fields, files, boundary), null, headers, context,); + } + Request._(this.method, this.url, body, Encoding encoding, From 4f75ac7457be16515b6c2a81f18aed422e54858c Mon Sep 17 00:00:00 2001 From: Don Date: Wed, 27 Sep 2017 18:30:19 -0700 Subject: [PATCH 03/20] Fixing some nits --- lib/src/boundary_characters.dart | 37 ---------------- lib/src/multipart_file.dart | 19 +++++--- lib/src/multipart_request.dart | 75 ++++++++++++++++---------------- lib/src/request.dart | 40 ++++++++++++++++- 4 files changed, 88 insertions(+), 83 deletions(-) delete mode 100644 lib/src/boundary_characters.dart diff --git a/lib/src/boundary_characters.dart b/lib/src/boundary_characters.dart deleted file mode 100644 index 249acbd047..0000000000 --- a/lib/src/boundary_characters.dart +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright (c) 2013, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -import 'dart:math'; - -/// All character codes that are valid in multipart boundaries. This is the -/// intersection of the characters allowed in the `bcharsnospace` production -/// defined in [RFC 2046][] and those allowed in the `token` production -/// defined in [RFC 1521][]. -/// -/// [RFC 2046]: http://tools.ietf.org/html/rfc2046#section-5.1.1. -/// [RFC 1521]: https://tools.ietf.org/html/rfc1521#section-4 -const List BOUNDARY_CHARACTERS = const [ - 43, 95, 45, 46, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, - 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, - 84, 85, 86, 87, 88, 89, 90, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, - 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, - 122 -]; - -/// The total length of the multipart boundaries used when building the -/// request body. According to http://tools.ietf.org/html/rfc1341.html, this -/// can't be longer than 70. -const int BOUNDARY_LENGTH = 70; - -final Random _random = new Random(); - -/// Returns a randomly-generated multipart boundary string -String boundaryString() { - var prefix = "dart-http-boundary-"; - var list = new List.generate(BOUNDARY_LENGTH - prefix.length, - (index) => - BOUNDARY_CHARACTERS[_random.nextInt(BOUNDARY_CHARACTERS.length)], - growable: false); - return "$prefix${new String.fromCharCodes(list)}"; -} diff --git a/lib/src/multipart_file.dart b/lib/src/multipart_file.dart index 0736886bae..31675047dd 100644 --- a/lib/src/multipart_file.dart +++ b/lib/src/multipart_file.dart @@ -22,19 +22,24 @@ class MultipartFile { /// The name of the form field for the file. final String field; - /// The size of the file in bytes. This must be known in advance, even if this - /// file is created from a [Stream]. + /// The size of the file in bytes. + /// + /// This must be known in advance, even if this file is created from a + /// [Stream]. final int length; /// The basename of the file. May be null. final String filename; - /// The content-type of the file. Defaults to `application/octet-stream`. + /// The content-type of the file. + /// + /// Defaults to `application/octet-stream`. final MediaType contentType; - /// Creates a new [MultipartFile] from a chunked [Stream] of bytes. The length - /// of the file in bytes must be known in advance. If it's not, read the data - /// from the stream and use [MultipartFile.fromBytes] instead. + /// Creates a new [MultipartFile] from a chunked [Stream] of bytes. + /// + /// The length of the file in bytes must be known in advance. If it's not, + /// read the data from the stream and use [MultipartFile.fromBytes] instead. /// /// [contentType] currently defaults to `application/octet-stream`, but in the /// future may be inferred from [filename]. @@ -92,7 +97,7 @@ class MultipartFile { filename: filename, contentType: contentType); } - /// Returns a [Stream] representing the file. + /// Returns a [Stream] representing the contents of the file. /// /// Can only be called once. Stream> read() { diff --git a/lib/src/multipart_request.dart b/lib/src/multipart_request.dart index 07712910d8..2062f32af3 100644 --- a/lib/src/multipart_request.dart +++ b/lib/src/multipart_request.dart @@ -4,6 +4,7 @@ import 'dart:async'; import 'dart:convert'; +import 'dart:math'; import 'body.dart'; import 'multipart_file.dart'; @@ -28,11 +29,9 @@ class MultipartBody implements Body { /// Creates a [MultipartBody] from the given [fields] and [files]. /// /// The [boundary] is used to - factory MultipartBody( - Map fields, - List files, - String boundary, - ) { + factory MultipartBody(Map fields, + List files, + String boundary,) { var controller = new StreamController>(sync: true); var contentLength = 0; @@ -84,7 +83,8 @@ class MultipartBody implements Body { Future.forEach(files, (file) { assert(files[i] == file); controller.add(fileHeaders[i++]); - return writeStreamToSink(file.read(), controller).then((_) => controller.add([13, 10])); + return writeStreamToSink(file.read(), controller).then((_) => + controller.add([13, 10])); }).then((_) { // TODO(nweiz): pass any errors propagated through this future on to // the stream. See issue 3657. @@ -109,38 +109,39 @@ class MultipartBody implements Body { _stream = null; return stream; } -} - -/// Returns the header string for a field. -String _headerForField(String name, String value) { - var header = - 'content-disposition: form-data; name="${_browserEncode(name)}"'; - if (!isPlainAscii(value)) { - header = '$header\r\n' - 'content-type: text/plain; charset=utf-8\r\n' - 'content-transfer-encoding: binary'; + + /// Returns the header string for a field. + static String _headerForField(String name, String value) { + var header = + 'content-disposition: form-data; name="${_browserEncode(name)}"'; + if (!isPlainAscii(value)) { + header = '$header\r\n' + 'content-type: text/plain; charset=utf-8\r\n' + 'content-transfer-encoding: binary'; + } + return '$header\r\n\r\n'; } - return '$header\r\n\r\n'; -} -/// Returns the header string for a file. The return value is guaranteed to -/// contain only ASCII characters. -String _headerForFile(MultipartFile file) { - var header = 'content-type: ${file.contentType}\r\n' - 'content-disposition: form-data; name="${_browserEncode(file.field)}"'; + /// Returns the header string for a file. + /// + /// The return value is guaranteed to contain only ASCII characters. + static String _headerForFile(MultipartFile file) { + var header = 'content-type: ${file.contentType}\r\n' + 'content-disposition: form-data; name="${_browserEncode(file.field)}"'; + + if (file.filename != null) { + header = '$header; filename="${_browserEncode(file.filename)}"'; + } + return '$header\r\n\r\n'; + } - if (file.filename != null) { - header = '$header; filename="${_browserEncode(file.filename)}"'; + /// Encode [value] in the same way browsers do. + static String _browserEncode(String value) { + // http://tools.ietf.org/html/rfc2388 mandates some complex encodings for + // field names and file names, but in practice user agents seem not to + // follow this at all. Instead, they URL-encode `\r`, `\n`, and `\r\n` as + // `\r\n`; URL-encode `"`; and do nothing else (even for `%` or non-ASCII + // characters). We follow their behavior. + return value.replaceAll(_newlineRegExp, "%0D%0A").replaceAll('"', "%22"); } - return '$header\r\n\r\n'; -} - -/// Encode [value] in the same way browsers do. -String _browserEncode(String value) { - // http://tools.ietf.org/html/rfc2388 mandates some complex encodings for - // field names and file names, but in practice user agents seem not to - // follow this at all. Instead, they URL-encode `\r`, `\n`, and `\r\n` as - // `\r\n`; URL-encode `"`; and do nothing else (even for `%` or non-ASCII - // characters). We follow their behavior. - return value.replaceAll(_newlineRegExp, "%0D%0A").replaceAll('"', "%22"); -} +} \ No newline at end of file diff --git a/lib/src/request.dart b/lib/src/request.dart index 89b5d33bbb..4694d577fe 100644 --- a/lib/src/request.dart +++ b/lib/src/request.dart @@ -3,8 +3,8 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:convert'; +import 'dart:math'; -import 'boundary_characters.dart'; import 'message.dart'; import 'multipart_file.dart'; import 'multipart_request.dart'; @@ -137,7 +137,7 @@ class Request extends Message { fields ??= {}; files ??= []; - var boundary = boundaryString(); + var boundary = _boundaryString(); headers ??= {}; headers['content-type'] = 'multipart/form-data; boundary=$boundary'; @@ -178,4 +178,40 @@ class Request extends Message { updatedHeaders, updatedContext); } + + + /// All character codes that are valid in multipart boundaries. + /// + /// This is the intersection of the characters allowed in the `bcharsnospace` + /// production defined in [RFC 2046][] and those allowed in the `token` + /// production defined in [RFC 1521][]. + /// + /// [RFC 2046]: http://tools.ietf.org/html/rfc2046#section-5.1.1. + /// [RFC 1521]: https://tools.ietf.org/html/rfc1521#section-4 + static const List _boundaryCharacters = const [ + 43, 95, 45, 46, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, + 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, + 84, 85, 86, 87, 88, 89, 90, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, + 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, + 122 + ]; + + /// The total length of the multipart boundaries used when building the + /// request body. + /// + /// According to http://tools.ietf.org/html/rfc1341.html, this can't be longer + /// than 70. + static const int _boundaryLength = 70; + + static final Random _random = new Random(); + + /// Returns a randomly-generated multipart boundary string + static String _boundaryString() { + var prefix = "dart-http-boundary-"; + var list = new List.generate(_boundaryLength - prefix.length, + (index) => + _boundaryCharacters[_random.nextInt(_boundaryCharacters.length)], + growable: false); + return "$prefix${new String.fromCharCodes(list)}"; + } } From 8592a0ed20b2e3204de1fb0312ac532720011e2f Mon Sep 17 00:00:00 2001 From: Don Date: Wed, 27 Sep 2017 18:45:28 -0700 Subject: [PATCH 04/20] Additional nits --- lib/src/multipart_request.dart | 18 +++++++++--------- lib/src/request.dart | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/src/multipart_request.dart b/lib/src/multipart_request.dart index 2062f32af3..2aceb08576 100644 --- a/lib/src/multipart_request.dart +++ b/lib/src/multipart_request.dart @@ -4,14 +4,11 @@ import 'dart:async'; import 'dart:convert'; -import 'dart:math'; import 'body.dart'; import 'multipart_file.dart'; import 'utils.dart'; -final _newlineRegExp = new RegExp(r"\r\n|\r|\n"); - class MultipartBody implements Body { /// The contents of the message body. /// @@ -49,7 +46,7 @@ class MultipartBody implements Body { contentLength += 2; } - // Write the fields to the stream + // Write the fields to the stream. fields.forEach((name, value) { writeAscii('--$boundary\r\n'); writeUtf8(_headerForField(name, value)); @@ -57,7 +54,8 @@ class MultipartBody implements Body { writeLine(); }); - // Iterate over the files to get the length + // Iterate over the files to get the length and compute the headers ahead + // time so the length can be synchronously accessed. var fileCount = files.length; var fileHeaders = new List>(fileCount); @@ -70,14 +68,14 @@ class MultipartBody implements Body { fileHeaders[i] = header; } - // Ending characters + // Ending characters. var ending = '--$boundary--\r\n'.codeUnits; contentLength += ending.length; - // Write the files to the stream + // Write the files to the stream. // // Future.forEach will ensure that the actions happen in sequence so i is - // used to get the fileHeaders + // used to get the fileHeaders. var i = 0; Future.forEach(files, (file) { @@ -135,6 +133,8 @@ class MultipartBody implements Body { return '$header\r\n\r\n'; } + static final _newlineRegExp = new RegExp(r"\r\n|\r|\n"); + /// Encode [value] in the same way browsers do. static String _browserEncode(String value) { // http://tools.ietf.org/html/rfc2388 mandates some complex encodings for @@ -144,4 +144,4 @@ class MultipartBody implements Body { // characters). We follow their behavior. return value.replaceAll(_newlineRegExp, "%0D%0A").replaceAll('"', "%22"); } -} \ No newline at end of file +} diff --git a/lib/src/request.dart b/lib/src/request.dart index 4694d577fe..bb691a2cc3 100644 --- a/lib/src/request.dart +++ b/lib/src/request.dart @@ -133,7 +133,7 @@ class Request extends Message { {Map headers, Map context}) : this('DELETE', url, headers: headers, context: context); - factory Request.multipart(url, {Map headers, Map context, Map fields, List files,}) { + factory Request.multipart(url, {Map headers, Map context, Map fields, Iterable files}) { fields ??= {}; files ??= []; From 19dcfde96c1a558ec0df0555c31395f0a43205d1 Mon Sep 17 00:00:00 2001 From: Don Date: Wed, 27 Sep 2017 18:49:49 -0700 Subject: [PATCH 05/20] Formatting --- lib/src/request.dart | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/src/request.dart b/lib/src/request.dart index bb691a2cc3..fbfeb0a7a0 100644 --- a/lib/src/request.dart +++ b/lib/src/request.dart @@ -133,7 +133,11 @@ class Request extends Message { {Map headers, Map context}) : this('DELETE', url, headers: headers, context: context); - factory Request.multipart(url, {Map headers, Map context, Map fields, Iterable files}) { + factory Request.multipart(url, + {Map headers, + Map context, + Map fields, + Iterable files}) { fields ??= {}; files ??= []; @@ -142,7 +146,14 @@ class Request extends Message { headers ??= {}; headers['content-type'] = 'multipart/form-data; boundary=$boundary'; - return new Request._('POST', url, new MultipartBody(fields, files, boundary), null, headers, context,); + return new Request._( + 'POST', + url, + new MultipartBody(fields, files, boundary), + null, + headers, + context, + ); } Request._(this.method, this.url, From 69f6d956d6a09b10a870ec2942e653522c5ca15e Mon Sep 17 00:00:00 2001 From: Don Date: Wed, 27 Sep 2017 19:21:39 -0700 Subject: [PATCH 06/20] Rename --- lib/src/{multipart_request.dart => multipart_body.dart} | 0 lib/src/request.dart | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename lib/src/{multipart_request.dart => multipart_body.dart} (100%) diff --git a/lib/src/multipart_request.dart b/lib/src/multipart_body.dart similarity index 100% rename from lib/src/multipart_request.dart rename to lib/src/multipart_body.dart diff --git a/lib/src/request.dart b/lib/src/request.dart index fbfeb0a7a0..d056d78868 100644 --- a/lib/src/request.dart +++ b/lib/src/request.dart @@ -7,7 +7,7 @@ import 'dart:math'; import 'message.dart'; import 'multipart_file.dart'; -import 'multipart_request.dart'; +import 'multipart_body.dart'; import 'utils.dart'; /// Represents an HTTP request to be sent to a server. From 26e7bf962992dd416ce726d24046c4ad2cda5a00 Mon Sep 17 00:00:00 2001 From: Don Date: Thu, 19 Oct 2017 14:04:38 -0700 Subject: [PATCH 07/20] Use updateMap to add content type --- lib/src/request.dart | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/src/request.dart b/lib/src/request.dart index d056d78868..6563dfce01 100644 --- a/lib/src/request.dart +++ b/lib/src/request.dart @@ -143,17 +143,15 @@ class Request extends Message { var boundary = _boundaryString(); - headers ??= {}; - headers['content-type'] = 'multipart/form-data; boundary=$boundary'; - return new Request._( - 'POST', - url, - new MultipartBody(fields, files, boundary), - null, - headers, - context, - ); + 'POST', + url, + new MultipartBody(fields, files, boundary), + null, + updateMap(headers, { + 'content-type': 'multipart/form-data; boundary=$boundary' + }), + context); } Request._(this.method, this.url, From 20afe261de24d97bf484ce5d6d6e3e2bd919b739 Mon Sep 17 00:00:00 2001 From: Don Date: Thu, 19 Oct 2017 16:16:03 -0700 Subject: [PATCH 08/20] Add comment to control dartfmt of boundary characters --- lib/src/request.dart | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/src/request.dart b/lib/src/request.dart index 6563dfce01..17dee0bea4 100644 --- a/lib/src/request.dart +++ b/lib/src/request.dart @@ -188,7 +188,6 @@ class Request extends Message { updatedContext); } - /// All character codes that are valid in multipart boundaries. /// /// This is the intersection of the characters allowed in the `bcharsnospace` @@ -198,10 +197,10 @@ class Request extends Message { /// [RFC 2046]: http://tools.ietf.org/html/rfc2046#section-5.1.1. /// [RFC 1521]: https://tools.ietf.org/html/rfc1521#section-4 static const List _boundaryCharacters = const [ - 43, 95, 45, 46, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, - 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, - 84, 85, 86, 87, 88, 89, 90, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, - 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, + 43, 95, 45, 46, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 65, 66, 67, 68, // + 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, // + 87, 88, 89, 90, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, // + 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, // 122 ]; From 7788518e72d41be8f11fde8a7aa4858104209a69 Mon Sep 17 00:00:00 2001 From: Don Date: Fri, 20 Oct 2017 16:32:23 -0700 Subject: [PATCH 09/20] nits --- lib/src/multipart_body.dart | 9 +++++---- lib/src/multipart_file.dart | 12 +++++------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/src/multipart_body.dart b/lib/src/multipart_body.dart index 2aceb08576..16f7ffd94c 100644 --- a/lib/src/multipart_body.dart +++ b/lib/src/multipart_body.dart @@ -27,8 +27,7 @@ class MultipartBody implements Body { /// /// The [boundary] is used to factory MultipartBody(Map fields, - List files, - String boundary,) { + Iterable files, String boundary) { var controller = new StreamController>(sync: true); var contentLength = 0; @@ -36,11 +35,13 @@ class MultipartBody implements Body { controller.add(string.codeUnits); contentLength += string.length; } + void writeUtf8(String string) { var encoded = UTF8.encode(string); controller.add(encoded); contentLength += encoded.length; } + void writeLine() { controller.add([13, 10]); // \r\n contentLength += 2; @@ -81,8 +82,8 @@ class MultipartBody implements Body { Future.forEach(files, (file) { assert(files[i] == file); controller.add(fileHeaders[i++]); - return writeStreamToSink(file.read(), controller).then((_) => - controller.add([13, 10])); + return writeStreamToSink(file.read(), controller) + .then((_) => controller.add([13, 10])); }).then((_) { // TODO(nweiz): pass any errors propagated through this future on to // the stream. See issue 3657. diff --git a/lib/src/multipart_file.dart b/lib/src/multipart_file.dart index 31675047dd..d3cc44398a 100644 --- a/lib/src/multipart_file.dart +++ b/lib/src/multipart_file.dart @@ -11,7 +11,7 @@ import 'package:collection/collection.dart'; import 'package:http_parser/http_parser.dart'; import 'package:path/path.dart' as path; -import 'utils.dart'; +import 'content_type.dart'; /// A file to be uploaded as part of a [MultipartRequest]. This doesn't need to /// correspond to a physical file. @@ -46,9 +46,8 @@ class MultipartFile { MultipartFile(this.field, Stream> stream, this.length, {this.filename, MediaType contentType}) : this._stream = stream, - this.contentType = contentType != null - ? contentType - : new MediaType("application", "octet-stream"); + this.contentType = + contentType ?? new MediaType("application", "octet-stream"); /// Creates a new [MultipartFile] from a byte array. /// @@ -69,9 +68,8 @@ class MultipartFile { /// the future may be inferred from [filename]. factory MultipartFile.fromString(String field, String value, {String filename, MediaType contentType}) { - contentType = - contentType == null ? new MediaType("text", "plain") : contentType; - var encoding = encodingForCharset(contentType.parameters['charset'], UTF8); + contentType ??= new MediaType("text", "plain"); + var encoding = encodingForMediaType(contentType) ?? UTF8; contentType = contentType.change(parameters: {'charset': encoding.name}); return new MultipartFile.fromBytes(field, encoding.encode(value), From 55b1666144febf8aa5a78cdfc08c007e6bb4cce4 Mon Sep 17 00:00:00 2001 From: Don Date: Tue, 24 Oct 2017 16:48:39 -0700 Subject: [PATCH 10/20] Add and pass tests --- lib/src/multipart_body.dart | 2 +- lib/src/request.dart | 1 + test/multipart_test.dart | 249 ++++++++++++++++++++++++++++++++++++ test/utils.dart | 39 +++++- 4 files changed, 288 insertions(+), 3 deletions(-) create mode 100644 test/multipart_test.dart diff --git a/lib/src/multipart_body.dart b/lib/src/multipart_body.dart index 16f7ffd94c..3697bb0bf0 100644 --- a/lib/src/multipart_body.dart +++ b/lib/src/multipart_body.dart @@ -27,7 +27,7 @@ class MultipartBody implements Body { /// /// The [boundary] is used to factory MultipartBody(Map fields, - Iterable files, String boundary) { + List files, String boundary) { var controller = new StreamController>(sync: true); var contentLength = 0; diff --git a/lib/src/request.dart b/lib/src/request.dart index 17dee0bea4..9bcc6b6d5d 100644 --- a/lib/src/request.dart +++ b/lib/src/request.dart @@ -140,6 +140,7 @@ class Request extends Message { Iterable files}) { fields ??= {}; files ??= []; + headers ??= {}; var boundary = _boundaryString(); diff --git a/test/multipart_test.dart b/test/multipart_test.dart new file mode 100644 index 0000000000..ba782396df --- /dev/null +++ b/test/multipart_test.dart @@ -0,0 +1,249 @@ +// Copyright (c) 2013, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:async'; + +import 'package:http/http.dart' as http; +import 'package:http_parser/http_parser.dart'; +import 'package:test/test.dart'; + +import 'utils.dart'; + +void main() { + test('empty', () { + var request = new http.Request.multipart(dummyUrl); + expect(request, bodyMatches(''' + --{{boundary}}-- + ''')); + }); + + test('with fields and files', () { + var fields = { + 'field1': 'value1', + 'field2': 'value2', + }; + var files = [ + new http.MultipartFile.fromString("file1", "contents1", + filename: "filename1.txt"), + new http.MultipartFile.fromString("file2", "contents2"), + ]; + + var request = + new http.Request.multipart(dummyUrl, fields: fields, files: files); + + expect(request, bodyMatches(''' + --{{boundary}} + content-disposition: form-data; name="field1" + + value1 + --{{boundary}} + content-disposition: form-data; name="field2" + + value2 + --{{boundary}} + content-type: text/plain; charset=utf-8 + content-disposition: form-data; name="file1"; filename="filename1.txt" + + contents1 + --{{boundary}} + content-type: text/plain; charset=utf-8 + content-disposition: form-data; name="file2" + + contents2 + --{{boundary}}-- + ''')); + }); + + test('with a unicode field name', () { + var fields = {'fïēld': 'value'}; + + var request = new http.Request.multipart(dummyUrl, fields: fields); + + expect(request, bodyMatches(''' + --{{boundary}} + content-disposition: form-data; name="fïēld" + + value + --{{boundary}}-- + ''')); + }); + + test('with a field name with newlines', () { + var fields = {'foo\nbar\rbaz\r\nbang': 'value'}; + var request = new http.Request.multipart(dummyUrl, fields: fields); + + expect(request, bodyMatches(''' + --{{boundary}} + content-disposition: form-data; name="foo%0D%0Abar%0D%0Abaz%0D%0Abang" + + value + --{{boundary}}-- + ''')); + }); + + test('with a field name with a quote', () { + var fields = {'foo"bar': 'value'}; + var request = new http.Request.multipart(dummyUrl, fields: fields); + + expect(request, bodyMatches(''' + --{{boundary}} + content-disposition: form-data; name="foo%22bar" + + value + --{{boundary}}-- + ''')); + }); + + test('with a unicode field value', () { + var fields = {'field': 'vⱥlūe'}; + var request = new http.Request.multipart(dummyUrl, fields: fields); + + expect(request, bodyMatches(''' + --{{boundary}} + content-disposition: form-data; name="field" + content-type: text/plain; charset=utf-8 + content-transfer-encoding: binary + + vⱥlūe + --{{boundary}}-- + ''')); + }); + + test('with a unicode filename', () { + var files = [ + new http.MultipartFile.fromString('file', 'contents', + filename: 'fïlēname.txt') + ]; + var request = new http.Request.multipart(dummyUrl, files: files); + + expect(request, bodyMatches(''' + --{{boundary}} + content-type: text/plain; charset=utf-8 + content-disposition: form-data; name="file"; filename="fïlēname.txt" + + contents + --{{boundary}}-- + ''')); + }); + + test('with a filename with newlines', () { + var files = [ + new http.MultipartFile.fromString('file', 'contents', + filename: 'foo\nbar\rbaz\r\nbang') + ]; + var request = new http.Request.multipart(dummyUrl, files: files); + + expect(request, bodyMatches(''' + --{{boundary}} + content-type: text/plain; charset=utf-8 + content-disposition: form-data; name="file"; filename="foo%0D%0Abar%0D%0Abaz%0D%0Abang" + + contents + --{{boundary}}-- + ''')); + }); + + test('with a filename with a quote', () { + var files = [ + new http.MultipartFile.fromString('file', 'contents', filename: 'foo"bar') + ]; + var request = new http.Request.multipart(dummyUrl, files: files); + + expect(request, bodyMatches(''' + --{{boundary}} + content-type: text/plain; charset=utf-8 + content-disposition: form-data; name="file"; filename="foo%22bar" + + contents + --{{boundary}}-- + ''')); + }); + + test('with a string file with a content-type but no charset', () { + var files = [ + new http.MultipartFile.fromString('file', '{"hello": "world"}', + contentType: new MediaType('application', 'json')) + ]; + var request = new http.Request.multipart(dummyUrl, files: files); + + expect(request, bodyMatches(''' + --{{boundary}} + content-type: application/json; charset=utf-8 + content-disposition: form-data; name="file" + + {"hello": "world"} + --{{boundary}}-- + ''')); + }); + + test('with a file with a iso-8859-1 body', () { + // "Ã¥" encoded as ISO-8859-1 and then read as UTF-8 results in "å". + var files = [ + new http.MultipartFile.fromString('file', 'non-ascii: "Ã¥"', + contentType: + new MediaType('text', 'plain', {'charset': 'iso-8859-1'})) + ]; + var request = new http.Request.multipart(dummyUrl, files: files); + + expect(request, bodyMatches(''' + --{{boundary}} + content-type: text/plain; charset=iso-8859-1 + content-disposition: form-data; name="file" + + non-ascii: "å" + --{{boundary}}-- + ''')); + }); + + test('with a stream file', () { + var controller = new StreamController(sync: true); + var files = [new http.MultipartFile('file', controller.stream, 5)]; + var request = new http.Request.multipart(dummyUrl, files: files); + + expect(request, bodyMatches(''' + --{{boundary}} + content-type: application/octet-stream + content-disposition: form-data; name="file" + + hello + --{{boundary}}-- + ''')); + + controller.add([104, 101, 108, 108, 111]); + controller.close(); + }); + + test('with an empty stream file', () { + var controller = new StreamController(sync: true); + var files = [new http.MultipartFile('file', controller.stream, 0)]; + var request = new http.Request.multipart(dummyUrl, files: files); + + expect(request, bodyMatches(''' + --{{boundary}} + content-type: application/octet-stream + content-disposition: form-data; name="file" + + + --{{boundary}}-- + ''')); + + controller.close(); + }); + + test('with a byte file', () { + var files = [ + new http.MultipartFile.fromBytes('file', [104, 101, 108, 108, 111]) + ]; + var request = new http.Request.multipart(dummyUrl, files: files); + + expect(request, bodyMatches(''' + --{{boundary}} + content-type: application/octet-stream + content-disposition: form-data; name="file" + + hello + --{{boundary}}-- + ''')); + }); +} diff --git a/test/utils.dart b/test/utils.dart index 505ee5fda3..a8858ad29c 100644 --- a/test/utils.dart +++ b/test/utils.dart @@ -4,9 +4,10 @@ import 'dart:convert'; -import 'package:test/test.dart'; - +import 'package:async/async.dart'; import 'package:http/http.dart' as http; +import 'package:http_parser/http_parser.dart'; +import 'package:test/test.dart'; /// A dummy URL for constructing requests that won't be sent. Uri get dummyUrl => Uri.parse('http://dartlang.org/'); @@ -66,6 +67,40 @@ class _Parse extends Matcher { } } +/// A matcher that validates the body of a multipart request after finalization. +/// The string "{{boundary}}" in [pattern] will be replaced by the boundary +/// string for the request, and LF newlines will be replaced with CRLF. +/// Indentation will be normalized. +Matcher bodyMatches(String pattern) => new _BodyMatches(pattern); + +class _BodyMatches extends Matcher { + final String _pattern; + + _BodyMatches(this._pattern); + + bool matches(item, Map matchState) { + if (item is! http.Request) return false; + + var future = collectBytes(item.read()).then((bodyBytes) { + var body = UTF8.decode(bodyBytes); + var contentType = new MediaType.parse(item.headers['content-type']); + var boundary = contentType.parameters['boundary']; + var expected = cleanUpLiteral(_pattern) + .replaceAll("\n", "\r\n") + .replaceAll("{{boundary}}", boundary); + + expect(body, equals(expected)); + expect(item.contentLength, equals(bodyBytes.length)); + }); + + return completes.matches(future, matchState); + } + + Description describe(Description description) { + return description.add('has a body that matches "$_pattern"'); + } +} + /// A matcher that matches a [http.ClientException] with the given [message]. /// /// [message] can be a String or a [Matcher]. From 4866d51166e4d3a23aa1ea91ce5ae26733cea37f Mon Sep 17 00:00:00 2001 From: Don Date: Tue, 24 Oct 2017 17:42:49 -0700 Subject: [PATCH 11/20] dartfmt --- lib/src/multipart_body.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/multipart_body.dart b/lib/src/multipart_body.dart index 3697bb0bf0..13045b917b 100644 --- a/lib/src/multipart_body.dart +++ b/lib/src/multipart_body.dart @@ -26,8 +26,8 @@ class MultipartBody implements Body { /// Creates a [MultipartBody] from the given [fields] and [files]. /// /// The [boundary] is used to - factory MultipartBody(Map fields, - List files, String boundary) { + factory MultipartBody( + Map fields, List files, String boundary) { var controller = new StreamController>(sync: true); var contentLength = 0; From 7db4f18042657ad387ac5a4fc3aaefcc72c03398 Mon Sep 17 00:00:00 2001 From: Don Date: Tue, 24 Oct 2017 17:54:28 -0700 Subject: [PATCH 12/20] Use Uint8Buffer --- lib/src/multipart_body.dart | 33 ++++++++++++++++++--------------- pubspec.yaml | 1 + 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/lib/src/multipart_body.dart b/lib/src/multipart_body.dart index 13045b917b..efb1f75a01 100644 --- a/lib/src/multipart_body.dart +++ b/lib/src/multipart_body.dart @@ -5,6 +5,8 @@ import 'dart:async'; import 'dart:convert'; +import 'package:typed_data/typed_buffers.dart'; + import 'body.dart'; import 'multipart_file.dart'; import 'utils.dart'; @@ -26,25 +28,21 @@ class MultipartBody implements Body { /// Creates a [MultipartBody] from the given [fields] and [files]. /// /// The [boundary] is used to - factory MultipartBody( - Map fields, List files, String boundary) { + factory MultipartBody(Map fields, + Iterable files, String boundary) { var controller = new StreamController>(sync: true); - var contentLength = 0; + var buffer = new Uint8Buffer(); void writeAscii(String string) { - controller.add(string.codeUnits); - contentLength += string.length; + buffer.addAll(string.codeUnits); } void writeUtf8(String string) { - var encoded = UTF8.encode(string); - controller.add(encoded); - contentLength += encoded.length; + buffer.addAll(UTF8.encode(string)); } void writeLine() { - controller.add([13, 10]); // \r\n - contentLength += 2; + buffer.addAll([13, 10]); // \r\n } // Write the fields to the stream. @@ -55,23 +53,27 @@ class MultipartBody implements Body { writeLine(); }); + controller.add(buffer); + // Iterate over the files to get the length and compute the headers ahead // time so the length can be synchronously accessed. + var fileList = files.toList(); var fileCount = files.length; var fileHeaders = new List>(fileCount); + var fileContentsLength = 0; for (var i = 0; i < fileCount; ++i) { - var file = files[i]; + var file = fileList[i]; var header = []; header.addAll('--$boundary\r\n'.codeUnits); header.addAll(UTF8.encode(_headerForFile(file))); - contentLength += header.length + file.length + 2; + fileContentsLength += header.length + file.length + 2; fileHeaders[i] = header; } // Ending characters. var ending = '--$boundary--\r\n'.codeUnits; - contentLength += ending.length; + fileContentsLength += ending.length; // Write the files to the stream. // @@ -80,7 +82,7 @@ class MultipartBody implements Body { var i = 0; Future.forEach(files, (file) { - assert(files[i] == file); + assert(fileList[i] == file); controller.add(fileHeaders[i++]); return writeStreamToSink(file.read(), controller) .then((_) => controller.add([13, 10])); @@ -91,7 +93,8 @@ class MultipartBody implements Body { controller.close(); }); - return new MultipartBody._(controller.stream, contentLength); + return new MultipartBody._( + controller.stream, buffer.length + fileContentsLength); } MultipartBody._(this._stream, this.contentLength); diff --git a/pubspec.yaml b/pubspec.yaml index 958b2babf8..503f083bf5 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -8,6 +8,7 @@ dependencies: collection: "^1.5.0" http_parser: ">=0.0.1 <4.0.0" path: ">=0.9.0 <2.0.0" + typed_data: "^1.1.5" dev_dependencies: test: "^0.12.18" # Override dependency on package_resolver to enable test From 9609d5600a11ebda40badd8af2a42adc86a6ba90 Mon Sep 17 00:00:00 2001 From: Don Date: Tue, 24 Oct 2017 18:06:11 -0700 Subject: [PATCH 13/20] Use async function for writing files --- lib/src/multipart_body.dart | 46 +++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/lib/src/multipart_body.dart b/lib/src/multipart_body.dart index efb1f75a01..db1921f90b 100644 --- a/lib/src/multipart_body.dart +++ b/lib/src/multipart_body.dart @@ -11,6 +11,9 @@ import 'body.dart'; import 'multipart_file.dart'; import 'utils.dart'; +/// A `multipart/form-data` request [Body]. Such a request has both string +/// fields, which function as normal form fields, and (potentially streamed) +/// binary files. class MultipartBody implements Body { /// The contents of the message body. /// @@ -58,40 +61,23 @@ class MultipartBody implements Body { // Iterate over the files to get the length and compute the headers ahead // time so the length can be synchronously accessed. var fileList = files.toList(); - var fileCount = files.length; - var fileHeaders = new List>(fileCount); + var fileHeaders = >[]; var fileContentsLength = 0; - for (var i = 0; i < fileCount; ++i) { - var file = fileList[i]; + for (var file in fileList) { var header = []; header.addAll('--$boundary\r\n'.codeUnits); header.addAll(UTF8.encode(_headerForFile(file))); fileContentsLength += header.length + file.length + 2; - fileHeaders[i] = header; + fileHeaders.add(header); } // Ending characters. var ending = '--$boundary--\r\n'.codeUnits; fileContentsLength += ending.length; - // Write the files to the stream. - // - // Future.forEach will ensure that the actions happen in sequence so i is - // used to get the fileHeaders. - var i = 0; - - Future.forEach(files, (file) { - assert(fileList[i] == file); - controller.add(fileHeaders[i++]); - return writeStreamToSink(file.read(), controller) - .then((_) => controller.add([13, 10])); - }).then((_) { - // TODO(nweiz): pass any errors propagated through this future on to - // the stream. See issue 3657. - controller.add(ending); - controller.close(); - }); + // Write the files to the stream asynchronously. + _writeFilesToStream(controller, files, fileHeaders, ending); return new MultipartBody._( controller.stream, buffer.length + fileContentsLength); @@ -112,6 +98,22 @@ class MultipartBody implements Body { return stream; } + /// Writes the [files] to the [controller]. + static Future _writeFilesToStream( + StreamController> controller, + List files, + List> fileHeaders, + List ending) async { + for (var i = 0; i < files.length; ++i) { + controller.add(fileHeaders[i]); + await writeStreamToSink(files[i].read(), controller); + controller.add([13, 10]); + } + + controller.add(ending); + controller.close(); + } + /// Returns the header string for a field. static String _headerForField(String name, String value) { var header = From 39667b7fa2e6fbb7b3ddf10d17c3f9866a6ed365 Mon Sep 17 00:00:00 2001 From: Don Date: Wed, 25 Oct 2017 11:42:38 -0700 Subject: [PATCH 14/20] Fixing review comments --- lib/src/boundary.dart | 41 ++++++++++++++ lib/src/multipart_body.dart | 52 +++++++++--------- lib/src/multipart_file.dart | 5 +- lib/src/request.dart | 104 ++++++++++++------------------------ pubspec.yaml | 2 +- test/multipart_test.dart | 45 ++++++++-------- test/utils.dart | 34 ++++++------ 7 files changed, 148 insertions(+), 135 deletions(-) create mode 100644 lib/src/boundary.dart diff --git a/lib/src/boundary.dart b/lib/src/boundary.dart new file mode 100644 index 0000000000..6dd352daad --- /dev/null +++ b/lib/src/boundary.dart @@ -0,0 +1,41 @@ +// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:math'; + +/// All character codes that are valid in multipart boundaries. +/// +/// This is the intersection of the characters allowed in the `bcharsnospace` +/// production defined in [RFC 2046][] and those allowed in the `token` +/// production defined in [RFC 1521][]. +/// +/// [RFC 2046]: http://tools.ietf.org/html/rfc2046#section-5.1.1. +/// [RFC 1521]: https://tools.ietf.org/html/rfc1521#section-4 +const List _boundaryCharacters = const [ + 43, 95, 45, 46, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 65, 66, 67, 68, // + 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, + 87, 88, 89, 90, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, + 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, + 122 +]; + +/// The total length of the multipart boundaries used when building the +/// request body. +/// +/// According to http://tools.ietf.org/html/rfc1341.html, this can't be longer +/// than 70. +const int _boundaryLength = 70; + +final Random _random = new Random(); + +/// Returns a randomly-generated multipart boundary string +String boundaryString() { + var prefix = 'dart-http-boundary-'; + var list = new List.generate( + _boundaryLength - prefix.length, + (index) => + _boundaryCharacters[_random.nextInt(_boundaryCharacters.length)], + growable: false); + return '$prefix${new String.fromCharCodes(list)}'; +} diff --git a/lib/src/multipart_body.dart b/lib/src/multipart_body.dart index db1921f90b..f546e78b2b 100644 --- a/lib/src/multipart_body.dart +++ b/lib/src/multipart_body.dart @@ -1,4 +1,4 @@ -// Copyright (c) 2013, the Dart project authors. Please see the AUTHORS file +// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. @@ -20,9 +20,6 @@ class MultipartBody implements Body { /// This will be `null` after [read] is called. Stream> _stream; - /// The length of the stream returned by [read]. - /// - /// This is calculated from the fields and files passed into the body. final int contentLength; /// Multipart forms do not have an encoding. @@ -30,7 +27,7 @@ class MultipartBody implements Body { /// Creates a [MultipartBody] from the given [fields] and [files]. /// - /// The [boundary] is used to + /// The [boundary] is used to separate key value pairs within the body. factory MultipartBody(Map fields, Iterable files, String boundary) { var controller = new StreamController>(sync: true); @@ -45,10 +42,10 @@ class MultipartBody implements Body { } void writeLine() { - buffer.addAll([13, 10]); // \r\n + buffer..add(13)..add(10); // \r\n } - // Write the fields to the stream. + // Write the fields to the buffer. fields.forEach((name, value) { writeAscii('--$boundary\r\n'); writeUtf8(_headerForField(name, value)); @@ -58,16 +55,17 @@ class MultipartBody implements Body { controller.add(buffer); - // Iterate over the files to get the length and compute the headers ahead + // Iterate over the files to get the length and compute the headers ahead of // time so the length can be synchronously accessed. var fileList = files.toList(); var fileHeaders = >[]; var fileContentsLength = 0; for (var file in fileList) { - var header = []; - header.addAll('--$boundary\r\n'.codeUnits); - header.addAll(UTF8.encode(_headerForFile(file))); + var header = [] + ..addAll('--$boundary\r\n'.codeUnits) + ..addAll(UTF8.encode(_headerForFile(file))); + fileContentsLength += header.length + file.length + 2; fileHeaders.add(header); } @@ -77,7 +75,7 @@ class MultipartBody implements Body { fileContentsLength += ending.length; // Write the files to the stream asynchronously. - _writeFilesToStream(controller, files, fileHeaders, ending); + _writeFilesToStream(controller, fileList, fileHeaders, ending); return new MultipartBody._( controller.stream, buffer.length + fileContentsLength); @@ -91,7 +89,7 @@ class MultipartBody implements Body { Stream> read() { if (_stream == null) { throw new StateError("The 'read' method can only be called once on a " - "http.Request/http.Response object."); + 'http.Request/http.Response object.'); } var stream = _stream; _stream = null; @@ -106,12 +104,17 @@ class MultipartBody implements Body { List ending) async { for (var i = 0; i < files.length; ++i) { controller.add(fileHeaders[i]); - await writeStreamToSink(files[i].read(), controller); + try { + await writeStreamToSink(files[i].read(), controller); + } on Exception catch (exception, stackTrace) { + controller.addError(exception, stackTrace); + } controller.add([13, 10]); } - controller.add(ending); - controller.close(); + controller + ..add(ending) + ..close(); } /// Returns the header string for a field. @@ -139,15 +142,14 @@ class MultipartBody implements Body { return '$header\r\n\r\n'; } - static final _newlineRegExp = new RegExp(r"\r\n|\r|\n"); + static final _newlineRegExp = new RegExp(r'\r\n|\r|\n'); /// Encode [value] in the same way browsers do. - static String _browserEncode(String value) { - // http://tools.ietf.org/html/rfc2388 mandates some complex encodings for - // field names and file names, but in practice user agents seem not to - // follow this at all. Instead, they URL-encode `\r`, `\n`, and `\r\n` as - // `\r\n`; URL-encode `"`; and do nothing else (even for `%` or non-ASCII - // characters). We follow their behavior. - return value.replaceAll(_newlineRegExp, "%0D%0A").replaceAll('"', "%22"); - } + static String _browserEncode(String value) => + // http://tools.ietf.org/html/rfc2388 mandates some complex encodings for + // field names and file names, but in practice user agents seem not to + // follow this at all. Instead, they URL-encode `\r`, `\n`, and `\r\n` as + // `\r\n`; URL-encode `"`; and do nothing else (even for `%` or non-ASCII + // characters). We follow their behavior. + value.replaceAll(_newlineRegExp, '%0D%0A').replaceAll('"', '%22'); } diff --git a/lib/src/multipart_file.dart b/lib/src/multipart_file.dart index d3cc44398a..9aef80e1eb 100644 --- a/lib/src/multipart_file.dart +++ b/lib/src/multipart_file.dart @@ -13,8 +13,9 @@ import 'package:path/path.dart' as path; import 'content_type.dart'; -/// A file to be uploaded as part of a [MultipartRequest]. This doesn't need to -/// correspond to a physical file. +/// A file to be uploaded as part of a `multipart/form-data` Request. +/// +/// This doesn't need to correspond to a physical file. class MultipartFile { /// The stream that will emit the file's contents. Stream> _stream; diff --git a/lib/src/request.dart b/lib/src/request.dart index 9bcc6b6d5d..20c1c2727e 100644 --- a/lib/src/request.dart +++ b/lib/src/request.dart @@ -3,11 +3,11 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:convert'; -import 'dart:math'; +import 'boundary.dart'; import 'message.dart'; -import 'multipart_file.dart'; import 'multipart_body.dart'; +import 'multipart_file.dart'; import 'utils.dart'; /// Represents an HTTP request to be sent to a server. @@ -48,8 +48,7 @@ class Request extends Message { /// /// Extra [context] can be used to pass information between inner middleware /// and handlers. - Request.head(url, - {Map headers, Map context}) + Request.head(url, {Map headers, Map context}) : this('HEAD', url, headers: headers, context: context); /// Creates a new GET [Request] to [url], which can be a [Uri] or a [String]. @@ -59,8 +58,7 @@ class Request extends Message { /// /// Extra [context] can be used to pass information between inner middleware /// and handlers. - Request.get(url, - {Map headers, Map context}) + Request.get(url, {Map headers, Map context}) : this('GET', url, headers: headers, context: context); /// Creates a new POST [Request] to [url], which can be a [Uri] or a [String]. @@ -80,7 +78,7 @@ class Request extends Message { Map headers, Map context}) : this('POST', url, - body: body, encoding: encoding, headers: headers, context: context); + body: body, encoding: encoding, headers: headers, context: context); /// Creates a new PUT [Request] to [url], which can be a [Uri] or a [String]. /// @@ -99,7 +97,7 @@ class Request extends Message { Map headers, Map context}) : this('PUT', url, - body: body, encoding: encoding, headers: headers, context: context); + body: body, encoding: encoding, headers: headers, context: context); /// Creates a new PATCH [Request] to [url], which can be a [Uri] or a /// [String]. @@ -119,7 +117,7 @@ class Request extends Message { Map headers, Map context}) : this('PATCH', url, - body: body, encoding: encoding, headers: headers, context: context); + body: body, encoding: encoding, headers: headers, context: context); /// Creates a new DELETE [Request] to [url], which can be a [Uri] or a /// [String]. @@ -133,33 +131,43 @@ class Request extends Message { {Map headers, Map context}) : this('DELETE', url, headers: headers, context: context); + /// Creates a new `multipart/form-data` [Request] to [url], which can be a + /// [Uri] or a [String]. + /// + /// The content of the body is specified through the values of [fields] and + /// [files]. + /// + /// If [method] is not specified it defaults to POST. + /// + /// [headers] are the HTTP headers for the request. If [headers] is `null`, + /// it is treated as empty. + /// + /// Extra [context] can be used to pass information between inner middleware + /// and handlers. factory Request.multipart(url, - {Map headers, - Map context, - Map fields, - Iterable files}) { + {String method, + Map headers, + Map context, + Map fields, + Iterable files}) { fields ??= {}; files ??= []; headers ??= {}; - var boundary = _boundaryString(); + var boundary = boundaryString(); return new Request._( - 'POST', - url, + method ?? 'POST', + getUrl(url), new MultipartBody(fields, files, boundary), null, - updateMap(headers, { - 'content-type': 'multipart/form-data; boundary=$boundary' - }), + updateMap(headers, + {'content-type': 'multipart/form-data; boundary=$boundary'}), context); } - Request._(this.method, this.url, - body, - Encoding encoding, - Map headers, - Map context) + Request._(this.method, this.url, body, Encoding encoding, + Map headers, Map context) : super(body, encoding: encoding, headers: headers, context: context); /// Creates a new [Request] by copying existing values and applying specified @@ -174,53 +182,11 @@ class Request extends Message { /// [body] is the request body. It may be either a [String], a [List], a /// [Stream>], or `null` to indicate no body. Request change( - {Map headers, - Map context, - body}) { + {Map headers, Map context, body}) { var updatedHeaders = updateMap(this.headers, headers); var updatedContext = updateMap(this.context, context); - return new Request._( - this.method, - this.url, - body ?? getBody(this), - this.encoding, - updatedHeaders, - updatedContext); - } - - /// All character codes that are valid in multipart boundaries. - /// - /// This is the intersection of the characters allowed in the `bcharsnospace` - /// production defined in [RFC 2046][] and those allowed in the `token` - /// production defined in [RFC 1521][]. - /// - /// [RFC 2046]: http://tools.ietf.org/html/rfc2046#section-5.1.1. - /// [RFC 1521]: https://tools.ietf.org/html/rfc1521#section-4 - static const List _boundaryCharacters = const [ - 43, 95, 45, 46, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 65, 66, 67, 68, // - 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, // - 87, 88, 89, 90, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, // - 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, // - 122 - ]; - - /// The total length of the multipart boundaries used when building the - /// request body. - /// - /// According to http://tools.ietf.org/html/rfc1341.html, this can't be longer - /// than 70. - static const int _boundaryLength = 70; - - static final Random _random = new Random(); - - /// Returns a randomly-generated multipart boundary string - static String _boundaryString() { - var prefix = "dart-http-boundary-"; - var list = new List.generate(_boundaryLength - prefix.length, - (index) => - _boundaryCharacters[_random.nextInt(_boundaryCharacters.length)], - growable: false); - return "$prefix${new String.fromCharCodes(list)}"; + return new Request._(this.method, this.url, body ?? getBody(this), + this.encoding, updatedHeaders, updatedContext); } } diff --git a/pubspec.yaml b/pubspec.yaml index 503f083bf5..fad3da5f58 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -8,7 +8,7 @@ dependencies: collection: "^1.5.0" http_parser: ">=0.0.1 <4.0.0" path: ">=0.9.0 <2.0.0" - typed_data: "^1.1.5" + typed_data: "^1.0.0" dev_dependencies: test: "^0.12.18" # Override dependency on package_resolver to enable test diff --git a/test/multipart_test.dart b/test/multipart_test.dart index ba782396df..f6b9afff3e 100644 --- a/test/multipart_test.dart +++ b/test/multipart_test.dart @@ -1,4 +1,4 @@ -// Copyright (c) 2013, the Dart project authors. Please see the AUTHORS file +// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. @@ -13,7 +13,7 @@ import 'utils.dart'; void main() { test('empty', () { var request = new http.Request.multipart(dummyUrl); - expect(request, bodyMatches(''' + expect(request, multipartBodyMatches(''' --{{boundary}}-- ''')); }); @@ -24,15 +24,15 @@ void main() { 'field2': 'value2', }; var files = [ - new http.MultipartFile.fromString("file1", "contents1", - filename: "filename1.txt"), - new http.MultipartFile.fromString("file2", "contents2"), + new http.MultipartFile.fromString('file1', 'contents1', + filename: 'filename1.txt'), + new http.MultipartFile.fromString('file2', 'contents2'), ]; var request = new http.Request.multipart(dummyUrl, fields: fields, files: files); - expect(request, bodyMatches(''' + expect(request, multipartBodyMatches(''' --{{boundary}} content-disposition: form-data; name="field1" @@ -60,7 +60,7 @@ void main() { var request = new http.Request.multipart(dummyUrl, fields: fields); - expect(request, bodyMatches(''' + expect(request, multipartBodyMatches(''' --{{boundary}} content-disposition: form-data; name="fïēld" @@ -73,7 +73,7 @@ void main() { var fields = {'foo\nbar\rbaz\r\nbang': 'value'}; var request = new http.Request.multipart(dummyUrl, fields: fields); - expect(request, bodyMatches(''' + expect(request, multipartBodyMatches(''' --{{boundary}} content-disposition: form-data; name="foo%0D%0Abar%0D%0Abaz%0D%0Abang" @@ -86,7 +86,7 @@ void main() { var fields = {'foo"bar': 'value'}; var request = new http.Request.multipart(dummyUrl, fields: fields); - expect(request, bodyMatches(''' + expect(request, multipartBodyMatches(''' --{{boundary}} content-disposition: form-data; name="foo%22bar" @@ -99,7 +99,7 @@ void main() { var fields = {'field': 'vⱥlūe'}; var request = new http.Request.multipart(dummyUrl, fields: fields); - expect(request, bodyMatches(''' + expect(request, multipartBodyMatches(''' --{{boundary}} content-disposition: form-data; name="field" content-type: text/plain; charset=utf-8 @@ -117,7 +117,7 @@ void main() { ]; var request = new http.Request.multipart(dummyUrl, files: files); - expect(request, bodyMatches(''' + expect(request, multipartBodyMatches(''' --{{boundary}} content-type: text/plain; charset=utf-8 content-disposition: form-data; name="file"; filename="fïlēname.txt" @@ -134,7 +134,7 @@ void main() { ]; var request = new http.Request.multipart(dummyUrl, files: files); - expect(request, bodyMatches(''' + expect(request, multipartBodyMatches(''' --{{boundary}} content-type: text/plain; charset=utf-8 content-disposition: form-data; name="file"; filename="foo%0D%0Abar%0D%0Abaz%0D%0Abang" @@ -150,7 +150,7 @@ void main() { ]; var request = new http.Request.multipart(dummyUrl, files: files); - expect(request, bodyMatches(''' + expect(request, multipartBodyMatches(''' --{{boundary}} content-type: text/plain; charset=utf-8 content-disposition: form-data; name="file"; filename="foo%22bar" @@ -167,7 +167,7 @@ void main() { ]; var request = new http.Request.multipart(dummyUrl, files: files); - expect(request, bodyMatches(''' + expect(request, multipartBodyMatches(''' --{{boundary}} content-type: application/json; charset=utf-8 content-disposition: form-data; name="file" @@ -186,7 +186,7 @@ void main() { ]; var request = new http.Request.multipart(dummyUrl, files: files); - expect(request, bodyMatches(''' + expect(request, multipartBodyMatches(''' --{{boundary}} content-type: text/plain; charset=iso-8859-1 content-disposition: form-data; name="file" @@ -197,11 +197,11 @@ void main() { }); test('with a stream file', () { - var controller = new StreamController(sync: true); + var controller = new StreamController>(sync: true); var files = [new http.MultipartFile('file', controller.stream, 5)]; var request = new http.Request.multipart(dummyUrl, files: files); - expect(request, bodyMatches(''' + expect(request, multipartBodyMatches(''' --{{boundary}} content-type: application/octet-stream content-disposition: form-data; name="file" @@ -210,16 +210,17 @@ void main() { --{{boundary}}-- ''')); - controller.add([104, 101, 108, 108, 111]); - controller.close(); + controller + ..add([104, 101, 108, 108, 111]) + ..close(); }); test('with an empty stream file', () { - var controller = new StreamController(sync: true); + var controller = new StreamController>(sync: true); var files = [new http.MultipartFile('file', controller.stream, 0)]; var request = new http.Request.multipart(dummyUrl, files: files); - expect(request, bodyMatches(''' + expect(request, multipartBodyMatches(''' --{{boundary}} content-type: application/octet-stream content-disposition: form-data; name="file" @@ -237,7 +238,7 @@ void main() { ]; var request = new http.Request.multipart(dummyUrl, files: files); - expect(request, bodyMatches(''' + expect(request, multipartBodyMatches(''' --{{boundary}} content-type: application/octet-stream content-disposition: form-data; name="file" diff --git a/test/utils.dart b/test/utils.dart index a8858ad29c..72e77791ec 100644 --- a/test/utils.dart +++ b/test/utils.dart @@ -62,21 +62,24 @@ class _Parse extends Matcher { } Description describe(Description description) { - return description.add('parses to a value that ') - .addDescriptionOf(_matcher); + return description + .add('parses to a value that ') + .addDescriptionOf(_matcher); } } /// A matcher that validates the body of a multipart request after finalization. +/// /// The string "{{boundary}}" in [pattern] will be replaced by the boundary /// string for the request, and LF newlines will be replaced with CRLF. /// Indentation will be normalized. -Matcher bodyMatches(String pattern) => new _BodyMatches(pattern); +Matcher multipartBodyMatches(String pattern) => + new _MultipartBodyMatches(pattern); -class _BodyMatches extends Matcher { +class _MultipartBodyMatches extends Matcher { final String _pattern; - _BodyMatches(this._pattern); + _MultipartBodyMatches(this._pattern); bool matches(item, Map matchState) { if (item is! http.Request) return false; @@ -86,8 +89,8 @@ class _BodyMatches extends Matcher { var contentType = new MediaType.parse(item.headers['content-type']); var boundary = contentType.parameters['boundary']; var expected = cleanUpLiteral(_pattern) - .replaceAll("\n", "\r\n") - .replaceAll("{{boundary}}", boundary); + .replaceAll('\n', '\r\n') + .replaceAll('{{boundary}}', boundary); expect(body, equals(expected)); expect(item.contentLength, equals(bodyBytes.length)); @@ -96,21 +99,20 @@ class _BodyMatches extends Matcher { return completes.matches(future, matchState); } - Description describe(Description description) { - return description.add('has a body that matches "$_pattern"'); - } + Description describe(Description description) => + description.add('has a body that matches "$_pattern"'); } /// A matcher that matches a [http.ClientException] with the given [message]. /// /// [message] can be a String or a [Matcher]. Matcher isClientException([message]) => predicate((error) { - expect(error, new isInstanceOf()); - if (message != null) { - expect(error.message, message); - } - return true; -}); + expect(error, new isInstanceOf()); + if (message != null) { + expect(error.message, message); + } + return true; + }); /// A matcher that matches function or future that throws a /// [http.ClientException] with the given [message]. From 28208ae90e6fe99e431cc3cfe92f394b1a6a988f Mon Sep 17 00:00:00 2001 From: Don Date: Wed, 25 Oct 2017 12:01:53 -0700 Subject: [PATCH 15/20] nits --- lib/src/request.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/request.dart b/lib/src/request.dart index 20c1c2727e..5e45428d6e 100644 --- a/lib/src/request.dart +++ b/lib/src/request.dart @@ -150,9 +150,9 @@ class Request extends Message { Map context, Map fields, Iterable files}) { - fields ??= {}; - files ??= []; - headers ??= {}; + fields ??= const {}; + files ??= const []; + headers ??= {}; var boundary = boundaryString(); From 83bfc8d36b1540e93575b0b75cde0c93c0e89550 Mon Sep 17 00:00:00 2001 From: Don Date: Wed, 25 Oct 2017 16:14:59 -0700 Subject: [PATCH 16/20] Update with modified constructors --- lib/src/multipart_file.dart | 106 ++++++++++++++++++------------------ pubspec.yaml | 2 +- test/multipart_test.dart | 26 +++++---- 3 files changed, 68 insertions(+), 66 deletions(-) diff --git a/lib/src/multipart_file.dart b/lib/src/multipart_file.dart index 9aef80e1eb..c53ad7c265 100644 --- a/lib/src/multipart_file.dart +++ b/lib/src/multipart_file.dart @@ -4,12 +4,10 @@ import 'dart:async'; import 'dart:convert'; -import 'dart:io'; import 'package:async/async.dart'; -import 'package:collection/collection.dart'; import 'package:http_parser/http_parser.dart'; -import 'package:path/path.dart' as path; +import 'package:mime/mime.dart'; import 'content_type.dart'; @@ -37,62 +35,52 @@ class MultipartFile { /// Defaults to `application/octet-stream`. final MediaType contentType; - /// Creates a new [MultipartFile] from a chunked [Stream] of bytes. /// - /// The length of the file in bytes must be known in advance. If it's not, - /// read the data from the stream and use [MultipartFile.fromBytes] instead. - /// - /// [contentType] currently defaults to `application/octet-stream`, but in the - /// future may be inferred from [filename]. - MultipartFile(this.field, Stream> stream, this.length, - {this.filename, MediaType contentType}) - : this._stream = stream, - this.contentType = - contentType ?? new MediaType("application", "octet-stream"); - - /// Creates a new [MultipartFile] from a byte array. - /// - /// [contentType] currently defaults to `application/octet-stream`, but in the - /// future may be inferred from [filename]. - factory MultipartFile.fromBytes(String field, List value, - {String filename, MediaType contentType}) { - var stream = new Stream.fromIterable([DelegatingList.typed(value)]); - return new MultipartFile(field, stream, value.length, - filename: filename, contentType: contentType); - } + factory MultipartFile(String field, value, + {String filename, MediaType contentType, Encoding encoding}) { + List bytes; + var defaultMediaType; - /// Creates a new [MultipartFile] from a string. - /// - /// The encoding to use when translating [value] into bytes is taken from - /// [contentType] if it has a charset set. Otherwise, it defaults to UTF-8. - /// [contentType] currently defaults to `text/plain; charset=utf-8`, but in - /// the future may be inferred from [filename]. - factory MultipartFile.fromString(String field, String value, - {String filename, MediaType contentType}) { - contentType ??= new MediaType("text", "plain"); - var encoding = encodingForMediaType(contentType) ?? UTF8; - contentType = contentType.change(parameters: {'charset': encoding.name}); - - return new MultipartFile.fromBytes(field, encoding.encode(value), + if (value is String) { + encoding ??= encodingForMediaType(contentType) ?? UTF8; + bytes = encoding.encode(value); + defaultMediaType = new MediaType('text', 'plain'); + } else if (value is List) { + bytes = value; + defaultMediaType = new MediaType('application', 'octet-stream'); + } else { + throw new ArgumentError.value( + value, + 'value', + 'value must be either a ' + 'String or a List'); + } + + contentType ??= _lookupMediaType(filename, bytes) ?? defaultMediaType; + + if (encoding != null) { + contentType = contentType.change(parameters: {'charset': encoding.name}); + } + + return new MultipartFile.fromStream( + field, new Stream>.fromIterable([bytes]), bytes.length, filename: filename, contentType: contentType); } - // TODO(nweiz): Infer the content-type from the filename. - /// Creates a new [MultipartFile] from a path to a file on disk. - /// - /// [filename] defaults to the basename of [filePath]. [contentType] currently - /// defaults to `application/octet-stream`, but in the future may be inferred - /// from [filename]. - /// - /// Throws an [UnsupportedError] if `dart:io` isn't supported in this - /// environment. - static Future fromPath(String field, String filePath, + MultipartFile.fromStream(this.field, Stream> stream, this.length, + {String filename, MediaType contentType}) + : _stream = stream, + filename = filename, + contentType = contentType ?? + _lookupMediaType(filename) ?? + new MediaType('application', 'octet-stream'); + + static Future loadStream( + String field, Stream> stream, {String filename, MediaType contentType}) async { - if (filename == null) filename = path.basename(filePath); - var file = new File(filePath); - var length = await file.length(); - var stream = DelegatingStream.typed(file.openRead()); - return new MultipartFile(field, stream, length, + var bytes = await collectBytes(stream); + + return new MultipartFile(field, bytes, filename: filename, contentType: contentType); } @@ -108,4 +96,16 @@ class MultipartFile { _stream = null; return stream; } + + /// Looks up the [MediaType] from the [filename]'s extension or from + /// magic numbers contained within a file header's [bytes]. + static MediaType _lookupMediaType(String filename, [List bytes]) { + if ((filename == null) && (bytes == null)) return null; + + // lookupMimeType expects filename to be non-null but its possible that + // this can be called with bytes but no filename. + var mimeType = lookupMimeType(filename ?? '', headerBytes: bytes); + + return mimeType != null ? new MediaType.parse(mimeType) : null; + } } diff --git a/pubspec.yaml b/pubspec.yaml index fad3da5f58..1c3ebefa2f 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -7,7 +7,7 @@ dependencies: async: "^1.13.0" collection: "^1.5.0" http_parser: ">=0.0.1 <4.0.0" - path: ">=0.9.0 <2.0.0" + mime: ">=0.9.0 < 1.0.0" typed_data: "^1.0.0" dev_dependencies: test: "^0.12.18" diff --git a/test/multipart_test.dart b/test/multipart_test.dart index f6b9afff3e..d3302d0930 100644 --- a/test/multipart_test.dart +++ b/test/multipart_test.dart @@ -24,9 +24,8 @@ void main() { 'field2': 'value2', }; var files = [ - new http.MultipartFile.fromString('file1', 'contents1', - filename: 'filename1.txt'), - new http.MultipartFile.fromString('file2', 'contents2'), + new http.MultipartFile('file1', 'contents1', filename: 'filename1.txt'), + new http.MultipartFile('file2', 'contents2'), ]; var request = @@ -112,8 +111,7 @@ void main() { test('with a unicode filename', () { var files = [ - new http.MultipartFile.fromString('file', 'contents', - filename: 'fïlēname.txt') + new http.MultipartFile('file', 'contents', filename: 'fïlēname.txt') ]; var request = new http.Request.multipart(dummyUrl, files: files); @@ -129,7 +127,7 @@ void main() { test('with a filename with newlines', () { var files = [ - new http.MultipartFile.fromString('file', 'contents', + new http.MultipartFile('file', 'contents', filename: 'foo\nbar\rbaz\r\nbang') ]; var request = new http.Request.multipart(dummyUrl, files: files); @@ -146,7 +144,7 @@ void main() { test('with a filename with a quote', () { var files = [ - new http.MultipartFile.fromString('file', 'contents', filename: 'foo"bar') + new http.MultipartFile('file', 'contents', filename: 'foo"bar') ]; var request = new http.Request.multipart(dummyUrl, files: files); @@ -162,7 +160,7 @@ void main() { test('with a string file with a content-type but no charset', () { var files = [ - new http.MultipartFile.fromString('file', '{"hello": "world"}', + new http.MultipartFile('file', '{"hello": "world"}', contentType: new MediaType('application', 'json')) ]; var request = new http.Request.multipart(dummyUrl, files: files); @@ -180,7 +178,7 @@ void main() { test('with a file with a iso-8859-1 body', () { // "Ã¥" encoded as ISO-8859-1 and then read as UTF-8 results in "å". var files = [ - new http.MultipartFile.fromString('file', 'non-ascii: "Ã¥"', + new http.MultipartFile('file', 'non-ascii: "Ã¥"', contentType: new MediaType('text', 'plain', {'charset': 'iso-8859-1'})) ]; @@ -198,7 +196,9 @@ void main() { test('with a stream file', () { var controller = new StreamController>(sync: true); - var files = [new http.MultipartFile('file', controller.stream, 5)]; + var files = [ + new http.MultipartFile.fromStream('file', controller.stream, 5) + ]; var request = new http.Request.multipart(dummyUrl, files: files); expect(request, multipartBodyMatches(''' @@ -217,7 +217,9 @@ void main() { test('with an empty stream file', () { var controller = new StreamController>(sync: true); - var files = [new http.MultipartFile('file', controller.stream, 0)]; + var files = [ + new http.MultipartFile.fromStream('file', controller.stream, 0) + ]; var request = new http.Request.multipart(dummyUrl, files: files); expect(request, multipartBodyMatches(''' @@ -234,7 +236,7 @@ void main() { test('with a byte file', () { var files = [ - new http.MultipartFile.fromBytes('file', [104, 101, 108, 108, 111]) + new http.MultipartFile('file', [104, 101, 108, 108, 111]) ]; var request = new http.Request.multipart(dummyUrl, files: files); From 04d482f01ef64b9fa2b646123ca0db5ae4e6dff6 Mon Sep 17 00:00:00 2001 From: Don Date: Wed, 25 Oct 2017 16:59:12 -0700 Subject: [PATCH 17/20] Formating --- lib/src/multipart_file.dart | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/src/multipart_file.dart b/lib/src/multipart_file.dart index c53ad7c265..e6a9f97efb 100644 --- a/lib/src/multipart_file.dart +++ b/lib/src/multipart_file.dart @@ -50,10 +50,7 @@ class MultipartFile { defaultMediaType = new MediaType('application', 'octet-stream'); } else { throw new ArgumentError.value( - value, - 'value', - 'value must be either a ' - 'String or a List'); + value, 'value', 'value must be either a String or a List'); } contentType ??= _lookupMediaType(filename, bytes) ?? defaultMediaType; @@ -89,8 +86,8 @@ class MultipartFile { /// Can only be called once. Stream> read() { if (_stream == null) { - throw new StateError("The 'read' method can only be called once on a " - "http.MultipartFile object."); + throw new StateError('The "read" method can only be called once on a ' + 'http.MultipartFile object.'); } var stream = _stream; _stream = null; From 7246b9ac1adcba6d38077877d9dad2d4d3f83320 Mon Sep 17 00:00:00 2001 From: Don Date: Thu, 26 Oct 2017 12:32:34 -0700 Subject: [PATCH 18/20] Add readAsBytes to Message --- lib/src/message.dart | 13 +++++++++++-- test/utils.dart | 2 +- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/src/message.dart b/lib/src/message.dart index ae16e24963..6111027948 100644 --- a/lib/src/message.dart +++ b/lib/src/message.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:convert'; +import 'package:async/async.dart'; import 'package:collection/collection.dart'; import 'package:http_parser/http_parser.dart'; @@ -121,16 +122,24 @@ abstract class Message { /// Returns the message body as byte chunks. /// - /// Throws a [StateError] if [read] or [readAsString] has already been called. + /// Throws a [StateError] if [read] or [readAsBytes] or [readAsString] has + /// already been called. Stream> read() => _body.read(); + /// Returns the message body as a list of bytes. + /// + /// Throws a [StateError] if [read] or [readAsBytes] or [readAsString] has + /// already been called. + Future> readAsBytes() => collectBytes(read()); + /// Returns the message body as a string. /// /// If [encoding] is passed, that's used to decode the body. Otherwise the /// encoding is taken from the Content-Type header. If that doesn't exist or /// doesn't have a "charset" parameter, UTF-8 is used. /// - /// Throws a [StateError] if [read] or [readAsString] has already been called. + /// Throws a [StateError] if [read] or [readAsBytes] or [readAsString] has + /// already been called. Future readAsString([Encoding encoding]) { encoding ??= this.encoding ?? UTF8; return encoding.decodeStream(read()); diff --git a/test/utils.dart b/test/utils.dart index 72e77791ec..bf8da3a2d4 100644 --- a/test/utils.dart +++ b/test/utils.dart @@ -84,7 +84,7 @@ class _MultipartBodyMatches extends Matcher { bool matches(item, Map matchState) { if (item is! http.Request) return false; - var future = collectBytes(item.read()).then((bodyBytes) { + var future = item.readAsBytes().then((bodyBytes) { var body = UTF8.decode(bodyBytes); var contentType = new MediaType.parse(item.headers['content-type']); var boundary = contentType.parameters['boundary']; From 05350c1284ac7c3ec1d084d66ebb819fa4389123 Mon Sep 17 00:00:00 2001 From: Don Date: Thu, 26 Oct 2017 12:32:50 -0700 Subject: [PATCH 19/20] Fix nits --- lib/src/multipart_file.dart | 39 +++++++++++++++++++++++++++++++------ pubspec.yaml | 2 +- test/multipart_test.dart | 4 +++- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/lib/src/multipart_file.dart b/lib/src/multipart_file.dart index e6a9f97efb..de75371c01 100644 --- a/lib/src/multipart_file.dart +++ b/lib/src/multipart_file.dart @@ -35,14 +35,25 @@ class MultipartFile { /// Defaults to `application/octet-stream`. final MediaType contentType; + /// Creates a [MultipartFile] from the [value]. /// + /// [value] can be either a [String] or a [List]. + /// + /// For a String [value] the content will be encoded using [encoding] which + /// defaults to [UTF8]. The `charset` from [contentType] is ignored when + /// encoding the String. + /// + /// [contentType] if not specified will attempt to be looked up from the + /// bytes contained within the [stream] and the [filename] if provided. It + /// will default to `plain/text` for [String]s and `application/octet-stream` + /// for [List]. factory MultipartFile(String field, value, {String filename, MediaType contentType, Encoding encoding}) { List bytes; var defaultMediaType; if (value is String) { - encoding ??= encodingForMediaType(contentType) ?? UTF8; + encoding ??= UTF8; bytes = encoding.encode(value); defaultMediaType = new MediaType('text', 'plain'); } else if (value is List) { @@ -53,25 +64,40 @@ class MultipartFile { value, 'value', 'value must be either a String or a List'); } - contentType ??= _lookupMediaType(filename, bytes) ?? defaultMediaType; + contentType ??= _lookUpMediaType(filename, bytes) ?? defaultMediaType; if (encoding != null) { contentType = contentType.change(parameters: {'charset': encoding.name}); } return new MultipartFile.fromStream( - field, new Stream>.fromIterable([bytes]), bytes.length, + field, new Stream.fromIterable([bytes]), bytes.length, filename: filename, contentType: contentType); } + /// Creates a new [MultipartFile] from a chunked [stream] of bytes. + /// + /// The [length] of the file in bytes must be known in advance. If it's not + /// then use [loadStream] to create the [MultipartFile] instance. + /// + /// [contentType] if not specified will attempt to be looked up from the + /// [filename] if provided. It will default to `application/octet-stream`. MultipartFile.fromStream(this.field, Stream> stream, this.length, {String filename, MediaType contentType}) : _stream = stream, filename = filename, contentType = contentType ?? - _lookupMediaType(filename) ?? + _lookUpMediaType(filename) ?? new MediaType('application', 'octet-stream'); + /// Creates a new [MultipartFile] from the [stream]. + /// + /// This method should be used when the length of [stream] in bytes is not + /// known ahead of time. + /// + /// [contentType] if not specified will attempt to be looked up from the + /// bytes contained within the [stream] and the [filename] if provided. It + /// will default to `application/octet-stream`. static Future loadStream( String field, Stream> stream, {String filename, MediaType contentType}) async { @@ -96,11 +122,12 @@ class MultipartFile { /// Looks up the [MediaType] from the [filename]'s extension or from /// magic numbers contained within a file header's [bytes]. - static MediaType _lookupMediaType(String filename, [List bytes]) { - if ((filename == null) && (bytes == null)) return null; + static MediaType _lookUpMediaType(String filename, [List bytes]) { + if (filename == null && bytes == null) return null; // lookupMimeType expects filename to be non-null but its possible that // this can be called with bytes but no filename. + // FIXME: https://github.com/dart-lang/mime/issues/11 var mimeType = lookupMimeType(filename ?? '', headerBytes: bytes); return mimeType != null ? new MediaType.parse(mimeType) : null; diff --git a/pubspec.yaml b/pubspec.yaml index 1c3ebefa2f..fbe50ba65e 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -7,7 +7,7 @@ dependencies: async: "^1.13.0" collection: "^1.5.0" http_parser: ">=0.0.1 <4.0.0" - mime: ">=0.9.0 < 1.0.0" + mime: "^0.9.0" typed_data: "^1.0.0" dev_dependencies: test: "^0.12.18" diff --git a/test/multipart_test.dart b/test/multipart_test.dart index d3302d0930..7e539f5da0 100644 --- a/test/multipart_test.dart +++ b/test/multipart_test.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:async'; +import 'dart:convert'; import 'package:http/http.dart' as http; import 'package:http_parser/http_parser.dart'; @@ -179,8 +180,9 @@ void main() { // "Ã¥" encoded as ISO-8859-1 and then read as UTF-8 results in "å". var files = [ new http.MultipartFile('file', 'non-ascii: "Ã¥"', + encoding: LATIN1, contentType: - new MediaType('text', 'plain', {'charset': 'iso-8859-1'})) + new MediaType('text', 'plain')) ]; var request = new http.Request.multipart(dummyUrl, files: files); From d6be80407a255b033b7906fc7a4fd8f0a5b2c665 Mon Sep 17 00:00:00 2001 From: Don Date: Thu, 26 Oct 2017 12:38:02 -0700 Subject: [PATCH 20/20] More nits --- lib/src/multipart_body.dart | 12 ++++++++---- lib/src/request.dart | 5 +++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/src/multipart_body.dart b/lib/src/multipart_body.dart index f546e78b2b..961db27973 100644 --- a/lib/src/multipart_body.dart +++ b/lib/src/multipart_body.dart @@ -11,9 +11,10 @@ import 'body.dart'; import 'multipart_file.dart'; import 'utils.dart'; -/// A `multipart/form-data` request [Body]. Such a request has both string -/// fields, which function as normal form fields, and (potentially streamed) -/// binary files. +/// A `multipart/form-data` request [Body]. +/// +/// Such a request has both string fields, which function as normal form +/// fields, and (potentially streamed) binary files. class MultipartBody implements Body { /// The contents of the message body. /// @@ -104,11 +105,14 @@ class MultipartBody implements Body { List ending) async { for (var i = 0; i < files.length; ++i) { controller.add(fileHeaders[i]); + + // file.read() can throw synchronously try { await writeStreamToSink(files[i].read(), controller); - } on Exception catch (exception, stackTrace) { + } catch (exception, stackTrace) { controller.addError(exception, stackTrace); } + controller.add([13, 10]); } diff --git a/lib/src/request.dart b/lib/src/request.dart index 5e45428d6e..c50f018648 100644 --- a/lib/src/request.dart +++ b/lib/src/request.dart @@ -131,8 +131,9 @@ class Request extends Message { {Map headers, Map context}) : this('DELETE', url, headers: headers, context: context); - /// Creates a new `multipart/form-data` [Request] to [url], which can be a - /// [Uri] or a [String]. + /// Creates a new + /// [`multipart/form-data`](https://en.wikipedia.org/wiki/MIME#Multipart_messages) + /// [Request] to [url], which can be a [Uri] or a [String]. /// /// The content of the body is specified through the values of [fields] and /// [files].