Head split values conformance tests

pkgs/http/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 2.0.0-wip
+
+* `BaseResponse` can be constructed `headers` as a
+  `Map<String, List<String>>`.
+* Move `headersSplitValues` to `BaseResponse`.
+
 ## 1.2.0-wip
 
 * Add `MockClient.pngResponse`, which makes it easier to fake image responses.

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';

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:
+/// - <sane-cookie-date> e.g. "Expires=Sun, 06 Nov 1994 08:49:37 GMT"
+/// - <path-value> e.g. "Path=somepath,"
+/// - <extension-av> 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 ",<valid token>=" is more likely to
+/// start a new <cookie-pair> then be part of <path-value> or <extension-av>.
+///
+/// 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<String, String> | Map<String, List<String>>
+
   /// 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<String>`, 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<String, String> 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<String, String> get headers => switch (_headers) {
+        Map<String, String> commaHeaders => commaHeaders,
+        Map<String, List<String>> 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:
-/// - <sane-cookie-date> e.g. "Expires=Sun, 06 Nov 1994 08:49:37 GMT"
-/// - <path-value> e.g. "Path=somepath,"
-/// - <extension-av> 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 ",<valid token>=" is more likely to
-/// start a new <cookie-pair> then be part of <path-value> or <extension-av>.
-///
-/// 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<String, List<String>> get headersSplitValues {
+    if (_headers is Map<String, List<String>>) {
+      return _headers;
+    } else if (_headers is! Map<String, String>) {
+      throw StateError('unexpected header type: ${_headers.runtimeType}');
+    }
     var headersWithFieldLists = <String, List<String>>{};
-    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 <String, List<String>>{},
+      this.isRedirect = false,
+      this.persistentConnection = true,
+      this.reasonPhrase})
+      : _headers = headers {
+    if (_headers is! Map<String, List<String>> &&
+        _headers is! Map<String, String>) {
+      throw ArgumentError.value(headers, 'headers',
+          'must be a Map<String, List<String>> or Map<String, String>');
+    }
+    if (statusCode < 100) {
+      throw ArgumentError('Invalid status code $statusCode.');
+    } else if (contentLength != null && contentLength! < 0) {
+      throw ArgumentError('Invalid content length $contentLength.');
+    }
+  }
 }

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 = <String, String>{};
+      var headers = <String, List<String>>{};
       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(

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<String, String> headers = const {},
+      Object headers = const <String, List<String>>{},
       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<String, String> 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<String, String> headers) {
-  var contentType = headers['content-type'];
+MediaType _contentTypeForHeaders(Object headers) {
+  final contentType = switch (headers) {
+    Map<String, String> commaHeaders => commaHeaders['content-type'],
+    Map<String, List<String>> listHeaders => listHeaders['content-type']?[0],
+    _ => throw StateError('unexpected header type: ${headers.runtimeType}')
+  };
+
   if (contentType != null) return MediaType.parse(contentType);
   return MediaType('application', 'octet-stream');
 }

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
 

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 <String, List<String>>{});
+      expect(response.headers, const <String, String>{});
+    });
+
+    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 <String, String>{});
       expect(response.headersSplitValues, const <String, List<String>>{});
     });
 

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