From 75be84aaa84fc8ba920b090a48d7a0371346e584 Mon Sep 17 00:00:00 2001 From: Don Date: Fri, 22 Sep 2017 15:00:02 -0700 Subject: [PATCH 1/6] Fix content type --- lib/src/body.dart | 42 +++------- lib/src/message.dart | 188 ++++++++++++++++++++++++++++--------------- lib/src/utils.dart | 6 ++ 3 files changed, 141 insertions(+), 95 deletions(-) diff --git a/lib/src/body.dart b/lib/src/body.dart index e3589d0326..e5fa9fa864 100644 --- a/lib/src/body.dart +++ b/lib/src/body.dart @@ -8,6 +8,8 @@ import 'dart:convert'; import 'package:async/async.dart'; import 'package:collection/collection.dart'; +import 'utils.dart'; + /// The body of a request or response. /// /// This tracks whether the body has been read. It's separate from [Message] @@ -36,25 +38,20 @@ class Body { /// used to convert it to a [Stream>]. factory Body(body, [Encoding encoding]) { if (body is Body) return body; + if (body == null) + return new Body._(new Stream.fromIterable([]), encoding, 0); Stream> stream; int contentLength; - if (body == null) { - contentLength = 0; - stream = new Stream.fromIterable([]); - } else if (body is String) { - 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]); - } + + if (body is Map) { + body = mapToQuery(body, encoding: encoding); + } + + if (body is String) { + 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)]); @@ -68,19 +65,6 @@ class Body { 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; - - // Non-ASCII code units between U+0080 and U+009F produce 8-bit characters - // with the high bit set. - return bytes.every((byte) => byte & 0x80 == 0); - } - /// Returns a [Stream] representing the body. /// /// Can only be called once. diff --git a/lib/src/message.dart b/lib/src/message.dart index 96608dda66..ff63be7073 100644 --- a/lib/src/message.dart +++ b/lib/src/message.dart @@ -5,7 +5,6 @@ import 'dart:async'; import 'dart:convert'; -import 'package:collection/collection.dart'; import 'package:http_parser/http_parser.dart'; import 'body.dart'; @@ -45,6 +44,9 @@ abstract class Message { /// This can be read via [read] or [readAsString]. final Body _body; + /// The parsed version of the Content-Type header in [headers]. + final MediaType _contentType; + /// Creates a new [Message]. /// /// [body] is the message body. It may be either a [String], a [List], a @@ -61,14 +63,23 @@ abstract class Message { {Encoding encoding, Map headers, Map context}) - : this._(new Body(body, encoding), headers, context); + : this.__(body, _determineMediaType(body, encoding, headers), headers, + context); + + Message.__(body, MediaType contentType, Map headers, + Map context) + : this._(new Body(body, encodingForMediaType(contentType, null)), + contentType, headers, context); - Message._(Body body, Map headers, Map context) + Message._(Body body, MediaType contentType, Map headers, + Map context) : _body = body, - headers = new HttpUnmodifiableMap(_adjustHeaders(headers, body), + headers = new HttpUnmodifiableMap( + _adjustHeaders(headers, body, contentType), ignoreKeyCase: true), context = - new HttpUnmodifiableMap(context, ignoreKeyCase: false); + new HttpUnmodifiableMap(context, ignoreKeyCase: false), + _contentType = contentType; /// If `true`, the stream returned by [read] won't emit any bytes. /// @@ -84,6 +95,7 @@ abstract class Message { _contentLengthCache = int.parse(headers['content-length']); return _contentLengthCache; } + int _contentLengthCache; /// The MIME type declared in [headers]. @@ -92,11 +104,7 @@ abstract class Message { /// the MIME type, without any Content-Type parameters. /// /// If [headers] doesn't have a Content-Type header, this will be `null`. - String get mimeType { - var contentType = _contentType; - if (contentType == null) return null; - return contentType.mimeType; - } + String get mimeType => _contentType?.mimeType; /// The encoding of the body returned by [read]. /// @@ -105,23 +113,7 @@ abstract class Message { /// /// If [headers] doesn't have a Content-Type header or it specifies an /// encoding that [dart:convert] doesn't support, this will be `null`. - Encoding get encoding { - var contentType = _contentType; - if (contentType == null) return null; - if (!contentType.parameters.containsKey('charset')) return null; - return Encoding.getByName(contentType.parameters['charset']); - } - - /// The parsed version of the Content-Type header in [headers]. - /// - /// This is cached for efficient access. - MediaType get _contentType { - if (_contentTypeCache != null) return _contentTypeCache; - if (!headers.containsKey('content-type')) return null; - _contentTypeCache = new MediaType.parse(headers['content-type']); - return _contentTypeCache; - } - MediaType _contentTypeCache; + Encoding get encoding => _body.encoding; /// Returns the message body as byte chunks. /// @@ -144,55 +136,119 @@ abstract class Message { /// changes. Message change( {Map headers, Map context, body}); -} -/// Adds information about encoding to [headers]. -/// -/// Returns a new map without modifying [headers]. -Map _adjustHeaders(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 HttpUnmodifiableMap.empty(); - } else if (body.contentLength == 0 && - (headers == null || headers.isEmpty)) { - return const HttpUnmodifiableMap.empty(); + /// Determines the media type based on the [headers], [encoding] and [body]. + static MediaType _determineMediaType( + body, Encoding encoding, Map headers) => + _headerMediaType(headers, encoding) ?? _defaultMediaType(body, encoding); + + static MediaType _defaultMediaType(body, Encoding encoding) { + //if (body == null) return null; + + var parameters = {'charset': encoding?.name ?? UTF8.name}; + + if (body is String) { + return new MediaType('text', 'plain', parameters); + } else if (body is Map) { + return new MediaType('application', 'x-www-form-urlencoded', parameters); + } else if (encoding != null) { + return new MediaType('application', 'octet-stream', parameters); } + + return null; } - 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(); - } + static MediaType _headerMediaType( + Map headers, Encoding encoding) { + var contentTypeHeader = getHeader(headers, 'content-type'); + if (contentTypeHeader == null) return null; + + var contentType = new MediaType.parse(contentTypeHeader); + var parameters = { + 'charset': + encoding?.name ?? contentType.parameters['charset'] ?? UTF8.name + }; + + return contentType.change(parameters: parameters); } - if (body.contentLength != null) { - var coding = newHeaders['transfer-encoding']; - if (coding == null || equalsIgnoreAsciiCase(coding, 'identity')) { - newHeaders['content-length'] = body.contentLength.toString(); + /// Adjusts the [headers] to include information from the [body]. + /// + /// Returns a new map without modifying [headers]. + /// + /// The following headers could be added or modified. + /// * content-length + /// * content-type + static Map _adjustHeaders( + Map headers, Body body, MediaType contentType) { + var modified = {}; + + var contentLengthHeader = _adjustContentLengthHeader(headers, body); + if (contentLengthHeader.isNotEmpty) { + modified['content-length'] = contentLengthHeader; + } + + var contentTypeHeader = _adjustContentTypeHeader(headers, contentType); + if (contentTypeHeader.isNotEmpty) { + modified['content-type'] = contentTypeHeader; + } + + if (modified.isEmpty) { + return headers ?? const HttpUnmodifiableMap.empty(); } + + var newHeaders = headers == null + ? new CaseInsensitiveMap() + : new CaseInsensitiveMap.from(headers); + + newHeaders.addAll(modified); + + return newHeaders; } - return newHeaders; -} + /// Checks the `content-length` header to see if it requires modification. + /// + /// Returns an empty string when no modification is required, otherwise it + /// returns the value to set. + /// + /// If there is a contentLength specified within the [body] and it does not + /// match what is specified in the [headers] it will be modified to the body's + /// value. + static String _adjustContentLengthHeader( + Map headers, Body body) { + var bodyContentLength = body.contentLength ?? -1; + + if (bodyContentLength >= 0) { + var bodyContentHeader = bodyContentLength.toString(); + + if (getHeader(headers, 'content-length') != bodyContentHeader) { + return bodyContentHeader; + } + } + + return ''; + } -/// Returns whether [headers] declares the same encoding as [body]. -bool _sameEncoding(Map headers, Body body) { - if (body.encoding == null) return true; + /// Checks the `content-type` header to see if it requires modification. + /// + /// Returns an empty string when no modification is required, otherwise it + /// returns the value to set. + /// + /// If the contentType within [body] is different than the one specified in the + /// [headers] then body's value will be used. The [headers] were already used + /// when creating the body's contentType so this will only actually change + /// things when headers did not contain a `content-type`. + static String _adjustContentTypeHeader( + Map headers, MediaType contentType) { + var headerContentType = getHeader(headers, 'content-type'); + var bodyContentType = contentType?.toString(); - var contentType = getHeader(headers, 'content-type'); - if (contentType == null) return false; + // Neither are set so don't modify it + if ((headerContentType == null) && (bodyContentType == null)) { + return ''; + } - var charset = new MediaType.parse(contentType).parameters['charset']; - return Encoding.getByName(charset) == body.encoding; + // The value of bodyContentType will have the overridden values so use that + return headerContentType != bodyContentType ? bodyContentType : ''; + } } diff --git a/lib/src/utils.dart b/lib/src/utils.dart index 3e9fdf39bb..9073cbf9bb 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -7,6 +7,7 @@ import 'dart:convert'; import 'dart:typed_data'; import 'package:collection/collection.dart'; +import 'package:http_parser/http_parser.dart'; import 'http_unmodifiable_map.dart'; @@ -53,6 +54,11 @@ List split1(String toSplit, String pattern) { ]; } +Encoding encodingForMediaType(MediaType type, [Encoding fallback = LATIN1]) { + if (type == null) return null; + return encodingForCharset(type.parameters['charset'], fallback); +} + /// Returns the [Encoding] that corresponds to [charset]. Returns [fallback] if /// [charset] is null or if no [Encoding] was found that corresponds to /// [charset]. From 928d7a13dd060b7afac54951b22dc3acace96682 Mon Sep 17 00:00:00 2001 From: Don Date: Tue, 26 Sep 2017 22:39:34 -0700 Subject: [PATCH 2/6] Add fromValues constructor --- lib/src/message.dart | 57 +++++++++++++++++++++++++++++++------------- 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/lib/src/message.dart b/lib/src/message.dart index ff63be7073..af486bfbd6 100644 --- a/lib/src/message.dart +++ b/lib/src/message.dart @@ -44,9 +44,6 @@ abstract class Message { /// This can be read via [read] or [readAsString]. final Body _body; - /// The parsed version of the Content-Type header in [headers]. - final MediaType _contentType; - /// Creates a new [Message]. /// /// [body] is the message body. It may be either a [String], a [List], a @@ -61,25 +58,30 @@ abstract class Message { /// Content-Type header, it will be set to "application/octet-stream". Message(body, {Encoding encoding, - Map headers, - Map context}) + Map headers, + Map context}) : this.__(body, _determineMediaType(body, encoding, headers), headers, - context); + context); Message.__(body, MediaType contentType, Map headers, Map context) - : this._(new Body(body, encodingForMediaType(contentType, null)), - contentType, headers, context); + : this._(new Body(body, encodingForMediaType(contentType, null)), contentType, + headers, context); Message._(Body body, MediaType contentType, Map headers, Map context) + : this.fromValues(body, _adjustHeaders(headers, body, contentType), context); + + /// Creates a new [Message]. + /// + /// This constructor should be used when no computation is required for the + /// [body], [headers] or [context]. + Message.fromValues( + Body body, Map headers, Map context) : _body = body, - headers = new HttpUnmodifiableMap( - _adjustHeaders(headers, body, contentType), - ignoreKeyCase: true), + headers = new HttpUnmodifiableMap(headers, ignoreKeyCase: true), context = - new HttpUnmodifiableMap(context, ignoreKeyCase: false), - _contentType = contentType; + new HttpUnmodifiableMap(context, ignoreKeyCase: false); /// If `true`, the stream returned by [read] won't emit any bytes. /// @@ -104,7 +106,11 @@ abstract class Message { /// the MIME type, without any Content-Type parameters. /// /// If [headers] doesn't have a Content-Type header, this will be `null`. - String get mimeType => _contentType?.mimeType; + String get mimeType { + var contentType = _contentType; + if (contentType == null) return null; + return contentType.mimeType; + } /// The encoding of the body returned by [read]. /// @@ -113,7 +119,24 @@ abstract class Message { /// /// If [headers] doesn't have a Content-Type header or it specifies an /// encoding that [dart:convert] doesn't support, this will be `null`. - Encoding get encoding => _body.encoding; + Encoding get encoding { + var contentType = _contentType; + if (contentType == null) return null; + if (!contentType.parameters.containsKey('charset')) return null; + return Encoding.getByName(contentType.parameters['charset']); + } + + /// The parsed version of the Content-Type header in [headers]. + /// + /// This is cached for efficient access. + MediaType get _contentType { + if (_contentTypeCache != null) return _contentTypeCache; + if (!headers.containsKey('content-type')) return null; + _contentTypeCache = new MediaType.parse(headers['content-type']); + return _contentTypeCache; + } + + MediaType _contentTypeCache; /// Returns the message body as byte chunks. /// @@ -139,7 +162,7 @@ abstract class Message { /// Determines the media type based on the [headers], [encoding] and [body]. static MediaType _determineMediaType( - body, Encoding encoding, Map headers) => + body, Encoding encoding, Map headers) => _headerMediaType(headers, encoding) ?? _defaultMediaType(body, encoding); static MediaType _defaultMediaType(body, Encoding encoding) { @@ -166,7 +189,7 @@ abstract class Message { var contentType = new MediaType.parse(contentTypeHeader); var parameters = { 'charset': - encoding?.name ?? contentType.parameters['charset'] ?? UTF8.name + encoding?.name ?? contentType.parameters['charset'] ?? UTF8.name }; return contentType.change(parameters: parameters); From 5c7b451165838884b66b3fea80370b7fba442292 Mon Sep 17 00:00:00 2001 From: Don Date: Wed, 27 Sep 2017 17:36:15 -0700 Subject: [PATCH 3/6] Return an emptyStream with null body --- lib/src/body.dart | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/src/body.dart b/lib/src/body.dart index e5fa9fa864..f030ab9a1e 100644 --- a/lib/src/body.dart +++ b/lib/src/body.dart @@ -29,6 +29,9 @@ class Body { /// determined efficiently. final int contentLength; + /// An empty stream for use with empty bodies. + static const _emptyStream = const Stream.empty(); + Body._(this._stream, this.encoding, this.contentLength); /// Converts [body] to a byte stream and wraps it in a [Body]. @@ -38,8 +41,9 @@ class Body { /// used to convert it to a [Stream>]. factory Body(body, [Encoding encoding]) { if (body is Body) return body; - if (body == null) - return new Body._(new Stream.fromIterable([]), encoding, 0); + if (body == null) { + return new Body._(_emptyStream, encoding, 0); + } Stream> stream; int contentLength; From 7da22ce008ce4a172900a5020238298a34e1cfac Mon Sep 17 00:00:00 2001 From: Don Date: Wed, 27 Sep 2017 17:49:08 -0700 Subject: [PATCH 4/6] Review comments --- lib/src/body.dart | 2 ++ lib/src/message.dart | 33 ++++++++++++++++----------------- lib/src/utils.dart | 5 ----- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/lib/src/body.dart b/lib/src/body.dart index f030ab9a1e..01a834eb6c 100644 --- a/lib/src/body.dart +++ b/lib/src/body.dart @@ -48,6 +48,8 @@ class Body { Stream> stream; int contentLength; + encoding ??= UTF8; + if (body is Map) { body = mapToQuery(body, encoding: encoding); } diff --git a/lib/src/message.dart b/lib/src/message.dart index af486bfbd6..d78d22eeb1 100644 --- a/lib/src/message.dart +++ b/lib/src/message.dart @@ -58,30 +58,31 @@ abstract class Message { /// Content-Type header, it will be set to "application/octet-stream". Message(body, {Encoding encoding, - Map headers, - Map context}) - : this.__(body, _determineMediaType(body, encoding, headers), headers, - context); + Map headers, + Map context}) + : this._forwardToContentType(body, encoding, + _determineMediaType(body, encoding, headers), headers, context); - Message.__(body, MediaType contentType, Map headers, - Map context) - : this._(new Body(body, encodingForMediaType(contentType, null)), contentType, - headers, context); + Message.withContentType(Body body, MediaType contentType, + Map headers, Map context) + : this._fromValues( + body, _adjustHeaders(headers, body, contentType), context); - Message._(Body body, MediaType contentType, Map headers, - Map context) - : this.fromValues(body, _adjustHeaders(headers, body, contentType), context); + Message._forwardToContentType(body, Encoding encoding, MediaType contentType, + Map headers, Map context) + : this.withContentType( + new Body(body, encoding), contentType, headers, context); /// Creates a new [Message]. /// /// This constructor should be used when no computation is required for the /// [body], [headers] or [context]. - Message.fromValues( + Message._fromValues( Body body, Map headers, Map context) : _body = body, headers = new HttpUnmodifiableMap(headers, ignoreKeyCase: true), context = - new HttpUnmodifiableMap(context, ignoreKeyCase: false); + new HttpUnmodifiableMap(context, ignoreKeyCase: false); /// If `true`, the stream returned by [read] won't emit any bytes. /// @@ -97,7 +98,6 @@ abstract class Message { _contentLengthCache = int.parse(headers['content-length']); return _contentLengthCache; } - int _contentLengthCache; /// The MIME type declared in [headers]. @@ -135,7 +135,6 @@ abstract class Message { _contentTypeCache = new MediaType.parse(headers['content-type']); return _contentTypeCache; } - MediaType _contentTypeCache; /// Returns the message body as byte chunks. @@ -162,7 +161,7 @@ abstract class Message { /// Determines the media type based on the [headers], [encoding] and [body]. static MediaType _determineMediaType( - body, Encoding encoding, Map headers) => + body, Encoding encoding, Map headers) => _headerMediaType(headers, encoding) ?? _defaultMediaType(body, encoding); static MediaType _defaultMediaType(body, Encoding encoding) { @@ -189,7 +188,7 @@ abstract class Message { var contentType = new MediaType.parse(contentTypeHeader); var parameters = { 'charset': - encoding?.name ?? contentType.parameters['charset'] ?? UTF8.name + encoding?.name ?? contentType.parameters['charset'] ?? UTF8.name }; return contentType.change(parameters: parameters); diff --git a/lib/src/utils.dart b/lib/src/utils.dart index 9073cbf9bb..9f4dc39cc3 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -54,11 +54,6 @@ List split1(String toSplit, String pattern) { ]; } -Encoding encodingForMediaType(MediaType type, [Encoding fallback = LATIN1]) { - if (type == null) return null; - return encodingForCharset(type.parameters['charset'], fallback); -} - /// Returns the [Encoding] that corresponds to [charset]. Returns [fallback] if /// [charset] is null or if no [Encoding] was found that corresponds to /// [charset]. From d05187ac0ff00b41ec439a7d8a21d28f9b49065d Mon Sep 17 00:00:00 2001 From: Don Date: Wed, 27 Sep 2017 19:22:59 -0700 Subject: [PATCH 5/6] unused --- lib/src/utils.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/src/utils.dart b/lib/src/utils.dart index 9f4dc39cc3..3e9fdf39bb 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -7,7 +7,6 @@ import 'dart:convert'; import 'dart:typed_data'; import 'package:collection/collection.dart'; -import 'package:http_parser/http_parser.dart'; import 'http_unmodifiable_map.dart'; From 9f5fbf4f4babe9b4a0671b7bf07bb61316e2bd24 Mon Sep 17 00:00:00 2001 From: Don Date: Thu, 28 Sep 2017 16:12:01 -0700 Subject: [PATCH 6/6] Add EmptyBody implementation --- lib/src/body.dart | 12 +++++++++++- lib/src/empty_body.dart | 25 +++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 lib/src/empty_body.dart diff --git a/lib/src/body.dart b/lib/src/body.dart index 01a834eb6c..89f968579c 100644 --- a/lib/src/body.dart +++ b/lib/src/body.dart @@ -8,6 +8,7 @@ import 'dart:convert'; import 'package:async/async.dart'; import 'package:collection/collection.dart'; +import 'empty_body.dart'; import 'utils.dart'; /// The body of a request or response. @@ -42,7 +43,7 @@ class Body { factory Body(body, [Encoding encoding]) { if (body is Body) return body; if (body == null) { - return new Body._(_emptyStream, encoding, 0); + return const EmptyBody(); } Stream> stream; @@ -84,3 +85,12 @@ class Body { return stream; } } + +class _EmptyBody implements Body { + const _EmptyBody(); + + Encoding get encoding => UTF8; + int get contentLength => 0; + + Stream> read() => const Stream>.empty(); +} diff --git a/lib/src/empty_body.dart b/lib/src/empty_body.dart new file mode 100644 index 0000000000..d811b78276 --- /dev/null +++ b/lib/src/empty_body.dart @@ -0,0 +1,25 @@ +// 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:async'; +import 'dart:convert'; + +import 'body.dart'; + +/// An empty request body. +/// +/// Used as an optimization when a `null` body is used. +class EmptyBody implements Body { + /// An empty body is not encoded. + Encoding get encoding => null; + + /// An empty body has no length. + int get contentLength => 0; + + /// Creates an instance of [EmptyBody]. + const EmptyBody(); + + /// Returns an empty [Stream] representing the body. + Stream> read() => const Stream>.empty(); +}