Use first Dart VM Service found with mDNS if there are duplicates (#119545)

* fix when duplicate mdns results are found

* put mdns auth code in it's own function and update tests

* add comments, refactor auth code parsing, other small tweaks

packages/flutter_tools/lib/src/mdns_discovery.dart

@@ -10,6 +10,7 @@ import 'base/context.dart';
 import 'base/io.dart';
 import 'base/logger.dart';
 import 'build_info.dart';
+import 'convert.dart';
 import 'device.dart';
 import 'reporting/reporting.dart';
 
@@ -201,7 +202,16 @@ class MDnsVmServiceDiscovery {
 
       final List<MDnsVmServiceDiscoveryResult> results =
           <MDnsVmServiceDiscoveryResult>[];
+
+      // uniqueDomainNames is used to track all domain names of Dart VM services
+      // It is later used in this function to determine whether or not to throw an error.
+      // We do not want to throw the error if it was unable to find any domain
+      // names because that indicates it may be a problem with mDNS, which has
+      // a separate error message in _checkForIPv4LinkLocal.
       final Set<String> uniqueDomainNames = <String>{};
+      // uniqueDomainNamesInResults is used to filter out duplicates with exactly
+      // the same domain name from the results.
+      final Set<String> uniqueDomainNamesInResults = <String>{};
 
       // Listen for mDNS connections until timeout.
       final Stream<PtrResourceRecord> ptrResourceStream = client.lookup<PtrResourceRecord>(
@@ -223,6 +233,11 @@ class MDnsVmServiceDiscovery {
           domainName = ptr.domainName;
         }
 
+        // Result with same domain name was already found, skip it.
+        if (uniqueDomainNamesInResults.contains(domainName)) {
+          continue;
+        }
+
         _logger.printTrace('Checking for available port on $domainName');
         final List<SrvResourceRecord> srvRecords = await client
           .lookup<SrvResourceRecord>(
@@ -279,41 +294,18 @@ class MDnsVmServiceDiscovery {
               ResourceRecordQuery.text(domainName),
           )
           .toList();
-        if (txt.isEmpty) {
-          results.add(MDnsVmServiceDiscoveryResult(domainName, srvRecord.port, ''));
-          if (quitOnFind) {
-            return results;
-          }
-          continue;
-        }
-        const String authCodePrefix = 'authCode=';
-        String? raw;
-        for (final String record in txt.first.text.split('\n')) {
-          if (record.startsWith(authCodePrefix)) {
-            raw = record;
-            break;
-          }
-        }
-        if (raw == null) {
-          results.add(MDnsVmServiceDiscoveryResult(domainName, srvRecord.port, ''));
-          if (quitOnFind) {
-            return results;
-          }
-          continue;
-        }
-        String authCode = raw.substring(authCodePrefix.length);
-        // The Dart VM Service currently expects a trailing '/' as part of the
-        // URI, otherwise an invalid authentication code response is given.
-        if (!authCode.endsWith('/')) {
-          authCode += '/';
-        }
 
+        String authCode = '';
+        if (txt.isNotEmpty) {
+          authCode = _getAuthCode(txt.first.text);
+        }
         results.add(MDnsVmServiceDiscoveryResult(
           domainName,
           srvRecord.port,
           authCode,
           ipAddress: ipAddress
         ));
+        uniqueDomainNamesInResults.add(domainName);
         if (quitOnFind) {
           return results;
         }
@@ -338,6 +330,22 @@ class MDnsVmServiceDiscovery {
     }
   }
 
+  String _getAuthCode(String txtRecord) {
+    const String authCodePrefix = 'authCode=';
+    final Iterable<String> matchingRecords =
+        LineSplitter.split(txtRecord).where((String record) => record.startsWith(authCodePrefix));
+    if (matchingRecords.isEmpty) {
+      return '';
+    }
+    String authCode = matchingRecords.first.substring(authCodePrefix.length);
+    // The Dart VM Service currently expects a trailing '/' as part of the
+    // URI, otherwise an invalid authentication code response is given.
+    if (!authCode.endsWith('/')) {
+      authCode += '/';
+    }
+    return authCode;
+  }
+
   /// Gets Dart VM Service Uri for `flutter attach`.
   /// Executes an mDNS query and waits until a Dart VM Service is found.
   ///

packages/flutter_tools/test/general.shard/mdns_discovery_test.dart

@@ -112,6 +112,56 @@ void main() {
         expect(portDiscovery.queryForAttach, throwsToolExit());
       });
 
+      testWithoutContext('Find duplicates in preliminary client', () async {
+        final MDnsClient client = FakeMDnsClient(
+          <PtrResourceRecord>[
+            PtrResourceRecord('foo', future, domainName: 'bar'),
+            PtrResourceRecord('foo', future, domainName: 'bar'),
+          ],
+          <String, List<SrvResourceRecord>>{
+            'bar': <SrvResourceRecord>[
+              SrvResourceRecord('bar', future, port: 123, weight: 1, priority: 1, target: 'appId'),
+            ],
+          },
+        );
+
+        final MDnsVmServiceDiscovery portDiscovery = MDnsVmServiceDiscovery(
+          mdnsClient: emptyClient,
+          preliminaryMDnsClient: client,
+          logger: BufferLogger.test(),
+          flutterUsage: TestUsage(),
+        );
+
+        final MDnsVmServiceDiscoveryResult? result = await portDiscovery.queryForAttach();
+        expect(result, isNotNull);
+      });
+
+      testWithoutContext('Find similar named in preliminary client', () async {
+        final MDnsClient client = FakeMDnsClient(
+          <PtrResourceRecord>[
+            PtrResourceRecord('foo', future, domainName: 'bar'),
+            PtrResourceRecord('foo', future, domainName: 'bar (2)'),
+          ],
+          <String, List<SrvResourceRecord>>{
+            'bar': <SrvResourceRecord>[
+              SrvResourceRecord('bar', future, port: 123, weight: 1, priority: 1, target: 'appId'),
+            ],
+            'bar (2)': <SrvResourceRecord>[
+              SrvResourceRecord('bar', future, port: 123, weight: 1, priority: 1, target: 'appId'),
+            ],
+          },
+        );
+
+        final MDnsVmServiceDiscovery portDiscovery = MDnsVmServiceDiscovery(
+          mdnsClient: emptyClient,
+          preliminaryMDnsClient: client,
+          logger: BufferLogger.test(),
+          flutterUsage: TestUsage(),
+        );
+
+        expect(portDiscovery.queryForAttach, throwsToolExit());
+      });
+
       testWithoutContext('No ports available', () async {
         final MDnsVmServiceDiscovery portDiscovery = MDnsVmServiceDiscovery(
           mdnsClient: emptyClient,
@@ -680,6 +730,112 @@ void main() {
       expect(result?.domainName, 'srv-bar');
       expect(result?.port, 222);
     });
+    testWithoutContext('find with no txt record', () async {
+      final MDnsClient client = FakeMDnsClient(
+        <PtrResourceRecord>[
+          PtrResourceRecord('foo', future, domainName: 'srv-foo'),
+        ],
+        <String, List<SrvResourceRecord>>{
+          'srv-foo': <SrvResourceRecord>[
+            SrvResourceRecord('srv-foo', future, port: 111, weight: 1, priority: 1, target: 'target-foo'),
+          ],
+        },
+        ipResponse: <String, List<IPAddressResourceRecord>>{
+          'target-foo': <IPAddressResourceRecord>[
+            IPAddressResourceRecord('target-foo', 0, address: InternetAddress.tryParse('111.111.111.111')!),
+          ],
+        },
+      );
+
+      final MDnsVmServiceDiscovery portDiscovery = MDnsVmServiceDiscovery(
+        mdnsClient: client,
+        logger: BufferLogger.test(),
+        flutterUsage: TestUsage(),
+      );
+      final MDnsVmServiceDiscoveryResult? result = await portDiscovery.firstMatchingVmService(
+        client,
+        applicationId: 'srv-foo',
+        isNetworkDevice: true,
+      );
+      expect(result?.domainName, 'srv-foo');
+      expect(result?.port, 111);
+      expect(result?.authCode, '');
+      expect(result?.ipAddress?.address, '111.111.111.111');
+    });
+    testWithoutContext('find with empty txt record', () async {
+      final MDnsClient client = FakeMDnsClient(
+        <PtrResourceRecord>[
+          PtrResourceRecord('foo', future, domainName: 'srv-foo'),
+        ],
+        <String, List<SrvResourceRecord>>{
+          'srv-foo': <SrvResourceRecord>[
+            SrvResourceRecord('srv-foo', future, port: 111, weight: 1, priority: 1, target: 'target-foo'),
+          ],
+        },
+        txtResponse: <String, List<TxtResourceRecord>>{
+          'srv-foo': <TxtResourceRecord>[
+            TxtResourceRecord('srv-foo', future, text: ''),
+          ],
+        },
+        ipResponse: <String, List<IPAddressResourceRecord>>{
+          'target-foo': <IPAddressResourceRecord>[
+            IPAddressResourceRecord('target-foo', 0, address: InternetAddress.tryParse('111.111.111.111')!),
+          ],
+        },
+      );
+
+      final MDnsVmServiceDiscovery portDiscovery = MDnsVmServiceDiscovery(
+        mdnsClient: client,
+        logger: BufferLogger.test(),
+        flutterUsage: TestUsage(),
+      );
+      final MDnsVmServiceDiscoveryResult? result = await portDiscovery.firstMatchingVmService(
+        client,
+        applicationId: 'srv-foo',
+        isNetworkDevice: true,
+      );
+      expect(result?.domainName, 'srv-foo');
+      expect(result?.port, 111);
+      expect(result?.authCode, '');
+      expect(result?.ipAddress?.address, '111.111.111.111');
+    });
+    testWithoutContext('find with valid txt record', () async {
+      final MDnsClient client = FakeMDnsClient(
+        <PtrResourceRecord>[
+          PtrResourceRecord('foo', future, domainName: 'srv-foo'),
+        ],
+        <String, List<SrvResourceRecord>>{
+          'srv-foo': <SrvResourceRecord>[
+            SrvResourceRecord('srv-foo', future, port: 111, weight: 1, priority: 1, target: 'target-foo'),
+          ],
+        },
+        txtResponse: <String, List<TxtResourceRecord>>{
+          'srv-foo': <TxtResourceRecord>[
+            TxtResourceRecord('srv-foo', future, text: 'authCode=xyz\n'),
+          ],
+        },
+        ipResponse: <String, List<IPAddressResourceRecord>>{
+          'target-foo': <IPAddressResourceRecord>[
+            IPAddressResourceRecord('target-foo', 0, address: InternetAddress.tryParse('111.111.111.111')!),
+          ],
+        },
+      );
+
+      final MDnsVmServiceDiscovery portDiscovery = MDnsVmServiceDiscovery(
+        mdnsClient: client,
+        logger: BufferLogger.test(),
+        flutterUsage: TestUsage(),
+      );
+      final MDnsVmServiceDiscoveryResult? result = await portDiscovery.firstMatchingVmService(
+        client,
+        applicationId: 'srv-foo',
+        isNetworkDevice: true,
+      );
+      expect(result?.domainName, 'srv-foo');
+      expect(result?.port, 111);
+      expect(result?.authCode, 'xyz/');
+      expect(result?.ipAddress?.address, '111.111.111.111');
+    });
   });
 }