Reland fuchsia_remote_debug_protocol allows open port on remote device (#66271)

packages/flutter_driver/lib/src/common/fuchsia_compat.dart

@@ -24,6 +24,9 @@ class _DummyPortForwarder implements PortForwarder {
   @override
   int get remotePort => _remotePort;
 
+  @override
+  String get openPortAddress => InternetAddress.loopbackIPv4.address;
+
   @override
   Future<void> stop() async { }
 }

packages/fuchsia_remote_debug_protocol/lib/src/common/network.dart

@@ -17,7 +17,10 @@ void validateAddress(String address) {
 /// Returns true if `address` is a valid IPv6 address.
 bool isIpV6Address(String address) {
   try {
-    Uri.parseIPv6Address(address);
+    // parseIpv6Address fails if there's a zone ID. Since this is still a valid
+    // IP, remove any zone ID before parsing.
+    final List<String> addressParts = address.split('%');
+    Uri.parseIPv6Address(addressParts[0]);
     return true;
   } on FormatException {
     return false;

packages/fuchsia_remote_debug_protocol/lib/src/fuchsia_remote_connection.dart

@@ -103,13 +103,13 @@ class DartVmEvent {
 /// This class can be connected to several instances of the Fuchsia device's
 /// Dart VM at any given time.
 class FuchsiaRemoteConnection {
-  FuchsiaRemoteConnection._(this._useIpV6Loopback, this._sshCommandRunner)
+  FuchsiaRemoteConnection._(this._useIpV6, this._sshCommandRunner)
     : _pollDartVms = false;
 
   bool _pollDartVms;
   final List<PortForwarder> _forwardedVmServicePorts = <PortForwarder>[];
   final SshCommandRunner _sshCommandRunner;
-  final bool _useIpV6Loopback;
+  final bool _useIpV6;
 
   /// A mapping of Dart VM ports (as seen on the target machine), to
   /// [PortForwarder] instances mapping from the local machine to the target
@@ -126,15 +126,15 @@ class FuchsiaRemoteConnection {
       StreamController<DartVmEvent>();
 
   /// VM service cache to avoid repeating handshakes across function
-  /// calls. Keys a forwarded port to a DartVm connection instance.
+  /// calls. Keys a URI to a DartVm connection instance.
-  final Map<int, DartVm> _dartVmCache = <int, DartVm>{};
+  final Map<Uri, DartVm> _dartVmCache = <Uri, DartVm>{};
 
   /// Same as [FuchsiaRemoteConnection.connect] albeit with a provided
   /// [SshCommandRunner] instance.
   static Future<FuchsiaRemoteConnection> connectWithSshCommandRunner(SshCommandRunner commandRunner) async {
     final FuchsiaRemoteConnection connection = FuchsiaRemoteConnection._(
         isIpV6Address(commandRunner.address), commandRunner);
-    await connection._forwardLocalPortsToDeviceServicePorts();
+    await connection._forwardOpenPortsToDeviceServicePorts();
 
     Stream<DartVmEvent> dartVmStream() {
       Future<void> listen() async {
@@ -224,14 +224,16 @@ class FuchsiaRemoteConnection {
     for (final PortForwarder pf in _forwardedVmServicePorts) {
       // Closes VM service first to ensure that the connection is closed cleanly
       // on the target before shutting down the forwarding itself.
-      final DartVm vmService = _dartVmCache[pf.port];
+      final Uri uri = _getDartVmUri(pf);
-      _dartVmCache[pf.port] = null;
+      final DartVm vmService = _dartVmCache[uri];
+      _dartVmCache[uri] = null;
       await vmService?.stop();
       await pf.stop();
     }
     for (final PortForwarder pf in _dartVmPortMap.values) {
-      final DartVm vmService = _dartVmCache[pf.port];
+      final Uri uri = _getDartVmUri(pf);
-      _dartVmCache[pf.port] = null;
+      final DartVm vmService = _dartVmCache[uri];
+      _dartVmCache[uri] = null;
       await vmService?.stop();
       await pf.stop();
     }
@@ -258,8 +260,8 @@ class FuchsiaRemoteConnection {
         if (event.eventType == DartVmEventType.started) {
           _log.fine('New VM found on port: ${event.servicePort}. Searching '
               'for Isolate: $pattern');
-          final DartVm vmService = await _getDartVm(event.uri.port,
+          final DartVm vmService = await _getDartVm(event.uri,
-              timeout: _kDartVmConnectionTimeout);
+            timeout: _kDartVmConnectionTimeout);
           // If the VM service is null, set the result to the empty list.
           final List<IsolateRef> result = await vmService
                   ?.getMainIsolatesByPattern(pattern, timeout: timeout) ??
@@ -307,7 +309,7 @@ class FuchsiaRemoteConnection {
         <Future<List<IsolateRef>>>[];
     for (final PortForwarder fp in _dartVmPortMap.values) {
       final DartVm vmService =
-          await _getDartVm(fp.port, timeout: vmConnectionTimeout);
+      await _getDartVm(_getDartVmUri(fp), timeout: vmConnectionTimeout);
       if (vmService == null) {
         continue;
       }
@@ -385,13 +387,13 @@ class FuchsiaRemoteConnection {
         _dartVmEventController.add(DartVmEvent._(
           eventType: DartVmEventType.stopped,
           servicePort: pf.remotePort,
-          uri: _getDartVmUri(pf.port),
+          uri: _getDartVmUri(pf),
         ));
       }
     }
 
     for (final PortForwarder pf in _dartVmPortMap.values) {
-      final DartVm service = await _getDartVm(pf.port);
+      final DartVm service = await _getDartVm(_getDartVmUri(pf));
       if (service == null) {
         await shutDownPortForwarder(pf);
       } else {
@@ -402,15 +404,16 @@ class FuchsiaRemoteConnection {
     return result;
   }
 
-  Uri _getDartVmUri(int port) {
+  Uri _getDartVmUri(PortForwarder pf) {
-    // While the IPv4 loopback can be used for the initial port forwarding
+    String addr;
-    // (see [PortForwarder.start]), the address is actually bound to the IPv6
+    if (pf.openPortAddress == null) {
-    // loopback device, so connecting to the IPv4 loopback would fail when the
+      addr = _useIpV6 ? '[$_ipv6Loopback]' : _ipv4Loopback;
-    // target address is IPv6 link-local.
+    } else {
-    final String addr = _useIpV6Loopback
+      addr = isIpV6Address(pf.openPortAddress)
-        ? 'http://[$_ipv6Loopback]:$port'
+        ? '[${pf.openPortAddress}]'
-        : 'http://$_ipv4Loopback:$port';
+        : pf.openPortAddress;
-    final Uri uri = Uri.parse(addr);
+    }
+    final Uri uri = Uri.http('$addr:${pf.port}', '/');
     return uri;
   }
 
@@ -419,17 +422,16 @@ class FuchsiaRemoteConnection {
   /// Returns null if either there is an [HttpException] or a
   /// [TimeoutException], else a [DartVm] instance.
   Future<DartVm> _getDartVm(
-    int port, {
+    Uri uri, {
     Duration timeout = _kDartVmConnectionTimeout,
   }) async {
-    if (!_dartVmCache.containsKey(port)) {
+    if (!_dartVmCache.containsKey(uri)) {
       // When raising an HttpException this means that there is no instance of
       // the Dart VM to communicate with.  The TimeoutException is raised when
       // the Dart VM instance is shut down in the middle of communicating.
       try {
-        final DartVm dartVm =
+        final DartVm dartVm = await DartVm.connect(uri, timeout: timeout);
-            await DartVm.connect(_getDartVmUri(port), timeout: timeout);
+        _dartVmCache[uri] = dartVm;
-        _dartVmCache[port] = dartVm;
       } on HttpException {
         _log.warning('HTTP Exception encountered connecting to new VM');
         return null;
@@ -438,7 +440,7 @@ class FuchsiaRemoteConnection {
         return null;
       }
     }
-    return _dartVmCache[port];
+    return _dartVmCache[uri];
   }
 
   /// Checks for changes in the list of Dart VM instances.
@@ -460,7 +462,7 @@ class FuchsiaRemoteConnection {
         _dartVmEventController.add(DartVmEvent._(
           eventType: DartVmEventType.started,
           servicePort: servicePort,
-          uri: _getDartVmUri(_dartVmPortMap[servicePort].port),
+          uri: _getDartVmUri(_dartVmPortMap[servicePort]),
         ));
       }
     }
@@ -482,11 +484,11 @@ class FuchsiaRemoteConnection {
     );
   }
 
-  /// Forwards a series of local device ports to the remote device.
+  /// Forwards a series of open ports to the remote device.
   ///
   /// When this function is run, all existing forwarded ports and connections
   /// are reset by way of [stop].
-  Future<void> _forwardLocalPortsToDeviceServicePorts() async {
+  Future<void> _forwardOpenPortsToDeviceServicePorts() async {
     await stop();
     final List<int> servicePorts = await getDeviceServicePorts();
     final List<PortForwarder> forwardedVmServicePorts =
@@ -548,9 +550,13 @@ class FuchsiaRemoteConnection {
 ///
 /// To shut down a port forwarder you must call the [stop] function.
 abstract class PortForwarder {
-  /// Determines the port which is being forwarded from the local machine.
+  /// Determines the port which is being forwarded.
   int get port;
 
+  /// The address on which the open port is accessible. Defaults to null to
+  /// indicate local loopback.
+  String get openPortAddress => null;
+
   /// The destination port on the other end of the port forwarding tunnel.
   int get remotePort;
 
@@ -581,6 +587,9 @@ class _SshPortForwarder implements PortForwarder {
   @override
   int get port => _localSocket.port;
 
+  @override
+  String get openPortAddress => _ipV6 ? _ipv6Loopback : _ipv4Loopback;
+
   @override
   int get remotePort => _remotePort;
 
@@ -602,8 +611,9 @@ class _SshPortForwarder implements PortForwarder {
     // TODO(awdavies): The square-bracket enclosure for using the IPv6 loopback
     // didn't appear to work, but when assigning to the IPv4 loopback device,
     // netstat shows that the local port is actually being used on the IPv6
-    // loopback (::1). While this can be used for forwarding to the destination
+    // loopback (::1). Therefore, while the IPv4 loopback can be used for
-    // IPv6 interface, it cannot be used to connect to a websocket.
+    // forwarding to the destination IPv6 interface, when connecting to the
+    // websocket, the IPV6 loopback should be used.
     final String formattedForwardingUrl =
         '${localSocket.port}:$_ipv4Loopback:$remotePort';
     final String targetAddress =

packages/fuchsia_remote_debug_protocol/test/fuchsia_remote_connection_test.dart

@@ -11,41 +11,11 @@ import 'common.dart';
 
 void main() {
   group('FuchsiaRemoteConnection.connect', () {
-    MockSshCommandRunner mockRunner;
     List<MockPortForwarder> forwardedPorts;
     List<MockPeer> mockPeerConnections;
     List<Uri> uriConnections;
 
     setUp(() {
-      mockRunner = MockSshCommandRunner();
-      // Adds some extra junk to make sure the strings will be cleaned up.
-      when(mockRunner.run(argThat(startsWith('/bin/find')))).thenAnswer(
-          (_) => Future<List<String>>.value(
-              <String>['/hub/blah/blah/blah/vmservice-port\n']));
-      when(mockRunner.run(argThat(startsWith('/bin/ls')))).thenAnswer(
-          (_) => Future<List<String>>.value(
-              <String>['123\n\n\n', '456  ', '789']));
-      const String address = 'fe80::8eae:4cff:fef4:9247';
-      const String interface = 'eno1';
-      when(mockRunner.address).thenReturn(address);
-      when(mockRunner.interface).thenReturn(interface);
-      forwardedPorts = <MockPortForwarder>[];
-      int port = 0;
-      Future<PortForwarder> mockPortForwardingFunction(
-        String address,
-        int remotePort, [
-        String interface = '',
-        String configFile,
-      ]) {
-        return Future<PortForwarder>(() {
-          final MockPortForwarder pf = MockPortForwarder();
-          forwardedPorts.add(pf);
-          when(pf.port).thenReturn(port++);
-          when(pf.remotePort).thenReturn(remotePort);
-          return pf;
-        });
-      }
-
       final List<Map<String, dynamic>> flutterViewCannedResponses =
           <Map<String, dynamic>>[
         <String, dynamic>{
@@ -88,6 +58,7 @@ void main() {
         },
       ];
 
+      forwardedPorts = <MockPortForwarder>[];
       mockPeerConnections = <MockPeer>[];
       uriConnections = <Uri>[];
       Future<json_rpc.Peer> mockVmConnectionFunction(
@@ -107,7 +78,6 @@ void main() {
         });
       }
 
-      fuchsiaPortForwardingFunction = mockPortForwardingFunction;
       fuchsiaVmServiceConnectionFunction = mockVmConnectionFunction;
     });
 
@@ -119,6 +89,34 @@ void main() {
     });
 
     test('end-to-end with three vm connections and flutter view query', () async {
+      int port = 0;
+      Future<PortForwarder> mockPortForwardingFunction(
+        String address,
+        int remotePort, [
+        String interface = '',
+        String configFile,
+      ]) {
+        return Future<PortForwarder>(() {
+          final MockPortForwarder pf = MockPortForwarder();
+          forwardedPorts.add(pf);
+          when(pf.port).thenReturn(port++);
+          when(pf.remotePort).thenReturn(remotePort);
+          return pf;
+        });
+      }
+
+      fuchsiaPortForwardingFunction = mockPortForwardingFunction;
+      final MockSshCommandRunner mockRunner = MockSshCommandRunner();
+      // Adds some extra junk to make sure the strings will be cleaned up.
+      when(mockRunner.run(argThat(startsWith('/bin/find')))).thenAnswer(
+          (_) => Future<List<String>>.value(
+              <String>['/hub/blah/blah/blah/vmservice-port\n']));
+      when(mockRunner.run(argThat(startsWith('/bin/ls')))).thenAnswer(
+          (_) => Future<List<String>>.value(
+              <String>['123\n\n\n', '456  ', '789']));
+      when(mockRunner.address).thenReturn('fe80::8eae:4cff:fef4:9247');
+      when(mockRunner.interface).thenReturn('eno1');
+
       final FuchsiaRemoteConnection connection =
           await FuchsiaRemoteConnection.connectWithSshCommandRunner(mockRunner);
 
@@ -133,6 +131,155 @@ void main() {
       expect(forwardedPorts[1].port, 1);
       expect(forwardedPorts[2].port, 2);
 
+      // VMs should be accessed via localhost ports given by
+      // [mockPortForwardingFunction].
+      expect(uriConnections[0],
+        Uri(scheme:'ws', host:'[::1]', port:0, path:'/ws'));
+      expect(uriConnections[1],
+        Uri(scheme:'ws', host:'[::1]', port:1, path:'/ws'));
+      expect(uriConnections[2],
+        Uri(scheme:'ws', host:'[::1]', port:2, path:'/ws'));
+
+      final List<FlutterView> views = await connection.getFlutterViews();
+      expect(views, isNot(null));
+      expect(views.length, 3);
+      // Since name can be null, check for the ID on all of them.
+      expect(views[0].id, 'flutterView0');
+      expect(views[1].id, 'flutterView1');
+      expect(views[2].id, 'flutterView2');
+
+      expect(views[0].name, equals(null));
+      expect(views[1].name, 'file://flutterBinary1');
+      expect(views[2].name, 'file://flutterBinary2');
+
+      // Ensure the ports are all closed after stop was called.
+      await connection.stop();
+      verify(forwardedPorts[0].stop());
+      verify(forwardedPorts[1].stop());
+      verify(forwardedPorts[2].stop());
+    });
+
+    test('end-to-end with three vms and remote open port', () async {
+      int port = 0;
+      Future<PortForwarder> mockPortForwardingFunction(
+        String address,
+        int remotePort, [
+        String interface = '',
+        String configFile,
+      ]) {
+        return Future<PortForwarder>(() {
+          final MockPortForwarder pf = MockPortForwarder();
+          forwardedPorts.add(pf);
+          when(pf.port).thenReturn(port++);
+          when(pf.remotePort).thenReturn(remotePort);
+          when(pf.openPortAddress).thenReturn('fe80::1:2%eno2');
+          return pf;
+        });
+      }
+
+      fuchsiaPortForwardingFunction = mockPortForwardingFunction;
+      final MockSshCommandRunner mockRunner = MockSshCommandRunner();
+      // Adds some extra junk to make sure the strings will be cleaned up.
+      when(mockRunner.run(argThat(startsWith('/bin/find')))).thenAnswer(
+          (_) => Future<List<String>>.value(
+              <String>['/hub/blah/blah/blah/vmservice-port\n']));
+      when(mockRunner.run(argThat(startsWith('/bin/ls')))).thenAnswer(
+          (_) => Future<List<String>>.value(
+              <String>['123\n\n\n', '456  ', '789']));
+      when(mockRunner.address).thenReturn('fe80::8eae:4cff:fef4:9247');
+      when(mockRunner.interface).thenReturn('eno1');
+      final FuchsiaRemoteConnection connection =
+          await FuchsiaRemoteConnection.connectWithSshCommandRunner(mockRunner);
+
+      // [mockPortForwardingFunction] will have returned three different
+      // forwarded ports, incrementing the port each time by one. (Just a sanity
+      // check that the forwarding port was called).
+      expect(forwardedPorts.length, 3);
+      expect(forwardedPorts[0].remotePort, 123);
+      expect(forwardedPorts[1].remotePort, 456);
+      expect(forwardedPorts[2].remotePort, 789);
+      expect(forwardedPorts[0].port, 0);
+      expect(forwardedPorts[1].port, 1);
+      expect(forwardedPorts[2].port, 2);
+
+      // VMs should be accessed via the alternate adddress given by
+      // [mockPortForwardingFunction].
+      expect(uriConnections[0],
+        Uri(scheme:'ws', host:'[fe80::1:2%25eno2]', port:0, path:'/ws'));
+      expect(uriConnections[1],
+        Uri(scheme:'ws', host:'[fe80::1:2%25eno2]', port:1, path:'/ws'));
+      expect(uriConnections[2],
+        Uri(scheme:'ws', host:'[fe80::1:2%25eno2]', port:2, path:'/ws'));
+
+      final List<FlutterView> views = await connection.getFlutterViews();
+      expect(views, isNot(null));
+      expect(views.length, 3);
+      // Since name can be null, check for the ID on all of them.
+      expect(views[0].id, 'flutterView0');
+      expect(views[1].id, 'flutterView1');
+      expect(views[2].id, 'flutterView2');
+
+      expect(views[0].name, equals(null));
+      expect(views[1].name, 'file://flutterBinary1');
+      expect(views[2].name, 'file://flutterBinary2');
+
+      // Ensure the ports are all closed after stop was called.
+      await connection.stop();
+      verify(forwardedPorts[0].stop());
+      verify(forwardedPorts[1].stop());
+      verify(forwardedPorts[2].stop());
+    });
+
+    test('end-to-end with three vms and ipv4', () async {
+      int port = 0;
+      Future<PortForwarder> mockPortForwardingFunction(
+        String address,
+        int remotePort, [
+        String interface = '',
+        String configFile,
+      ]) {
+        return Future<PortForwarder>(() {
+          final MockPortForwarder pf = MockPortForwarder();
+          forwardedPorts.add(pf);
+          when(pf.port).thenReturn(port++);
+          when(pf.remotePort).thenReturn(remotePort);
+          return pf;
+        });
+      }
+
+      fuchsiaPortForwardingFunction = mockPortForwardingFunction;
+      final MockSshCommandRunner mockRunner = MockSshCommandRunner();
+      // Adds some extra junk to make sure the strings will be cleaned up.
+      when(mockRunner.run(argThat(startsWith('/bin/find')))).thenAnswer(
+          (_) => Future<List<String>>.value(
+              <String>['/hub/blah/blah/blah/vmservice-port\n']));
+      when(mockRunner.run(argThat(startsWith('/bin/ls')))).thenAnswer(
+          (_) => Future<List<String>>.value(
+              <String>['123\n\n\n', '456  ', '789']));
+      when(mockRunner.address).thenReturn('196.168.1.4');
+
+      final FuchsiaRemoteConnection connection =
+          await FuchsiaRemoteConnection.connectWithSshCommandRunner(mockRunner);
+
+      // [mockPortForwardingFunction] will have returned three different
+      // forwarded ports, incrementing the port each time by one. (Just a sanity
+      // check that the forwarding port was called).
+      expect(forwardedPorts.length, 3);
+      expect(forwardedPorts[0].remotePort, 123);
+      expect(forwardedPorts[1].remotePort, 456);
+      expect(forwardedPorts[2].remotePort, 789);
+      expect(forwardedPorts[0].port, 0);
+      expect(forwardedPorts[1].port, 1);
+      expect(forwardedPorts[2].port, 2);
+
+      // VMs should be accessed via the ipv4 loopback.
+      expect(uriConnections[0],
+        Uri(scheme:'ws', host:'127.0.0.1', port:0, path:'/ws'));
+      expect(uriConnections[1],
+        Uri(scheme:'ws', host:'127.0.0.1', port:1, path:'/ws'));
+      expect(uriConnections[2],
+        Uri(scheme:'ws', host:'127.0.0.1', port:2, path:'/ws'));
+
       final List<FlutterView> views = await connection.getFlutterViews();
       expect(views, isNot(null));
       expect(views.length, 3);

packages/fuchsia_remote_debug_protocol/test/src/common/network_test.dart

@@ -0,0 +1,24 @@
+// Copyright 2014 The Flutter Authors. 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:fuchsia_remote_debug_protocol/src/common/network.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+void main() {
+  final List<String> ipv4Addresses = <String>['127.0.0.1', '8.8.8.8'];
+  final List<String> ipv6Addresses = <String>['::1',
+    'fe80::8eae:4cff:fef4:9247', 'fe80::8eae:4cff:fef4:9247%e0'];
+
+  group('test validation', () {
+    test('isIpV4Address', () {
+      expect(ipv4Addresses.map(isIpV4Address), everyElement(isTrue));
+      expect(ipv6Addresses.map(isIpV4Address), everyElement(isFalse));
+    });
+
+    test('isIpV6Address', () {
+      expect(ipv4Addresses.map(isIpV6Address), everyElement(isFalse));
+      expect(ipv6Addresses.map(isIpV6Address), everyElement(isTrue));
+    });
+  });
+}