From 27a235976a554201b717ecc65cb769bcae986010 Mon Sep 17 00:00:00 2001 From: Vyacheslav Egorov Date: Thu, 8 Sep 2022 14:41:34 +0200 Subject: [PATCH] Change default authority for UDS connections. (#577) We were using path to the UDS socket itself, which is incorrect `:authority` value. This was tripping checks in some HTTP2 protocol implementations. Instead default `:authority` to `localhost`, which in line with other gRPC implementations. Fixes #576 --- CHANGELOG.md | 4 ++++ lib/src/client/http2_connection.dart | 21 +++++++++++++++++---- test/common.dart | 25 +++++++++++++++---------- test/round_trip_test.dart | 28 +++++++++++++++++++++++++++- 4 files changed, 63 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d642cde..8878c5e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ * Expose a stream for connection state changes on ClientChannel to address [#428](https://github.com/grpc/grpc-dart/issues/428). This allows users to react to state changes in the connection. +* Fix [#576](https://github.com/grpc/grpc-dart/issues/576): set default + `:authority` value for UDS connections to `localhost` instead of using + UDS path. Using path triggers checks in HTTP2 servers which + attempt to validate `:authority` value. ## 3.0.2 diff --git a/lib/src/client/http2_connection.dart b/lib/src/client/http2_connection.dart index 2bc70268..5fc6d422 100644 --- a/lib/src/client/http2_connection.dart +++ b/lib/src/client/http2_connection.dart @@ -355,10 +355,23 @@ class _SocketTransportConnector implements ClientTransportConnector { @override String get authority { - final host = - _host is String ? _host as String : (_host as InternetAddress).host; - return _options.credentials.authority ?? - (_port == 443 ? host : '$host:$_port'); + return _options.credentials.authority ?? _makeAuthority(); + } + + String _makeAuthority() { + final host = _host; + final portSuffix = _port == 443 ? '' : ':$_port'; + final hostName; + if (host is String) { + hostName = '$host'; + } else { + host as InternetAddress; + if (host.type == InternetAddressType.unix) { + return 'localhost'; // UDS don't have a meaningful authority. + } + hostName = host.host; + } + return '$hostName$portSuffix'; } @override diff --git a/test/common.dart b/test/common.dart index 935097c1..bc7ba71a 100644 --- a/test/common.dart +++ b/test/common.dart @@ -16,20 +16,13 @@ import 'dart:async'; import 'dart:io'; import 'package:test/test.dart'; -/// Test functionality for both TCP and Unix domain sockets. -void testTcpAndUds( - String name, FutureOr Function(InternetAddress) testCase, - {String host = 'localhost'}) { - test(name, () async { - final address = await InternetAddress.lookup(host); - await testCase(address.first); - }); - +/// Test functionality for Unix domain socket. +void testUds(String name, FutureOr Function(InternetAddress) testCase) { if (Platform.isWindows) { return; } - test('$name (over uds)', () async { + test(name, () async { final tempDir = await Directory.systemTemp.createTemp(); final address = InternetAddress(tempDir.path + '/socket', type: InternetAddressType.unix); @@ -37,3 +30,15 @@ void testTcpAndUds( await testCase(address); }); } + +/// Test functionality for both TCP and Unix domain sockets. +void testTcpAndUds( + String name, FutureOr Function(InternetAddress) testCase, + {String host = 'localhost'}) { + test(name, () async { + final address = await InternetAddress.lookup(host); + await testCase(address.first); + }); + + testUds('$name (over uds)', testCase); +} diff --git a/test/round_trip_test.dart b/test/round_trip_test.dart index 2ba7c43b..90837a85 100644 --- a/test/round_trip_test.dart +++ b/test/round_trip_test.dart @@ -23,10 +23,12 @@ class TestClient extends Client { } class TestService extends Service { + final String? expectedAuthority; + @override String get $name => 'test.TestService'; - TestService() { + TestService({this.expectedAuthority}) { $addMethod(ServiceMethod('stream', stream, false, true, (List value) => value[0], (int value) => [value])); } @@ -35,12 +37,20 @@ class TestService extends Service { static const requestInfiniteStream = 2; Stream stream(ServiceCall call, Future request) async* { + checkMetadata(call.clientMetadata); + final isInfinite = 2 == await request; for (var i = 1; i <= 3 || isInfinite; i++) { yield i; await Future.delayed(Duration(milliseconds: 100)); } } + + void checkMetadata(Map? metadata) { + if (expectedAuthority != null) { + expect(metadata![':authority'], equals(expectedAuthority)); + } + } } class TestServiceWithOnMetadataException extends TestService { @@ -93,6 +103,22 @@ Future main() async { server.shutdown(); }); + testUds('UDS provides valid default authority', (address) async { + // round trip test of insecure connection. + final server = Server([TestService(expectedAuthority: 'localhost')]); + await server.serve(address: address, port: 0); + + final channel = FixedConnectionClientChannel(Http2ClientConnection( + address, + server.port!, + ChannelOptions(credentials: ChannelCredentials.insecure()), + )); + final testClient = TestClient(channel); + expect(await testClient.stream(TestService.requestFiniteStream).toList(), + [1, 2, 3]); + server.shutdown(); + }); + testTcpAndUds('round trip with outgoing and incoming compression', (address) async { final server = Server(