Revert "fuchsia_remote_debug_protocol allows open port on remote device (#63996)" (#65704)

This reverts commit 33f79950.

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

@@ -25,9 +25,6 @@ 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

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:io';
+import 'dart:core';
 
 /// Determines whether `address` is a valid IPv6 or IPv4 address.
 ///
@@ -17,7 +17,7 @@ void validateAddress(String address) {
 /// Returns true if `address` is a valid IPv6 address.
 bool isIpV6Address(String address) {
   try {
-    InternetAddress(address, type:InternetAddressType.IPv6);
+    Uri.parseIPv6Address(address);
     return true;
   } on FormatException {
     return false;
@@ -27,7 +27,7 @@ bool isIpV6Address(String address) {
 /// Returns true if `address` is a valid IPv4 address.
 bool isIpV4Address(String address) {
   try {
-    InternetAddress(address, type:InternetAddressType.IPv4);
+    Uri.parseIPv4Address(address);
     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._useIpV6, this._sshCommandRunner)
+  FuchsiaRemoteConnection._(this._useIpV6Loopback, this._sshCommandRunner)
     : _pollDartVms = false;
 
   bool _pollDartVms;
   final List<PortForwarder> _forwardedVmServicePorts = <PortForwarder>[];
   final SshCommandRunner _sshCommandRunner;
-  final bool _useIpV6;
+  final bool _useIpV6Loopback;
 
   /// 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 URI to a DartVm connection instance.
-  final Map<Uri, DartVm> _dartVmCache = <Uri, DartVm>{};
+  /// calls. Keys a forwarded port to a DartVm connection instance.
+  final Map<int, DartVm> _dartVmCache = <int, 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._forwardOpenPortsToDeviceServicePorts();
+    await connection._forwardLocalPortsToDeviceServicePorts();
 
     Stream<DartVmEvent> dartVmStream() {
       Future<void> listen() async {
@@ -224,16 +224,14 @@ 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 Uri uri = _getDartVmUri(pf);
-      final DartVm vmService = _dartVmCache[uri];
-      _dartVmCache[uri] = null;
+      final DartVm vmService = _dartVmCache[pf.port];
+      _dartVmCache[pf.port] = null;
       await vmService?.stop();
       await pf.stop();
     }
     for (final PortForwarder pf in _dartVmPortMap.values) {
-      final Uri uri = _getDartVmUri(pf);
-      final DartVm vmService = _dartVmCache[uri];
-      _dartVmCache[uri] = null;
+      final DartVm vmService = _dartVmCache[pf.port];
+      _dartVmCache[pf.port] = null;
       await vmService?.stop();
       await pf.stop();
     }
@@ -260,8 +258,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,
-            timeout: _kDartVmConnectionTimeout);
+          final DartVm vmService = await _getDartVm(event.uri.port,
+              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) ??
@@ -309,7 +307,7 @@ class FuchsiaRemoteConnection {
         <Future<List<IsolateRef>>>[];
     for (final PortForwarder fp in _dartVmPortMap.values) {
       final DartVm vmService =
-      await _getDartVm(_getDartVmUri(fp), timeout: vmConnectionTimeout);
+          await _getDartVm(fp.port, timeout: vmConnectionTimeout);
       if (vmService == null) {
         continue;
       }
@@ -387,13 +385,13 @@ class FuchsiaRemoteConnection {
         _dartVmEventController.add(DartVmEvent._(
           eventType: DartVmEventType.stopped,
           servicePort: pf.remotePort,
-          uri: _getDartVmUri(pf),
+          uri: _getDartVmUri(pf.port),
         ));
       }
     }
 
     for (final PortForwarder pf in _dartVmPortMap.values) {
-      final DartVm service = await _getDartVm(_getDartVmUri(pf));
+      final DartVm service = await _getDartVm(pf.port);
       if (service == null) {
         await shutDownPortForwarder(pf);
       } else {
@@ -404,14 +402,15 @@ class FuchsiaRemoteConnection {
     return result;
   }
 
-  Uri _getDartVmUri(PortForwarder pf) {
-    String addr;
-    if (pf.openPortAddress == null) {
-      addr = _useIpV6 ? '[$_ipv6Loopback]' : _ipv4Loopback;
-    } else {
-      addr = _useIpV6 ? '[${pf.openPortAddress}]' : pf.openPortAddress;
-    }
-    final Uri uri = Uri.http('$addr:${pf.port}', '/');
+  Uri _getDartVmUri(int port) {
+    // While the IPv4 loopback can be used for the initial port forwarding
+    // (see [PortForwarder.start]), the address is actually bound to the IPv6
+    // loopback device, so connecting to the IPv4 loopback would fail when the
+    // target address is IPv6 link-local.
+    final String addr = _useIpV6Loopback
+        ? 'http://[$_ipv6Loopback]:$port'
+        : 'http://$_ipv4Loopback:$port';
+    final Uri uri = Uri.parse(addr);
     return uri;
   }
 
@@ -420,16 +419,17 @@ class FuchsiaRemoteConnection {
   /// Returns null if either there is an [HttpException] or a
   /// [TimeoutException], else a [DartVm] instance.
   Future<DartVm> _getDartVm(
-    Uri uri, {
+    int port, {
     Duration timeout = _kDartVmConnectionTimeout,
   }) async {
-    if (!_dartVmCache.containsKey(uri)) {
+    if (!_dartVmCache.containsKey(port)) {
       // 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 = await DartVm.connect(uri, timeout: timeout);
-        _dartVmCache[uri] = dartVm;
+        final DartVm dartVm =
+            await DartVm.connect(_getDartVmUri(port), timeout: timeout);
+        _dartVmCache[port] = dartVm;
       } on HttpException {
         _log.warning('HTTP Exception encountered connecting to new VM');
         return null;
@@ -438,7 +438,7 @@ class FuchsiaRemoteConnection {
         return null;
       }
     }
-    return _dartVmCache[uri];
+    return _dartVmCache[port];
   }
 
   /// Checks for changes in the list of Dart VM instances.
@@ -460,7 +460,7 @@ class FuchsiaRemoteConnection {
         _dartVmEventController.add(DartVmEvent._(
           eventType: DartVmEventType.started,
           servicePort: servicePort,
-          uri: _getDartVmUri(_dartVmPortMap[servicePort]),
+          uri: _getDartVmUri(_dartVmPortMap[servicePort].port),
         ));
       }
     }
@@ -482,11 +482,11 @@ class FuchsiaRemoteConnection {
     );
   }
 
-  /// Forwards a series of open ports to the remote device.
+  /// Forwards a series of local device ports to the remote device.
   ///
   /// When this function is run, all existing forwarded ports and connections
   /// are reset by way of [stop].
-  Future<void> _forwardOpenPortsToDeviceServicePorts() async {
+  Future<void> _forwardLocalPortsToDeviceServicePorts() async {
     await stop();
     final List<int> servicePorts = await getDeviceServicePorts();
     final List<PortForwarder> forwardedVmServicePorts =
@@ -548,13 +548,9 @@ class FuchsiaRemoteConnection {
 ///
 /// To shut down a port forwarder you must call the [stop] function.
 abstract class PortForwarder {
-  /// Determines the port which is being forwarded.
+  /// Determines the port which is being forwarded from the local machine.
   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;
 
@@ -585,9 +581,6 @@ class _SshPortForwarder implements PortForwarder {
   @override
   int get port => _localSocket.port;
 
-  @override
-  String get openPortAddress => _ipV6 ? _ipv6Loopback : _ipv4Loopback;
-
   @override
   int get remotePort => _remotePort;
 
@@ -609,9 +602,8 @@ 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). Therefore, while the IPv4 loopback can be used for
-    // forwarding to the destination IPv6 interface, when connecting to the
-    // websocket, the IPV6 loopback should be used.
+    // loopback (::1). While this can be used for forwarding to the destination
+    // IPv6 interface, it cannot be used to connect to a websocket.
     final String formattedForwardingUrl =
         '${localSocket.port}:$_ipv4Loopback:$remotePort';
     final String targetAddress =

packages/fuchsia_remote_debug_protocol/test/fuchsia_remote_connection_test.dart

@@ -31,6 +31,23 @@ void main() {
       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>{
@@ -73,7 +90,6 @@ void main() {
         },
       ];
 
-      forwardedPorts = <MockPortForwarder>[];
       mockPeerConnections = <MockPeer>[];
       uriConnections = <Uri>[];
       Future<json_rpc.Peer> mockVmConnectionFunction(
@@ -93,6 +109,7 @@ void main() {
         });
       }
 
+      fuchsiaPortForwardingFunction = mockPortForwardingFunction;
       fuchsiaVmServiceConnectionFunction = mockVmConnectionFunction;
     });
 
@@ -104,86 +121,6 @@ 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 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 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 FuchsiaRemoteConnection connection =
           await FuchsiaRemoteConnection.connectWithSshCommandRunner(mockRunner);
 
@@ -198,15 +135,6 @@ void main() {
       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);