Skip to content

Commit

Permalink
Change default authority for UDS connections. (#577)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mraleph authored Sep 8, 2022
1 parent b8f872a commit 27a2359
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 15 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
21 changes: 17 additions & 4 deletions lib/src/client/http2_connection.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 15 additions & 10 deletions test/common.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,29 @@ 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<void> 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<void> 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);
addTearDown(() => tempDir.delete(recursive: true));
await testCase(address);
});
}

/// Test functionality for both TCP and Unix domain sockets.
void testTcpAndUds(
String name, FutureOr<void> Function(InternetAddress) testCase,
{String host = 'localhost'}) {
test(name, () async {
final address = await InternetAddress.lookup(host);
await testCase(address.first);
});

testUds('$name (over uds)', testCase);
}
28 changes: 27 additions & 1 deletion test/round_trip_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<int, int>('stream', stream, false, true,
(List<int> value) => value[0], (int value) => [value]));
}
Expand All @@ -35,12 +37,20 @@ class TestService extends Service {
static const requestInfiniteStream = 2;

Stream<int> 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<String, String>? metadata) {
if (expectedAuthority != null) {
expect(metadata![':authority'], equals(expectedAuthority));
}
}
}

class TestServiceWithOnMetadataException extends TestService {
Expand Down Expand Up @@ -93,6 +103,22 @@ Future<void> 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(
Expand Down

0 comments on commit 27a2359

Please sign in to comment.