diff --git a/pkgs/cronet_http/CHANGELOG.md b/pkgs/cronet_http/CHANGELOG.md index 7eecd298e0..59ef74fe62 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 c4d0d19006..272b602b3d 100644 --- a/pkgs/cronet_http/lib/src/cronet_client.dart +++ b/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 && diff --git a/pkgs/http/test/io/client_conformance_test.dart b/pkgs/http/test/io/client_conformance_test.dart index 5d8f7f598d..20bf39f281 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: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 + ); } 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 3d003193e9..bd83c02abb 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/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); 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 0000000000..bf05ec08e2 --- /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 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 0000000000..6ce6627f4c --- /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> startServer() async { + final controller = StreamChannelController(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 0000000000..8cddf5a175 --- /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> 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 0000000000..ec11387f3d --- /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 httpServerChannel; + late final StreamQueue 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); + }); + }); +}