diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e541a5b..3964e158 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,16 @@ +## 0.6.6 + +* Allow `List`s to be passed as request or response bodies. + +* Requests and responses now automatically set `Content-Length` headers when the + body length is known ahead of time. + +* Work around [sdk#27660][] by manually setting + `HttpResponse.chunkedTransferEncoding` to `false` for requests known to have + no content. + +[sdk#27660]: https://github.com/dart-lang/sdk/issues/27660 + ## 0.6.5+3 * Improve the documentation of `logRequests()`. diff --git a/lib/shelf_io.dart b/lib/shelf_io.dart index 49910894..7469d3f6 100644 --- a/lib/shelf_io.dart +++ b/lib/shelf_io.dart @@ -149,6 +149,11 @@ Future _writeResponse(Response response, HttpResponse httpResponse) { httpResponse.headers.date = new DateTime.now().toUtc(); } + // Work around sdk#27660. + if (response.contentLength == 0) { + httpResponse.headers.chunkedTransferEncoding = false; + } + return httpResponse .addStream(response.read()) .then((_) => httpResponse.close()); diff --git a/lib/src/body.dart b/lib/src/body.dart index 6fd0df25..0fee5bd4 100644 --- a/lib/src/body.dart +++ b/lib/src/body.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:convert'; +import 'package:collection/collection.dart'; import 'package:async/async.dart'; /// The body of a request or response. @@ -18,23 +19,45 @@ class Body { /// This will be `null` after [read] is called. Stream> _stream; - Body._(this._stream); + /// The encoding used to encode the stream returned by [read], or `null` if no + /// encoding was used. + final Encoding encoding; + + /// The length of the stream returned by [read], or `null` if that can't be + /// determined efficiently. + final int contentLength; + + Body._(this._stream, this.encoding, this.contentLength); /// Converts [body] to a byte stream and wraps it in a [Body]. /// - /// [body] may be either a [Body], a [String], a [Stream>], or - /// `null`. If it's a [String], [encoding] will be used to convert it to a - /// [Stream>]. + /// [body] may be either a [Body], a [String], a [List], a + /// [Stream>], or `null`. If it's a [String], [encoding] will be + /// used to convert it to a [Stream>]. factory Body(body, [Encoding encoding]) { - if (encoding == null) encoding = UTF8; - if (body is Body) return body; Stream> stream; + int contentLength; if (body == null) { + contentLength = 0; stream = new Stream.fromIterable([]); } else if (body is String) { - stream = new Stream.fromIterable([encoding.encode(body)]); + if (encoding == null) { + var encoded = UTF8.encode(body); + // If the text is plain ASCII, don't modify the encoding. This means + // that an encoding of "text/plain" will stay put. + if (!_isPlainAscii(encoded, body.length)) encoding = UTF8; + contentLength = encoded.length; + stream = new Stream.fromIterable([encoded]); + } else { + var encoded = encoding.encode(body); + contentLength = encoded.length; + stream = new Stream.fromIterable([encoded]); + } + } else if (body is List) { + contentLength = body.length; + stream = new Stream.fromIterable([DelegatingList.typed(body)]); } else if (body is Stream) { stream = DelegatingStream.typed(body); } else { @@ -42,7 +65,24 @@ class Body { 'Stream.'); } - return new Body._(stream); + return new Body._(stream, encoding, contentLength); + } + + /// Returns whether [bytes] is plain ASCII. + /// + /// [codeUnits] is the number of code units in the original string. + static bool _isPlainAscii(List bytes, int codeUnits) { + // Most non-ASCII code units will produce multiple bytes and make the text + // longer. + if (bytes.length != codeUnits) return false; + + for (var byte in bytes) { + // Non-ASCII code units between U+0080 and U+009F produce 8-bit + // characters with the high bit set. + if (byte & 0x80 != 0) return false; + } + + return true; } /// Returns a [Stream] representing the body. diff --git a/lib/src/message.dart b/lib/src/message.dart index 16965e1a..2059b9a7 100644 --- a/lib/src/message.dart +++ b/lib/src/message.dart @@ -13,6 +13,11 @@ import 'util.dart'; Body getBody(Message message) => message._body; +/// The default set of headers for a message created with no body and no +/// explicit headers. +final _defaultHeaders = new ShelfUnmodifiableMap( + {"content-length": "0"}, ignoreKeyCase: true); + /// Represents logic shared between [Request] and [Response]. abstract class Message { /// The HTTP headers. @@ -40,7 +45,7 @@ abstract class Message { /// Creates a new [Message]. /// - /// [body] is the response body. It may be either a [String], a + /// [body] is the response body. It may be either a [String], a [List], a /// [Stream>], or `null` to indicate no body. If it's a [String], /// [encoding] is used to encode it to a [Stream>]. It defaults to /// UTF-8. @@ -52,10 +57,13 @@ abstract class Message { /// Content-Type header, it will be set to "application/octet-stream". Message(body, {Encoding encoding, Map headers, Map context}) - : this._body = new Body(body, encoding), - this.headers = new ShelfUnmodifiableMap( - _adjustHeaders(headers, encoding), ignoreKeyCase: true), - this.context = new ShelfUnmodifiableMap(context, + : this._(new Body(body, encoding), headers, context); + + Message._(Body body, Map headers, Map context) + : _body = body, + headers = new ShelfUnmodifiableMap( + _adjustHeaders(headers, body), ignoreKeyCase: true), + context = new ShelfUnmodifiableMap(context, ignoreKeyCase: false); /// The contents of the content-length field in [headers]. @@ -63,6 +71,7 @@ abstract class Message { /// If not set, `null`. int get contentLength { if (_contentLengthCache != null) return _contentLengthCache; + if (_body.contentLength != null) return _body.contentLength; if (!headers.containsKey('content-length')) return null; _contentLengthCache = int.parse(headers['content-length']); return _contentLengthCache; @@ -134,17 +143,48 @@ abstract class Message { /// /// Returns a new map without modifying [headers]. Map _adjustHeaders( - Map headers, Encoding encoding) { - if (headers == null) headers = const {}; - if (encoding == null) return headers; - - headers = new CaseInsensitiveMap.from(headers); - if (headers['content-type'] == null) { - return addHeader(headers, 'content-type', - 'application/octet-stream; charset=${encoding.name}'); + Map headers, Body body) { + var sameEncoding = _sameEncoding(headers, body); + if (sameEncoding) { + if (body.contentLength == null || + getHeader(headers, 'content-length') == + body.contentLength.toString()) { + return headers ?? const ShelfUnmodifiableMap.empty(); + } else if (body.contentLength == 0 && + (headers == null || headers.isEmpty)) { + return _defaultHeaders; + } + } + + var newHeaders = headers == null + ? new CaseInsensitiveMap() + : new CaseInsensitiveMap.from(headers); + + if (!sameEncoding) { + if (newHeaders['content-type'] == null) { + newHeaders['content-type'] = + 'application/octet-stream; charset=${body.encoding.name}'; + } else { + var contentType = new MediaType.parse(newHeaders['content-type']) + .change(parameters: {'charset': body.encoding.name}); + newHeaders['content-type'] = contentType.toString(); + } + } + + if (body.contentLength != null) { + newHeaders['content-length'] = body.contentLength.toString(); } - var contentType = new MediaType.parse(headers['content-type']).change( - parameters: {'charset': encoding.name}); - return addHeader(headers, 'content-type', contentType.toString()); + return newHeaders; +} + +/// Returns whether [headers] declares the same encoding as [body]. +bool _sameEncoding(Map headers, Body body) { + if (body.encoding == null) return true; + + var contentType = getHeader(headers, 'content-type'); + if (contentType == null) return false; + + var charset = new MediaType.parse(contentType).parameters['charset']; + return Encoding.getByName(charset) == body.encoding; } diff --git a/lib/src/request.dart b/lib/src/request.dart index 1b184185..0d0154dd 100644 --- a/lib/src/request.dart +++ b/lib/src/request.dart @@ -96,10 +96,10 @@ class Request extends Message { /// and [url] to `requestedUri.path` without the initial `/`. If only one is /// passed, the other will be inferred. /// - /// [body] is the request body. It may be either a [String], a - /// [Stream>], or `null` to indicate no body. - /// If it's a [String], [encoding] is used to encode it to a - /// [Stream>]. The default encoding is UTF-8. + /// [body] is the request body. It may be either a [String], a [List], a + /// [Stream>], or `null` to indicate no body. If it's a [String], + /// [encoding] is used to encode it to a [Stream>]. The default + /// encoding is UTF-8. /// /// If [encoding] is passed, the "encoding" field of the Content-Type header /// in [headers] will be set appropriately. If there is no existing @@ -192,8 +192,8 @@ class Request extends Message { /// [Request]. All other context and header values from the [Request] will be /// included in the copied [Request] unchanged. /// - /// [body] is the request body. It may be either a [String] or a - /// [Stream>]. + /// [body] is the request body. It may be either a [String], a [List], a + /// [Stream>], or `null` to indicate no body. /// /// [path] is used to update both [handlerPath] and [url]. It's designed for /// routing middleware, and represents the path from the current handler to diff --git a/lib/src/response.dart b/lib/src/response.dart index 79455268..214b005b 100644 --- a/lib/src/response.dart +++ b/lib/src/response.dart @@ -43,7 +43,7 @@ class Response extends Message { /// /// This indicates that the request has succeeded. /// - /// [body] is the response body. It may be either a [String], a + /// [body] is the response body. It may be either a [String], a [List], a /// [Stream>], or `null` to indicate no body. If it's a [String], /// [encoding] is used to encode it to a [Stream>]. It defaults to /// UTF-8. @@ -62,7 +62,7 @@ class Response extends Message { /// URI. [location] is that URI; it can be either a [String] or a [Uri]. It's /// automatically set as the Location header in [headers]. /// - /// [body] is the response body. It may be either a [String], a + /// [body] is the response body. It may be either a [String], a [List], a /// [Stream>], or `null` to indicate no body. If it's a [String], /// [encoding] is used to encode it to a [Stream>]. It defaults to /// UTF-8. @@ -81,7 +81,7 @@ class Response extends Message { /// URI. [location] is that URI; it can be either a [String] or a [Uri]. It's /// automatically set as the Location header in [headers]. /// - /// [body] is the response body. It may be either a [String], a + /// [body] is the response body. It may be either a [String], a [List], a /// [Stream>], or `null` to indicate no body. If it's a [String], /// [encoding] is used to encode it to a [Stream>]. It defaults to /// UTF-8. @@ -101,7 +101,7 @@ class Response extends Message { /// [String] or a [Uri]. It's automatically set as the Location header in /// [headers]. /// - /// [body] is the response body. It may be either a [String], a + /// [body] is the response body. It may be either a [String], a [List], a /// [Stream>], or `null` to indicate no body. If it's a [String], /// [encoding] is used to encode it to a [Stream>]. It defaults to /// UTF-8. @@ -141,10 +141,10 @@ class Response extends Message { /// /// This indicates that the server is refusing to fulfill the request. /// - /// [body] is the response body. It may be a [String], a [Stream>], - /// or `null`. If it's a [String], [encoding] is used to encode it to a - /// [Stream>]. The default encoding is UTF-8. If it's `null` or not - /// passed, a default error message is used. + /// [body] is the response body. It may be a [String], a [List], a + /// [Stream>], or `null`. If it's a [String], [encoding] is used to + /// encode it to a [Stream>]. The default encoding is UTF-8. If it's + /// `null` or not passed, a default error message is used. /// /// If [encoding] is passed, the "encoding" field of the Content-Type header /// in [headers] will be set appropriately. If there is no existing @@ -161,10 +161,10 @@ class Response extends Message { /// This indicates that the server didn't find any resource matching the /// requested URI. /// - /// [body] is the response body. It may be a [String], a [Stream>], - /// or `null`. If it's a [String], [encoding] is used to encode it to a - /// [Stream>]. The default encoding is UTF-8. If it's `null` or not - /// passed, a default error message is used. + /// [body] is the response body. It may be a [String], a [List], a + /// [Stream>], or `null`. If it's a [String], [encoding] is used to + /// encode it to a [Stream>]. The default encoding is UTF-8. If it's + /// `null` or not passed, a default error message is used. /// /// If [encoding] is passed, the "encoding" field of the Content-Type header /// in [headers] will be set appropriately. If there is no existing @@ -181,10 +181,10 @@ class Response extends Message { /// This indicates that the server had an internal error that prevented it /// from fulfilling the request. /// - /// [body] is the response body. It may be a [String], a [Stream>], - /// or `null`. If it's a [String], [encoding] is used to encode it to a - /// [Stream>]. The default encoding is UTF-8. If it's `null` or not - /// passed, a default error message is used. + /// [body] is the response body. It may be a [String], a [List], a + /// [Stream>], or `null`. If it's a [String], [encoding] is used to + /// encode it to a [Stream>]. The default encoding is UTF-8. If it's + /// `null` or not passed, a default error message is used. /// /// If [encoding] is passed, the "encoding" field of the Content-Type header /// in [headers] will be set appropriately. If there is no existing @@ -200,10 +200,10 @@ class Response extends Message { /// /// [statusCode] must be greater than or equal to 100. /// - /// [body] is the response body. It may be either a [String], a - /// [Stream>], or `null` to indicate no body. - /// If it's a [String], [encoding] is used to encode it to a - /// [Stream>]. The default encoding is UTF-8. + /// [body] is the response body. It may be either a [String], a [List], a + /// [Stream>], or `null` to indicate no body. If it's a [String], + /// [encoding] is used to encode it to a [Stream>]. The default + /// encoding is UTF-8. /// /// If [encoding] is passed, the "encoding" field of the Content-Type header /// in [headers] will be set appropriately. If there is no existing @@ -229,8 +229,8 @@ class Response extends Message { /// All other context and header values from the [Response] will be included /// in the copied [Response] unchanged. /// - /// [body] is the request body. It may be either a [String] or a - /// [Stream>]. + /// [body] is the request body. It may be either a [String], a [List], a + /// [Stream>], or `null` to indicate no body. Response change( {Map headers, Map context, body}) { headers = updateMap(this.headers, headers); diff --git a/lib/src/shelf_unmodifiable_map.dart b/lib/src/shelf_unmodifiable_map.dart index 18884e6f..20eb20af 100644 --- a/lib/src/shelf_unmodifiable_map.dart +++ b/lib/src/shelf_unmodifiable_map.dart @@ -4,6 +4,7 @@ import 'dart:collection'; +import 'package:collection/collection.dart'; import 'package:http_parser/http_parser.dart'; /// A simple wrapper over [UnmodifiableMapView] which avoids re-wrapping itself. @@ -42,6 +43,9 @@ class ShelfUnmodifiableMap extends UnmodifiableMapView { return new ShelfUnmodifiableMap._(source, ignoreKeyCase); } + /// Returns an empty [ShelfUnmodifiableMap]. + const factory ShelfUnmodifiableMap.empty() = _EmptyShelfUnmodifiableMap; + ShelfUnmodifiableMap._(Map source, this._ignoreKeyCase) : super(source); } diff --git a/lib/src/util.dart b/lib/src/util.dart index 6fd62d6f..a59e9128 100644 --- a/lib/src/util.dart +++ b/lib/src/util.dart @@ -4,6 +4,10 @@ import 'dart:async'; +import 'package:collection/collection.dart'; + +import 'shelf_unmodifiable_map.dart'; + /// Like [new Future], but avoids around issue 11911 by using [new Future.value] /// under the covers. Future newFuture(callback()) => new Future.value().then((_) => callback()); @@ -44,3 +48,17 @@ Map addHeader( headers[name] = value; return headers; } + +/// Returns the header with the given [name] in [headers]. +/// +/// This works even if [headers] is `null`, or if it's not yet a +/// case-insensitive map. +String getHeader(Map headers, String name) { + if (headers == null) return null; + if (headers is ShelfUnmodifiableMap) return headers[name]; + + for (var key in headers.keys) { + if (equalsIgnoreAsciiCase(key, name)) return headers[key]; + } + return null; +} diff --git a/pubspec.yaml b/pubspec.yaml index 23760779..38ad9e86 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: shelf -version: 0.6.5+3 +version: 0.6.6 author: Dart Team description: Web Server Middleware for Dart homepage: https://github.com/dart-lang/shelf @@ -7,6 +7,7 @@ environment: sdk: '>=1.12.0 <2.0.0' dependencies: async: '^1.10.0' + collection: '^1.5.0' http_parser: '>=1.0.0 <4.0.0' path: '^1.0.0' stack_trace: '^1.0.0' diff --git a/test/message_change_test.dart b/test/message_change_test.dart index 20f77406..638f9dc5 100644 --- a/test/message_change_test.dart +++ b/test/message_change_test.dart @@ -53,7 +53,7 @@ void _testChange(Message factory( }); }); - test('with empty headers returns indentical instance', () { + test('with empty headers returns identical instance', () { var request = factory(headers: {'header1': 'header value 1'}); var copy = request.change(headers: {}); @@ -71,14 +71,18 @@ void _testChange(Message factory( var request = factory(headers: {'test': 'test value'}); var copy = request.change(headers: {'test2': 'test2 value'}); - expect(copy.headers, {'test': 'test value', 'test2': 'test2 value'}); + expect(copy.headers, { + 'test': 'test value', + 'test2': 'test2 value', + 'content-length': '0' + }); }); test('existing header values are overwritten', () { var request = factory(headers: {'test': 'test value'}); var copy = request.change(headers: {'test': 'new test value'}); - expect(copy.headers, {'test': 'new test value'}); + expect(copy.headers, {'test': 'new test value', 'content-length': '0'}); }); test('new context values are added', () { diff --git a/test/message_test.dart b/test/message_test.dart index 51596cbb..adc7f639 100644 --- a/test/message_test.dart +++ b/test/message_test.dart @@ -36,9 +36,11 @@ void main() { expect(message.headers, containsPair('FOO', 'bar')); }); - test('null header value becomes empty, immutable', () { + test('null header value becomes default', () { var message = _createMessage(); - expect(message.headers, isEmpty); + expect(message.headers, equals({'content-length': '0'})); + expect(message.headers, containsPair('CoNtEnT-lEnGtH', '0')); + expect(message.headers, same(_createMessage().headers)); expect(() => message.headers['h1'] = 'value1', throwsUnsupportedError); }); @@ -128,6 +130,13 @@ void main() { }); }); + test("supports a List body", () { + var controller = new StreamController(); + var request = _createMessage(body: HELLO_BYTES); + expect(request.read().toList(), + completion(equals([HELLO_BYTES]))); + }); + test("throws when calling read()/readAsString() multiple times", () { var request; @@ -149,16 +158,46 @@ void main() { }); }); - group("contentLength", () { - test("is null without a content-length header", () { + group("content-length", () { + test("is 0 with a default body and without a content-length header", () { var request = _createMessage(); + expect(request.contentLength, 0); + }); + + test("comes from a byte body", () { + var request = _createMessage(body: [1, 2, 3]); + expect(request.contentLength, 3); + }); + + test("comes from a string body", () { + var request = _createMessage(body: 'foobar'); + expect(request.contentLength, 6); + }); + + test("is set based on byte length for a string body", () { + var request = _createMessage(body: 'fööbär'); + expect(request.contentLength, 9); + + request = _createMessage(body: 'fööbär', encoding: LATIN1); + expect(request.contentLength, 6); + }); + + test("is null for a stream body", () { + var request = _createMessage(body: new Stream.empty()); expect(request.contentLength, isNull); }); - test("comes from the content-length header", () { - var request = _createMessage(headers: {'content-length': '42'}); + test("uses the content-length header for a stream body", () { + var request = _createMessage( + body: new Stream.empty(), headers: {'content-length': '42'}); expect(request.contentLength, 42); }); + + test("real body length takes precedence over content-length header", () { + var request = _createMessage( + body: [1, 2, 3], headers: {'content-length': '42'}); + expect(request.contentLength, 3); + }); }); group("mimeType", () {