Fix a bug where cronet_http sends incorrect HTTP request methods (#1058)

Author: brianquinlan

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
 

pkgs/cronet_http/lib/src/cronet_client.dart

@@ -317,7 +317,7 @@ class CronetClient extends BaseClient {
       jb.UrlRequestCallbackProxy.new1(
           _urlRequestCallbacks(request, responseCompleter)),
       _executor,
-    );
+    )..setHttpMethod(request.method.toJString());
 
     var headers = request.headers;
     if (body.isNotEmpty &&

pkgs/http/test/io/client_conformance_test.dart

@@ -10,5 +10,6 @@ import 'package:http_client_conformance_tests/http_client_conformance_tests.dart
 import 'package:test/test.dart';
 
 void main() {
-  testAll(IOClient.new);
+  testAll(IOClient.new, preservesMethodCase: false // https://dartbug.com/54187
+      );
 }

pkgs/http_client_conformance_tests/lib/http_client_conformance_tests.dart

@@ -12,6 +12,7 @@ import 'src/redirect_tests.dart';
 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/redirect_tests.dart' show testRedirect;
 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 @@ export 'src/server_errors_test.dart' show testServerErrors;
 /// 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 @@ void testAll(Client Function() clientFactory,
   testResponseBodyStreamed(clientFactory(),
       canStreamResponseBody: canStreamResponseBody);
   testRequestHeaders(clientFactory());
+  testRequestMethods(clientFactory(), preservesMethodCase: preservesMethodCase);
   testResponseHeaders(clientFactory());
   testResponseStatusLine(clientFactory());
   testRedirect(clientFactory(), redirectAlwaysAllowed: redirectAlwaysAllowed);

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());
+}

pkgs/http_client_conformance_tests/lib/src/request_methods_server_vm.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);
+    });
+  });
+}