Fix a bug where cronet_http sends incorrect HTTP request methods (#1058)
diff --git a/pkgs/cronet_http/CHANGELOG.md b/pkgs/cronet_http/CHANGELOG.md
index 7eecd29..59ef74f 100644
--- a/pkgs/cronet_http/CHANGELOG.md
+++ b/pkgs/cronet_http/CHANGELOG.md
@@ -1,8 +1,7 @@
-## 0.4.3-wip
-
## 0.4.2
* Require `package:jni >= 0.7.2` to remove a potential buffer overflow.
+* Fix a bug where incorrect HTTP request methods were sent.
## 0.4.1
diff --git a/pkgs/cronet_http/lib/src/cronet_client.dart b/pkgs/cronet_http/lib/src/cronet_client.dart
index c4d0d19..272b602 100644
--- a/pkgs/cronet_http/lib/src/cronet_client.dart
+++ b/pkgs/cronet_http/lib/src/cronet_client.dart
@@ -317,7 +317,7 @@
jb.UrlRequestCallbackProxy.new1(
_urlRequestCallbacks(request, responseCompleter)),
_executor,
- );
+ )..setHttpMethod(request.method.toJString());
var headers = request.headers;
if (body.isNotEmpty &&
diff --git a/pkgs/http/test/io/client_conformance_test.dart b/pkgs/http/test/io/client_conformance_test.dart
index 5d8f7f5..20bf39f 100644
--- a/pkgs/http/test/io/client_conformance_test.dart
+++ b/pkgs/http/test/io/client_conformance_test.dart
@@ -10,5 +10,6 @@
import 'package:test/test.dart';
void main() {
- testAll(IOClient.new);
+ testAll(IOClient.new, preservesMethodCase: false // https://dartbug.com/54187
+ );
}
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 3d00319..bd83c02 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
@@ -12,6 +12,7 @@
import 'src/request_body_streamed_tests.dart';
import 'src/request_body_tests.dart';
import 'src/request_headers_tests.dart';
+import 'src/request_methods_tests.dart';
import 'src/response_body_streamed_test.dart';
import 'src/response_body_tests.dart';
import 'src/response_headers_tests.dart';
@@ -27,6 +28,7 @@
export 'src/request_body_streamed_tests.dart' show testRequestBodyStreamed;
export 'src/request_body_tests.dart' show testRequestBody;
export 'src/request_headers_tests.dart' show testRequestHeaders;
+export 'src/request_methods_tests.dart' show testRequestMethods;
export 'src/response_body_streamed_test.dart' show testResponseBodyStreamed;
export 'src/response_body_tests.dart' show testResponseBody;
export 'src/response_headers_tests.dart' show testResponseHeaders;
@@ -49,14 +51,20 @@
/// If [canWorkInIsolates] is `false` then tests that require that the [Client]
/// work in Isolates other than the main isolate will be skipped.
///
+/// If [preservesMethodCase] is `false` then tests that assume that the
+/// [Client] preserves custom request method casing 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.
-void testAll(Client Function() clientFactory,
- {bool canStreamRequestBody = true,
- bool canStreamResponseBody = true,
- bool redirectAlwaysAllowed = false,
- bool canWorkInIsolates = true}) {
+void testAll(
+ Client Function() clientFactory, {
+ bool canStreamRequestBody = true,
+ bool canStreamResponseBody = true,
+ bool redirectAlwaysAllowed = false,
+ bool canWorkInIsolates = true,
+ bool preservesMethodCase = false,
+}) {
testRequestBody(clientFactory());
testRequestBodyStreamed(clientFactory(),
canStreamRequestBody: canStreamRequestBody);
@@ -65,6 +73,7 @@
testResponseBodyStreamed(clientFactory(),
canStreamResponseBody: canStreamResponseBody);
testRequestHeaders(clientFactory());
+ testRequestMethods(clientFactory(), preservesMethodCase: preservesMethodCase);
testResponseHeaders(clientFactory());
testResponseStatusLine(clientFactory());
testRedirect(clientFactory(), redirectAlwaysAllowed: redirectAlwaysAllowed);
diff --git a/pkgs/http_client_conformance_tests/lib/src/request_methods_server.dart b/pkgs/http_client_conformance_tests/lib/src/request_methods_server.dart
new file mode 100644
index 0000000..bf05ec0
--- /dev/null
+++ b/pkgs/http_client_conformance_tests/lib/src/request_methods_server.dart
@@ -0,0 +1,41 @@
+// Copyright (c) 2023, 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:io';
+
+import 'package:stream_channel/stream_channel.dart';
+
+/// Starts an HTTP server that captures the request headers.
+///
+/// Channel protocol:
+/// On Startup:
+/// - send port
+/// On Request Received:
+/// - send the received request method (e.g. GET) as a String
+/// 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 {
+ channel.sink.add(request.method);
+ }
+ 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/request_methods_server_vm.dart b/pkgs/http_client_conformance_tests/lib/src/request_methods_server_vm.dart
new file mode 100644
index 0000000..6ce6627
--- /dev/null
+++ b/pkgs/http_client_conformance_tests/lib/src/request_methods_server_vm.dart
@@ -0,0 +1,12 @@
+// Generated by generate_server_wrappers.dart. Do not edit.
+
+import 'package:stream_channel/stream_channel.dart';
+
+import 'request_methods_server.dart';
+
+/// Starts the redirect test HTTP server in the same process.
+Future<StreamChannel<Object?>> startServer() async {
+ final controller = StreamChannelController<Object?>(sync: true);
+ hybridMain(controller.foreign);
+ return controller.local;
+}
diff --git a/pkgs/http_client_conformance_tests/lib/src/request_methods_server_web.dart b/pkgs/http_client_conformance_tests/lib/src/request_methods_server_web.dart
new file mode 100644
index 0000000..8cddf5a
--- /dev/null
+++ b/pkgs/http_client_conformance_tests/lib/src/request_methods_server_web.dart
@@ -0,0 +1,9 @@
+// Generated by generate_server_wrappers.dart. Do not edit.
+
+import 'package:stream_channel/stream_channel.dart';
+import 'package:test/test.dart';
+
+/// Starts the redirect test HTTP server out-of-process.
+Future<StreamChannel<Object?>> startServer() async => spawnHybridUri(Uri(
+ scheme: 'package',
+ path: 'http_client_conformance_tests/src/request_methods_server.dart'));
diff --git a/pkgs/http_client_conformance_tests/lib/src/request_methods_tests.dart b/pkgs/http_client_conformance_tests/lib/src/request_methods_tests.dart
new file mode 100644
index 0000000..ec11387
--- /dev/null
+++ b/pkgs/http_client_conformance_tests/lib/src/request_methods_tests.dart
@@ -0,0 +1,88 @@
+// Copyright (c) 2023, 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 'request_methods_server_vm.dart'
+ if (dart.library.html) 'request_methods_server_web.dart';
+
+/// Tests that the [Client] correctly sends HTTP request methods
+/// (e.g. GET, HEAD).
+///
+/// If [preservesMethodCase] is `false` then tests that assume that the
+/// [Client] preserves custom request method casing will be skipped.
+void testRequestMethods(Client client,
+ {bool preservesMethodCase = true}) async {
+ group('request methods', () {
+ 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.next}';
+ });
+ tearDownAll(() => httpServerChannel.sink.add(null));
+
+ test('custom method - not case preserving', () async {
+ await client.send(Request(
+ 'CuStOm',
+ Uri.http(host, ''),
+ ));
+ final method = await httpServerQueue.next as String;
+ expect('CUSTOM', method.toUpperCase());
+ });
+
+ test('custom method case preserving', () async {
+ await client.send(Request(
+ 'CuStOm',
+ Uri.http(host, ''),
+ ));
+ final method = await httpServerQueue.next as String;
+ expect('CuStOm', method);
+ },
+ skip: preservesMethodCase
+ ? false
+ : 'does not preserve HTTP request method case');
+
+ test('delete', () async {
+ await client.delete(Uri.http(host, ''));
+ final method = await httpServerQueue.next as String;
+ expect('DELETE', method);
+ });
+
+ test('get', () async {
+ await client.get(Uri.http(host, ''));
+ final method = await httpServerQueue.next as String;
+ expect('GET', method);
+ });
+ test('head', () async {
+ await client.head(Uri.http(host, ''));
+ final method = await httpServerQueue.next as String;
+ expect('HEAD', method);
+ });
+
+ test('patch', () async {
+ await client.patch(Uri.http(host, ''));
+ final method = await httpServerQueue.next as String;
+ expect('PATCH', method);
+ });
+
+ test('post', () async {
+ await client.post(Uri.http(host, ''));
+ final method = await httpServerQueue.next as String;
+ expect('POST', method);
+ });
+
+ test('put', () async {
+ await client.put(Uri.http(host, ''));
+ final method = await httpServerQueue.next as String;
+ expect('PUT', method);
+ });
+ });
+}
