From 1999c4192961fa00c8f6caa558855936e6cced59 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Tue, 16 Jan 2024 13:14:40 -0800 Subject: [PATCH] Move `headersSplitValues` into `BaseClient` --- pkgs/http/CHANGELOG.md | 6 + pkgs/http/lib/http.dart | 2 +- pkgs/http/lib/src/base_response.dart | 124 ++++++++++-------- pkgs/http/lib/src/io_client.dart | 7 +- pkgs/http/lib/src/response.dart | 13 +- pkgs/http/pubspec.yaml | 2 +- pkgs/http/test/response_test.dart | 46 ++++++- .../lib/src/response_cookies_test.dart | 9 ++ .../lib/src/response_headers_tests.dart | 35 ++++- 9 files changed, 170 insertions(+), 74 deletions(-) diff --git a/pkgs/http/CHANGELOG.md b/pkgs/http/CHANGELOG.md index 90eb36aa6b..bd820d6370 100644 --- a/pkgs/http/CHANGELOG.md +++ b/pkgs/http/CHANGELOG.md @@ -1,3 +1,9 @@ +## 2.0.0-wip + +* `BaseResponse` can be constructed `headers` as a + `Map>`. +* Move `headersSplitValues` to `BaseResponse`. + ## 1.2.0-wip * Add `MockClient.pngResponse`, which makes it easier to fake image responses. diff --git a/pkgs/http/lib/http.dart b/pkgs/http/lib/http.dart index bd039c8519..f7a44b5431 100644 --- a/pkgs/http/lib/http.dart +++ b/pkgs/http/lib/http.dart @@ -16,7 +16,7 @@ import 'src/streamed_request.dart'; export 'src/base_client.dart'; export 'src/base_request.dart'; -export 'src/base_response.dart' show BaseResponse, HeadersWithSplitValues; +export 'src/base_response.dart' show BaseResponse; export 'src/byte_stream.dart'; export 'src/client.dart' hide zoneClient; export 'src/exception.dart'; diff --git a/pkgs/http/lib/src/base_response.dart b/pkgs/http/lib/src/base_response.dart index e1796e1b36..1e2ad29884 100644 --- a/pkgs/http/lib/src/base_response.dart +++ b/pkgs/http/lib/src/base_response.dart @@ -5,11 +5,42 @@ import 'base_client.dart'; import 'base_request.dart'; +/// "token" as defined in RFC 2616, 2.2 +/// See https://datatracker.ietf.org/doc/html/rfc2616#section-2.2 +const _tokenChars = r"!#$%&'*+\-.0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ^_`" + 'abcdefghijklmnopqrstuvwxyz|~'; + +/// Splits comma-seperated header values. +var _headerSplitter = RegExp(r'[ \t]*,[ \t]*'); + +/// Splits comma-seperated "Set-Cookie" header values. +/// +/// Set-Cookie strings can contain commas. In particular, the following +/// productions defined in RFC-6265, section 4.1.1: +/// - e.g. "Expires=Sun, 06 Nov 1994 08:49:37 GMT" +/// - e.g. "Path=somepath," +/// - e.g. "AnyString,Really," +/// +/// Some values are ambiguous e.g. +/// "Set-Cookie: lang=en; Path=/foo/" +/// "Set-Cookie: SID=x23" +/// and: +/// "Set-Cookie: lang=en; Path=/foo/,SID=x23" +/// would both be result in `response.headers` => "lang=en; Path=/foo/,SID=x23" +/// +/// The idea behind this regex is that ",=" is more likely to +/// start a new then be part of or . +/// +/// See https://datatracker.ietf.org/doc/html/rfc6265#section-4.1.1 +var _setCookieSplitter = RegExp(r'[ \t]*,[ \t]*(?=[' + _tokenChars + r']+=)'); + /// The base class for HTTP responses. /// /// Subclasses of [BaseResponse] are usually not constructed manually; instead, /// they're returned by [BaseClient.send] or other HTTP client methods. abstract class BaseResponse { + final Object _headers; // Map | Map> + /// The (frozen) request that triggered this response. final BaseRequest? request; @@ -43,64 +74,17 @@ abstract class BaseResponse { /// // values = ['Apple', 'Banana', 'Grape'] /// ``` /// - /// To retrieve the header values as a `List`, use - /// [HeadersWithSplitValues.headersSplitValues]. - /// /// If a header value contains whitespace then that whitespace may be replaced /// by a single space. Leading and trailing whitespace in header values are /// always removed. - final Map headers; - - final bool isRedirect; - - /// Whether the server requested that a persistent connection be maintained. - final bool persistentConnection; - - BaseResponse(this.statusCode, - {this.contentLength, - this.request, - this.headers = const {}, - this.isRedirect = false, - this.persistentConnection = true, - this.reasonPhrase}) { - if (statusCode < 100) { - throw ArgumentError('Invalid status code $statusCode.'); - } else if (contentLength != null && contentLength! < 0) { - throw ArgumentError('Invalid content length $contentLength.'); - } - } -} + Map get headers => switch (_headers) { + Map commaHeaders => commaHeaders, + Map> listHeaders => { + for (var v in listHeaders.entries) v.key: v.value.join(', ') + }, + _ => throw StateError('unexpected header type: ${_headers.runtimeType}') + }; -/// "token" as defined in RFC 2616, 2.2 -/// See https://datatracker.ietf.org/doc/html/rfc2616#section-2.2 -const _tokenChars = r"!#$%&'*+\-.0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ^_`" - 'abcdefghijklmnopqrstuvwxyz|~'; - -/// Splits comma-seperated header values. -var _headerSplitter = RegExp(r'[ \t]*,[ \t]*'); - -/// Splits comma-seperated "Set-Cookie" header values. -/// -/// Set-Cookie strings can contain commas. In particular, the following -/// productions defined in RFC-6265, section 4.1.1: -/// - e.g. "Expires=Sun, 06 Nov 1994 08:49:37 GMT" -/// - e.g. "Path=somepath," -/// - e.g. "AnyString,Really," -/// -/// Some values are ambiguous e.g. -/// "Set-Cookie: lang=en; Path=/foo/" -/// "Set-Cookie: SID=x23" -/// and: -/// "Set-Cookie: lang=en; Path=/foo/,SID=x23" -/// would both be result in `response.headers` => "lang=en; Path=/foo/,SID=x23" -/// -/// The idea behind this regex is that ",=" is more likely to -/// start a new then be part of or . -/// -/// See https://datatracker.ietf.org/doc/html/rfc6265#section-4.1.1 -var _setCookieSplitter = RegExp(r'[ \t]*,[ \t]*(?=[' + _tokenChars + r']+=)'); - -extension HeadersWithSplitValues on BaseResponse { /// The HTTP headers returned by the server. /// /// The header names are converted to lowercase and stored with their @@ -120,8 +104,13 @@ extension HeadersWithSplitValues on BaseResponse { /// Cookie.fromSetCookieValue(value) /// ]; Map> get headersSplitValues { + if (_headers is Map>) { + return _headers; + } else if (_headers is! Map) { + throw StateError('unexpected header type: ${_headers.runtimeType}'); + } var headersWithFieldLists = >{}; - headers.forEach((key, value) { + _headers.forEach((key, value) { if (!value.contains(',')) { headersWithFieldLists[key] = [value]; } else { @@ -134,4 +123,29 @@ extension HeadersWithSplitValues on BaseResponse { }); return headersWithFieldLists; } + + final bool isRedirect; + + /// Whether the server requested that a persistent connection be maintained. + final bool persistentConnection; + + BaseResponse(this.statusCode, + {this.contentLength, + this.request, + Object headers = const >{}, + this.isRedirect = false, + this.persistentConnection = true, + this.reasonPhrase}) + : _headers = headers { + if (_headers is! Map> && + _headers is! Map) { + throw ArgumentError.value(headers, 'headers', + 'must be a Map> or Map'); + } + if (statusCode < 100) { + throw ArgumentError('Invalid status code $statusCode.'); + } else if (contentLength != null && contentLength! < 0) { + throw ArgumentError('Invalid content length $contentLength.'); + } + } } diff --git a/pkgs/http/lib/src/io_client.dart b/pkgs/http/lib/src/io_client.dart index db66b028c4..7c3d7e6e7e 100644 --- a/pkgs/http/lib/src/io_client.dart +++ b/pkgs/http/lib/src/io_client.dart @@ -108,12 +108,9 @@ class IOClient extends BaseClient { var response = await stream.pipe(ioRequest) as HttpClientResponse; - var headers = {}; + var headers = >{}; response.headers.forEach((key, values) { - // TODO: Remove trimRight() when - // https://github.com/dart-lang/sdk/issues/53005 is resolved and the - // package:http SDK constraint requires that version or later. - headers[key] = values.map((value) => value.trimRight()).join(','); + headers[key] = values; }); return IOStreamedResponse( diff --git a/pkgs/http/lib/src/response.dart b/pkgs/http/lib/src/response.dart index 1ba7c466cf..1be9adb9dd 100644 --- a/pkgs/http/lib/src/response.dart +++ b/pkgs/http/lib/src/response.dart @@ -30,7 +30,7 @@ class Response extends BaseResponse { /// Creates a new HTTP response with a string body. Response(String body, int statusCode, {BaseRequest? request, - Map headers = const {}, + Object headers = const >{}, bool isRedirect = false, bool persistentConnection = true, String? reasonPhrase}) @@ -68,14 +68,19 @@ class Response extends BaseResponse { /// /// Defaults to [latin1] if the headers don't specify a charset or if that /// charset is unknown. -Encoding _encodingForHeaders(Map headers) => +Encoding _encodingForHeaders(Object headers) => encodingForCharset(_contentTypeForHeaders(headers).parameters['charset']); /// Returns the [MediaType] object for the given headers's content-type. /// /// Defaults to `application/octet-stream`. -MediaType _contentTypeForHeaders(Map headers) { - var contentType = headers['content-type']; +MediaType _contentTypeForHeaders(Object headers) { + final contentType = switch (headers) { + Map commaHeaders => commaHeaders['content-type'], + Map> listHeaders => listHeaders['content-type']?[0], + _ => throw StateError('unexpected header type: ${headers.runtimeType}') + }; + if (contentType != null) return MediaType.parse(contentType); return MediaType('application', 'octet-stream'); } diff --git a/pkgs/http/pubspec.yaml b/pkgs/http/pubspec.yaml index 31746fcb2d..1ba77e9606 100644 --- a/pkgs/http/pubspec.yaml +++ b/pkgs/http/pubspec.yaml @@ -1,5 +1,5 @@ name: http -version: 1.2.0-wip +version: 2.0.0-wip description: A composable, multi-platform, Future-based API for HTTP requests. repository: https://github.com/dart-lang/http/tree/master/pkgs/http diff --git a/pkgs/http/test/response_test.dart b/pkgs/http/test/response_test.dart index 1bd9fd8e38..a262c47e97 100644 --- a/pkgs/http/test/response_test.dart +++ b/pkgs/http/test/response_test.dart @@ -22,11 +22,18 @@ void main() { [72, 101, 108, 108, 111, 44, 32, 119, 111, 114, 108, 100, 33])); }); - test('respects the inferred encoding', () { + test('respects the inferred encoding, comma-separated values headers', () { var response = http.Response('föøbãr', 200, headers: {'content-type': 'text/plain; charset=iso-8859-1'}); expect(response.bodyBytes, equals([102, 246, 248, 98, 227, 114])); }); + + test('respects the inferred encoding, list values headers', () { + var response = http.Response('föøbãr', 200, headers: { + 'content-type': ['text/plain; charset=iso-8859-1'] + }); + expect(response.bodyBytes, equals([102, 246, 248, 98, 227, 114])); + }); }); group('.bytes()', () { @@ -40,11 +47,19 @@ void main() { expect(response.bodyBytes, equals([104, 101, 108, 108, 111])); }); - test('respects the inferred encoding', () { + test('respects the inferred encoding, comma-separated values headers', () { var response = http.Response.bytes([102, 246, 248, 98, 227, 114], 200, headers: {'content-type': 'text/plain; charset=iso-8859-1'}); expect(response.body, equals('föøbãr')); }); + + test('respects the inferred encoding, list values headers', () { + var response = http.Response.bytes([102, 246, 248, 98, 227, 114], 200, + headers: { + 'content-type': ['text/plain; charset=iso-8859-1'] + }); + expect(response.body, equals('föøbãr')); + }); }); group('.fromStream()', () { @@ -71,9 +86,32 @@ void main() { }); }); - group('.headersSplitValues', () { + group('.headers from values list', () { test('no headers', () async { - var response = http.Response('Hello, world!', 200); + var response = http.Response('Hello, world!', 200, + headers: const >{}); + expect(response.headers, const {}); + }); + + test('one header', () async { + var response = http.Response('Hello, world!', 200, headers: const { + 'fruit': ['apple'] + }); + expect(response.headers, const {'fruit': 'apple'}); + }); + + test('two headers', () async { + var response = http.Response('Hello, world!', 200, headers: { + 'fruit': ['apple', 'banana'] + }); + expect(response.headers, const {'fruit': 'apple, banana'}); + }); + }); + + group('.headersSplitValues from comma-separated values', () { + test('no headers', () async { + var response = http.Response('Hello, world!', 200, + headers: const {}); expect(response.headersSplitValues, const >{}); }); diff --git a/pkgs/http_client_conformance_tests/lib/src/response_cookies_test.dart b/pkgs/http_client_conformance_tests/lib/src/response_cookies_test.dart index f8e154d611..3195360f3e 100644 --- a/pkgs/http_client_conformance_tests/lib/src/response_cookies_test.dart +++ b/pkgs/http_client_conformance_tests/lib/src/response_cookies_test.dart @@ -32,6 +32,7 @@ void testResponseCookies(Client client, final response = await client.get(Uri.http(host, '')); expect(response.headers['set-cookie'], 'SID=1231AB3'); + expect(response.headersSplitValues['set-cookie'], ['SID=1231AB3']); }, skip: canReceiveSetCookieHeaders ? false @@ -51,6 +52,8 @@ void testResponseCookies(Client client, matches(r'SID=1231AB3' r'[ \t]*,[ \t]*' r'lang=en_US')); + expect(response.headersSplitValues['set-cookie'], + ['SID=1231AB3', 'lang=en_US']); }, skip: canReceiveSetCookieHeaders ? false @@ -63,6 +66,8 @@ void testResponseCookies(Client client, expect(response.headers['set-cookie'], 'id=a3fWa; Expires=Wed, 10 Jan 2024 07:28:00 GMT'); + expect(response.headersSplitValues['set-cookie'], + ['id=a3fWa; Expires=Wed, 10 Jan 2024 07:28:00 GMT']); }, skip: canReceiveSetCookieHeaders ? false @@ -84,6 +89,10 @@ void testResponseCookies(Client client, matches(r'id=a3fWa; Expires=Wed, 10 Jan 2024 07:28:00 GMT' r'[ \t]*,[ \t]*' r'id=2fasd; Expires=Wed, 21 Oct 2025 07:28:00 GMT')); + expect(response.headersSplitValues['set-cookie'], [ + 'id=a3fWa; Expires=Wed, 10 Jan 2024 07:28:00 GMT', + 'id=2fasd; Expires=Wed, 21 Oct 2025 07:28:00 GMT' + ]); }, skip: canReceiveSetCookieHeaders ? false diff --git a/pkgs/http_client_conformance_tests/lib/src/response_headers_tests.dart b/pkgs/http_client_conformance_tests/lib/src/response_headers_tests.dart index 84f0fb67f8..91f53902ce 100644 --- a/pkgs/http_client_conformance_tests/lib/src/response_headers_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/response_headers_tests.dart @@ -28,6 +28,7 @@ void testResponseHeaders(Client client) async { final response = await client.get(Uri.http(host, '')); expect(response.headers['foo'], 'bar'); + expect(response.headersSplitValues['foo'], ['bar']); }); test('UPPERCASE header name', () async { @@ -37,6 +38,7 @@ void testResponseHeaders(Client client) async { final response = await client.get(Uri.http(host, '')); expect(response.headers['foo'], 'bar'); + expect(response.headersSplitValues['foo'], ['bar']); }); test('UPPERCASE header value', () async { @@ -44,6 +46,7 @@ void testResponseHeaders(Client client) async { final response = await client.get(Uri.http(host, '')); expect(response.headers['foo'], 'BAR'); + expect(response.headersSplitValues['foo'], ['BAR']); }); test('space surrounding header value', () async { @@ -51,6 +54,7 @@ void testResponseHeaders(Client client) async { final response = await client.get(Uri.http(host, '')); expect(response.headers['foo'], 'BAR'); + expect(response.headersSplitValues['foo'], ['BAR']); }); test('space in header value', () async { @@ -58,6 +62,7 @@ void testResponseHeaders(Client client) async { final response = await client.get(Uri.http(host, '')); expect(response.headers['foo'], 'BAR BAZ'); + expect(response.headersSplitValues['foo'], ['BAR BAZ']); }); test('multiple spaces in header value', () async { @@ -67,8 +72,13 @@ void testResponseHeaders(Client client) async { httpServerChannel.sink.add('foo: BAR \t BAZ\r\n'); final response = await client.get(Uri.http(host, '')); + final valueMatch = matches(RegExp('BAR {0,2}[ \t] {0,3}BAZ')); + expect(response.headers['foo'], valueMatch); expect( - response.headers['foo'], matches(RegExp('BAR {0,2}[ \t] {0,3}BAZ'))); + response.headersSplitValues['foo'], + isA>() + .having((s) => s.length, 'length', 1) + .having((s) => s[0], 'field value', valueMatch)); }); test('multiple headers', () async { @@ -79,6 +89,9 @@ void testResponseHeaders(Client client) async { expect(response.headers['field1'], 'value1'); expect(response.headers['field2'], 'value2'); expect(response.headers['field3'], 'value3'); + expect(response.headersSplitValues['field1'], ['value1']); + expect(response.headersSplitValues['field2'], ['value2']); + expect(response.headersSplitValues['field3'], ['value3']); }); test('multiple values per header', () async { @@ -91,6 +104,8 @@ void testResponseHeaders(Client client) async { final response = await client.get(Uri.http(host, '')); expect(response.headers['list'], matches(r'apple[ \t]*,[ \t]*orange[ \t]*,[ \t]*banana')); + expect( + response.headersSplitValues['list'], ['apple', 'orange', 'banana']); }); test('multiple values per header surrounded with spaces', () async { @@ -100,6 +115,8 @@ void testResponseHeaders(Client client) async { final response = await client.get(Uri.http(host, '')); expect(response.headers['list'], matches(r'apple[ \t]*,[ \t]*orange[ \t]*,[ \t]*banana')); + expect( + response.headersSplitValues['list'], ['apple', 'orange', 'banana']); }); test('multiple headers with the same name', () async { @@ -110,6 +127,8 @@ void testResponseHeaders(Client client) async { final response = await client.get(Uri.http(host, '')); expect(response.headers['list'], matches(r'apple[ \t]*,[ \t]*orange[ \t]*,[ \t]*banana')); + expect( + response.headersSplitValues['list'], ['apple', 'orange', 'banana']); }); test('multiple headers with the same name but different cases', () async { @@ -120,6 +139,8 @@ void testResponseHeaders(Client client) async { final response = await client.get(Uri.http(host, '')); expect(response.headers['list'], matches(r'apple[ \t]*,[ \t]*orange[ \t]*,[ \t]*banana')); + expect( + response.headersSplitValues['list'], ['apple', 'orange', 'banana']); }); group('content length', () { @@ -162,6 +183,7 @@ void testResponseHeaders(Client client) async { final response = await client.get(Uri.http(host, '')); expect(response.headers['foo'], 'BAR BAZ'); + expect(response.headersSplitValues['foo'], ['BAR BAZ']); }, skip: 'Enable after https://github.com/dart-lang/sdk/issues/53185 ' 'is fixed'); @@ -170,12 +192,17 @@ void testResponseHeaders(Client client) async { httpServerChannel.sink.add('foo: BAR \t \r\n \t BAZ \t \r\n'); final response = await client.get(Uri.http(host, '')); + final valueMatch = allOf( + matches(RegExp(r'BAR {0,3}[ \t]? {0,7}[ \t]? {0,3}BAZ')), + contains(' ')); // RFC 2616 4.2 allows LWS between header values to be replace with a // single space. + expect(response.headers['foo'], valueMatch); expect( - response.headers['foo'], - allOf(matches(RegExp(r'BAR {0,3}[ \t]? {0,7}[ \t]? {0,3}BAZ')), - contains(' '))); + response.headersSplitValues['foo'], + isA>() + .having((s) => s.length, 'length', 1) + .having((s) => s[0], 'field value', valueMatch)); }); }); });