Skip to content

Commit

Permalink
Add tests to verify NUL, CR & LF header value behavior (#1440)
Browse files Browse the repository at this point in the history
  • Loading branch information
brianquinlan authored Jan 6, 2025
1 parent 6ecd13a commit f0bcf02
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ void main() {
CupertinoClient.defaultSessionConfiguration,
canReceiveSetCookieHeaders: true,
canSendCookieHeaders: true,
correctlyHandlesNullHeaderValues: false,
);
} finally {
HttpClientRequestProfile.profilingEnabled = profile;
Expand All @@ -33,6 +34,7 @@ void main() {
CupertinoClient.defaultSessionConfiguration,
canReceiveSetCookieHeaders: true,
canSendCookieHeaders: true,
correctlyHandlesNullHeaderValues: false,
);
} finally {
HttpClientRequestProfile.profilingEnabled = profile;
Expand All @@ -46,6 +48,7 @@ void main() {
canWorkInIsolates: false,
canReceiveSetCookieHeaders: true,
canSendCookieHeaders: true,
correctlyHandlesNullHeaderValues: false,
);
});
}
2 changes: 2 additions & 0 deletions pkgs/http/test/io/client_conformance_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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,
Expand All @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit f0bcf02

Please sign in to comment.