From 68173ed602cf772e1bee7a0c41f75a5b9979210c Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 26 Jun 2024 11:47:05 -0700 Subject: [PATCH 1/2] [cupertino_http] Fix a bug where content-length was not set for multipart messages Fixes https://github.com/dart-lang/http/issues/1236 --- pkgs/cupertino_http/CHANGELOG.md | 2 + .../lib/src/cupertino_client.dart | 12 +++-- .../lib/http_client_conformance_tests.dart | 3 ++ .../lib/src/multipart_server.dart | 48 +++++++++++++++++++ .../lib/src/multipart_server_vm.dart | 14 ++++++ .../lib/src/multipart_server_web.dart | 11 +++++ .../lib/src/multipart_tests.dart | 44 +++++++++++++++++ 7 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 pkgs/http_client_conformance_tests/lib/src/multipart_server.dart create mode 100644 pkgs/http_client_conformance_tests/lib/src/multipart_server_vm.dart create mode 100644 pkgs/http_client_conformance_tests/lib/src/multipart_server_web.dart create mode 100644 pkgs/http_client_conformance_tests/lib/src/multipart_tests.dart diff --git a/pkgs/cupertino_http/CHANGELOG.md b/pkgs/cupertino_http/CHANGELOG.md index bbbcb68320..0be851c2b6 100644 --- a/pkgs/cupertino_http/CHANGELOG.md +++ b/pkgs/cupertino_http/CHANGELOG.md @@ -1,6 +1,8 @@ ## 1.5.1-wip * Allow `1000` as a `code` argument in `CupertinoWebSocket.close`. +* Fix a bug where the `Content-Length` header would not be set under certain + circumstances. ## 1.5.0 diff --git a/pkgs/cupertino_http/lib/src/cupertino_client.dart b/pkgs/cupertino_http/lib/src/cupertino_client.dart index e2ec0a04ff..a525773316 100644 --- a/pkgs/cupertino_http/lib/src/cupertino_client.dart +++ b/pkgs/cupertino_http/lib/src/cupertino_client.dart @@ -308,16 +308,18 @@ class CupertinoClient extends BaseClient { ..headersCommaValues = request.headers ..maxRedirects = request.maxRedirects; - if (profile != null && request.contentLength != null) { - profile.requestData.headersListValues = { + final urlRequest = MutableURLRequest.fromUrl(request.url) + ..httpMethod = request.method; + + if (request.contentLength != null) { + profile?.requestData.headersListValues = { 'Content-Length': ['${request.contentLength}'], ...profile.requestData.headers! }; + urlRequest.setValueForHttpHeaderField( + 'Content-Length', '${request.contentLength}'); } - final urlRequest = MutableURLRequest.fromUrl(request.url) - ..httpMethod = request.method; - if (request is Request) { // Optimize the (typical) `Request` case since assigning to // `httpBodyStream` requires a lot of expensive setup and data passing. diff --git a/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart b/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart index 2d51c18f1b..2825a449ec 100644 --- a/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart @@ -7,6 +7,7 @@ import 'package:http/http.dart'; import 'src/close_tests.dart'; import 'src/compressed_response_body_tests.dart'; import 'src/isolate_test.dart'; +import 'src/multipart_tests.dart'; import 'src/multiple_clients_tests.dart'; import 'src/redirect_tests.dart'; import 'src/request_body_streamed_tests.dart'; @@ -25,6 +26,7 @@ export 'src/close_tests.dart' show testClose; export 'src/compressed_response_body_tests.dart' show testCompressedResponseBody; export 'src/isolate_test.dart' show testIsolate; +export 'src/multipart_tests.dart' show testMultipartRequests; export 'src/multiple_clients_tests.dart' show testMultipleClients; export 'src/redirect_tests.dart' show testRedirect; export 'src/request_body_streamed_tests.dart' show testRequestBodyStreamed; @@ -97,6 +99,7 @@ void testAll( testServerErrors(clientFactory()); testCompressedResponseBody(clientFactory()); testMultipleClients(clientFactory); + testMultipartRequests(clientFactory()); testClose(clientFactory); testIsolate(clientFactory, canWorkInIsolates: canWorkInIsolates); testRequestCookies(clientFactory(), diff --git a/pkgs/http_client_conformance_tests/lib/src/multipart_server.dart b/pkgs/http_client_conformance_tests/lib/src/multipart_server.dart new file mode 100644 index 0000000000..90e4d8b1a7 --- /dev/null +++ b/pkgs/http_client_conformance_tests/lib/src/multipart_server.dart @@ -0,0 +1,48 @@ +// Copyright (c) 2024, 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 'dart:io'; + +import 'package:stream_channel/stream_channel.dart'; + +/// Starts an HTTP server that captures the request headers and body. +/// +/// Channel protocol: +/// On Startup: +/// - send port +/// On Request Received: +/// - send the received headers and request body +/// When Receive Anything: +/// - exit +void hybridMain(StreamChannel channel) async { + late HttpServer server; + + server = (await HttpServer.bind('localhost', 0)) + ..listen((request) async { + request.response.headers.set('Access-Control-Allow-Origin', '*'); + if (request.method == 'OPTIONS') { + // Handle a CORS preflight request: + // https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#preflighted_requests + request.response.headers + ..set('Access-Control-Allow-Methods', '*') + ..set('Access-Control-Allow-Headers', '*'); + } else { + final headers = >{}; + request.headers.forEach((field, value) { + headers[field] = value; + }); + final body = + await const Utf8Decoder().bind(request).fold('', (x, y) => '$x$y'); + channel.sink.add((headers, body)); + } + unawaited(request.response.close()); + }); + + channel.sink.add(server.port); + await channel + .stream.first; // Any writes indicates that the server should exit. + unawaited(server.close()); +} diff --git a/pkgs/http_client_conformance_tests/lib/src/multipart_server_vm.dart b/pkgs/http_client_conformance_tests/lib/src/multipart_server_vm.dart new file mode 100644 index 0000000000..3168e6c036 --- /dev/null +++ b/pkgs/http_client_conformance_tests/lib/src/multipart_server_vm.dart @@ -0,0 +1,14 @@ +// Generated by generate_server_wrappers.dart. Do not edit. + +import 'package:stream_channel/stream_channel.dart'; + +import 'multipart_server.dart'; + +export 'server_queue_helpers.dart' show StreamQueueOfNullableObjectExtension; + +/// Starts the redirect test HTTP server in the same process. +Future> startServer() async { + final controller = StreamChannelController(sync: true); + hybridMain(controller.foreign); + return controller.local; +} diff --git a/pkgs/http_client_conformance_tests/lib/src/multipart_server_web.dart b/pkgs/http_client_conformance_tests/lib/src/multipart_server_web.dart new file mode 100644 index 0000000000..8d8e88108a --- /dev/null +++ b/pkgs/http_client_conformance_tests/lib/src/multipart_server_web.dart @@ -0,0 +1,11 @@ +// Generated by generate_server_wrappers.dart. Do not edit. + +import 'package:stream_channel/stream_channel.dart'; +import 'package:test/test.dart'; + +export 'server_queue_helpers.dart' show StreamQueueOfNullableObjectExtension; + +/// Starts the redirect test HTTP server out-of-process. +Future> startServer() async => spawnHybridUri(Uri( + scheme: 'package', + path: 'http_client_conformance_tests/src/multipart_server.dart')); diff --git a/pkgs/http_client_conformance_tests/lib/src/multipart_tests.dart b/pkgs/http_client_conformance_tests/lib/src/multipart_tests.dart new file mode 100644 index 0000000000..95065e519d --- /dev/null +++ b/pkgs/http_client_conformance_tests/lib/src/multipart_tests.dart @@ -0,0 +1,44 @@ +// Copyright (c) 2024, 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 'package:async/async.dart'; +import 'package:http/http.dart'; +import 'package:stream_channel/stream_channel.dart'; +import 'package:test/test.dart'; + +import 'multipart_server_vm.dart' + if (dart.library.js_interop) 'multipart_server_web.dart'; + +/// Tests that the [Client] correctly sends [MultipartRequest]. +void testMultipartRequests(Client client) async { + group('multipart requests', () { + late final String host; + late final StreamChannel httpServerChannel; + late final StreamQueue httpServerQueue; + + setUpAll(() async { + httpServerChannel = await startServer(); + httpServerQueue = StreamQueue(httpServerChannel.stream); + host = 'localhost:${await httpServerQueue.nextAsInt}'; + }); + tearDownAll(() => httpServerChannel.sink.add(null)); + + test('attached file', () async { + final request = MultipartRequest('POST', Uri.http(host, '')); + + request.files.add(MultipartFile.fromString('file1', 'Hello World')); + + await client.send(request); + final (headers, body) = + await httpServerQueue.next as (Map>, String); + expect(headers['content-length']!.single, '${request.contentLength}'); + expect(headers['content-type']!.single, + startsWith('multipart/form-data; boundary=')); + expect(body, contains('''content-type: text/plain; charset=utf-8\r +content-disposition: form-data; name="file1"\r +\r +Hello World''')); + }); + }); +} From 65f6ecaa6fac3f0cec6917f9ea19c4bb59ef3261 Mon Sep 17 00:00:00 2001 From: Brian Quinlan Date: Wed, 26 Jun 2024 12:04:04 -0700 Subject: [PATCH 2/2] Skip --- pkgs/http/test/html/client_conformance_test.dart | 3 ++- .../lib/http_client_conformance_tests.dart | 7 ++++++- .../lib/src/multipart_tests.dart | 11 +++++++++-- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/pkgs/http/test/html/client_conformance_test.dart b/pkgs/http/test/html/client_conformance_test.dart index b4f567dfa7..9505239564 100644 --- a/pkgs/http/test/html/client_conformance_test.dart +++ b/pkgs/http/test/html/client_conformance_test.dart @@ -14,5 +14,6 @@ void main() { redirectAlwaysAllowed: true, canStreamRequestBody: false, canStreamResponseBody: false, - canWorkInIsolates: false); + canWorkInIsolates: false, + supportsMultipartRequest: false); } diff --git a/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart b/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart index 2825a449ec..1a43a6b144 100644 --- a/pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart +++ b/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 [supportsMultipartRequest] is `false` then tests that assume that +/// multipart requests can be sent will be skipped. +/// /// The tests are run against a series of HTTP servers that are started by the /// tests. If the tests are run in the browser, then the test servers are /// started in another process. Otherwise, the test servers are run in-process. @@ -82,6 +85,7 @@ void testAll( bool supportsFoldedHeaders = true, bool canSendCookieHeaders = false, bool canReceiveSetCookieHeaders = false, + bool supportsMultipartRequest = true, }) { testRequestBody(clientFactory()); testRequestBodyStreamed(clientFactory(), @@ -99,7 +103,8 @@ void testAll( testServerErrors(clientFactory()); testCompressedResponseBody(clientFactory()); testMultipleClients(clientFactory); - testMultipartRequests(clientFactory()); + testMultipartRequests(clientFactory(), + supportsMultipartRequest: supportsMultipartRequest); testClose(clientFactory); testIsolate(clientFactory, canWorkInIsolates: canWorkInIsolates); testRequestCookies(clientFactory(), diff --git a/pkgs/http_client_conformance_tests/lib/src/multipart_tests.dart b/pkgs/http_client_conformance_tests/lib/src/multipart_tests.dart index 95065e519d..c9ecd90f82 100644 --- a/pkgs/http_client_conformance_tests/lib/src/multipart_tests.dart +++ b/pkgs/http_client_conformance_tests/lib/src/multipart_tests.dart @@ -11,7 +11,11 @@ import 'multipart_server_vm.dart' if (dart.library.js_interop) 'multipart_server_web.dart'; /// Tests that the [Client] correctly sends [MultipartRequest]. -void testMultipartRequests(Client client) async { +/// +/// If [supportsMultipartRequest] is `false` then tests that assume that +/// multipart requests can be sent will be skipped. +void testMultipartRequests(Client client, + {required bool supportsMultipartRequest}) async { group('multipart requests', () { late final String host; late final StreamChannel httpServerChannel; @@ -40,5 +44,8 @@ content-disposition: form-data; name="file1"\r \r Hello World''')); }); - }); + }, + skip: supportsMultipartRequest + ? false + : 'does not support multipart requests'); }