Skip to content

Commit

Permalink
[cupertino_http] Fix a bug where content-length was not set for multi…
Browse files Browse the repository at this point in the history
…part messages (#1247)
  • Loading branch information
brianquinlan committed Jun 26, 2024
1 parent 0bbd166 commit eb189e1
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 6 deletions.
2 changes: 2 additions & 0 deletions pkgs/cupertino_http/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
12 changes: 7 additions & 5 deletions pkgs/cupertino_http/lib/src/cupertino_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion pkgs/http/test/html/client_conformance_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ void main() {
redirectAlwaysAllowed: true,
canStreamRequestBody: false,
canStreamResponseBody: false,
canWorkInIsolates: false);
canWorkInIsolates: false,
supportsMultipartRequest: false);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand Down Expand Up @@ -67,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.
Expand All @@ -80,6 +85,7 @@ void testAll(
bool supportsFoldedHeaders = true,
bool canSendCookieHeaders = false,
bool canReceiveSetCookieHeaders = false,
bool supportsMultipartRequest = true,
}) {
testRequestBody(clientFactory());
testRequestBodyStreamed(clientFactory(),
Expand All @@ -97,6 +103,8 @@ void testAll(
testServerErrors(clientFactory());
testCompressedResponseBody(clientFactory());
testMultipleClients(clientFactory);
testMultipartRequests(clientFactory(),
supportsMultipartRequest: supportsMultipartRequest);
testClose(clientFactory);
testIsolate(clientFactory, canWorkInIsolates: canWorkInIsolates);
testRequestCookies(clientFactory(),
Expand Down
48 changes: 48 additions & 0 deletions pkgs/http_client_conformance_tests/lib/src/multipart_server.dart
Original file line number Diff line number Diff line change
@@ -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<Object?> 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 = <String, List<String>>{};
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());
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

51 changes: 51 additions & 0 deletions pkgs/http_client_conformance_tests/lib/src/multipart_tests.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// 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].
///
/// 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<Object?> httpServerChannel;
late final StreamQueue<Object?> 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, List<String>>, 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'''));
});
},
skip: supportsMultipartRequest
? false
: 'does not support multipart requests');
}

0 comments on commit eb189e1

Please sign in to comment.