Add tests to verify NUL, CR & LF header value behavior (#1440)

Author: brianquinlan

pkgs/cupertino_http/example/integration_test/client_conformance_test.dart

@@ -20,6 +20,7 @@ void main() {
           CupertinoClient.defaultSessionConfiguration,
           canReceiveSetCookieHeaders: true,
           canSendCookieHeaders: true,
+          correctlyHandlesNullHeaderValues: false,
         );
       } finally {
         HttpClientRequestProfile.profilingEnabled = profile;
@@ -33,6 +34,7 @@ void main() {
           CupertinoClient.defaultSessionConfiguration,
           canReceiveSetCookieHeaders: true,
           canSendCookieHeaders: true,
+          correctlyHandlesNullHeaderValues: false,
         );
       } finally {
         HttpClientRequestProfile.profilingEnabled = profile;
@@ -46,6 +48,7 @@ void main() {
       canWorkInIsolates: false,
       canReceiveSetCookieHeaders: true,
       canSendCookieHeaders: true,
+      correctlyHandlesNullHeaderValues: false,
     );
   });
 }

pkgs/http/test/io/client_conformance_test.dart

@@ -14,5 +14,7 @@ void main() {
     IOClient.new, preservesMethodCase: false, // https://dartbug.com/54187
     canReceiveSetCookieHeaders: true,
     canSendCookieHeaders: true,
+    correctlyHandlesNullHeaderValues:
+        false, // https://github.com/dart-lang/sdk/issues/56636
   );
 }

pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart

@@ -69,6 +69,9 @@ export 'src/server_errors_test.dart' show testServerErrors;
 /// If [supportsFoldedHeaders] is `false` then the tests that assume that the
 /// [Client] can parse folded headers will be skipped.
 ///
+/// If [correctlyHandlesNullHeaderValues] is `false` then the tests that assume
+/// that the [Client] correctly deals with NUL in header values are skipped.
+///
 /// If [supportsMultipartRequest] is `false` then tests that assume that
 /// multipart requests can be sent will be skipped.
 ///
@@ -83,6 +86,7 @@ void testAll(
   bool canWorkInIsolates = true,
   bool preservesMethodCase = false,
   bool supportsFoldedHeaders = true,
+  bool correctlyHandlesNullHeaderValues = true,
   bool canSendCookieHeaders = false,
   bool canReceiveSetCookieHeaders = false,
   bool supportsMultipartRequest = true,
@@ -97,7 +101,8 @@ void testAll(
   testRequestHeaders(clientFactory());
   testRequestMethods(clientFactory(), preservesMethodCase: preservesMethodCase);
   testResponseHeaders(clientFactory(),
-      supportsFoldedHeaders: supportsFoldedHeaders);
+      supportsFoldedHeaders: supportsFoldedHeaders,
+      correctlyHandlesNullHeaderValues: correctlyHandlesNullHeaderValues);
   testResponseStatusLine(clientFactory());
   testRedirect(clientFactory(), redirectAlwaysAllowed: redirectAlwaysAllowed);
   testServerErrors(clientFactory());

pkgs/http_client_conformance_tests/lib/src/response_headers_tests.dart

@@ -11,8 +11,15 @@ import 'response_headers_server_vm.dart'
     if (dart.library.js_interop) 'response_headers_server_web.dart';
 
 /// Tests that the [Client] correctly processes response headers.
+///
+/// If [supportsFoldedHeaders] is `false` then the tests that assume that the
+/// [Client] can parse folded headers will be skipped.
+///
+/// If [correctlyHandlesNullHeaderValues] is `false` then the tests that assume
+/// that the [Client] correctly deals with NUL in header values are skipped.
 void testResponseHeaders(Client client,
-    {bool supportsFoldedHeaders = true}) async {
+    {bool supportsFoldedHeaders = true,
+    bool correctlyHandlesNullHeaderValues = true}) async {
   group('server headers', () {
     late String host;
     late StreamChannel<Object?> httpServerChannel;
@@ -123,6 +130,77 @@ void testResponseHeaders(Client client,
           matches(r'apple[ \t]*,[ \t]*orange[ \t]*,[ \t]*banana'));
     });
 
+    group('invalid headers values', () {
+      // From RFC-9110:
+      // Field values containing CR, LF, or NUL characters are invalid and
+      // dangerous, due to the varying ways that implementations might parse and
+      // interpret those characters; a recipient of CR, LF, or NUL within a
+      // field value MUST either reject the message or replace each of those
+      // characters with SP before further processing or forwarding of that
+      // message.
+      test('NUL', () async {
+        httpServerChannel.sink.add('invalid: 1\x002\r\n');
+
+        try {
+          final response = await client.get(Uri.http(host, ''));
+          expect(response.headers['invalid'], '1 2');
+        } on ClientException {
+          // The client rejected the response, which is allowed per RFC-9110.
+        }
+      },
+          skip: !correctlyHandlesNullHeaderValues
+              ? 'does not correctly handle NUL in header values'
+              : false);
+
+      // Bare CR/LF seem to be interpreted the same as CR + LF by most clients
+      // so allow that behavior.
+      test('LF', () async {
+        httpServerChannel.sink.add('foo: 1\n2\r\n');
+
+        try {
+          final response = await client.get(Uri.http(host, ''));
+          expect(
+              response.headers['foo'],
+              anyOf(
+                  '1 2', // RFC-specified behavior
+                  '1' // Common client behavior.
+                  ));
+        } on ClientException {
+          // The client rejected the response, which is allowed per RFC-9110.
+        }
+      });
+
+      test('CR', () async {
+        httpServerChannel.sink.add('foo: 1\r2\r\n');
+
+        try {
+          final response = await client.get(Uri.http(host, ''));
+          expect(
+              response.headers['foo'],
+              anyOf(
+                  '1 2', // RFC-specified behavior
+                  '1' // Common client behavior.
+                  ));
+        } on ClientException {
+          // The client rejected the response, which is allowed per RFC-9110.
+        }
+      });
+    });
+
+    test('quotes', () async {
+      httpServerChannel.sink.add('FOO: "1, 2, 3"\r\n');
+
+      final response = await client.get(Uri.http(host, ''));
+      expect(response.headers['foo'], '"1, 2, 3"');
+    });
+
+    test('nested quotes', () async {
+      httpServerChannel.sink.add('FOO: "\\"1, 2, 3\\""\r\n');
+
+      final response = await client.get(Uri.http(host, ''));
+      expect(response.headers['foo'], '"\\"1, 2, 3\\""');
+    });
+
     group('content length', () {
       test('surrounded in spaces', () async {
         // RFC-2616 4.2 says: