-
Notifications
You must be signed in to change notification settings - Fork 382
Fix a bug where cronet_http sends incorrect HTTP request methods #1058
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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()); | ||
} |
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.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
// 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). | ||
void testRequestMethods(Client client) 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', () async { | ||
await client.send(Request( | ||
'CUSTOM', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we test case-sensitive method too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is that IOClient would not pass that test because it always uppercases the request method: This seems like a bug according to RFC-7232: https://www.rfc-editor.org/rfc/rfc7231#section-4.1 I will add a case-insensitive test and then a flag to disable it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh well, another bug, good spot. 👍 |
||
Uri.http(host, ''), | ||
)); | ||
final method = await httpServerQueue.next as String; | ||
expect('CUSTOM', method); | ||
}); | ||
|
||
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); | ||
}); | ||
}); | ||
} |
Uh oh!
There was an error while loading. Please reload this page.