[go_router] Fixes deep links with no path (flutter#6447)

This PR addresses an issue where root deep links did not correctly navigate to '/'. The problem originated from the assumption that only paths (like '/', '/details', etc.) would be passed to the `canonicalUri` method in `path_utils.dart`. However, in the case of deep links, the full URL (including the scheme and domain) would be passed, causing the trailing slash to be incorrectly removed for root URLs.

The first part of the fix modifies the `canonicalUri` method to check the actual path using `uri.path`. This ensures that the trailing slash is preserved for root URLs, even when the full URL is passed to the method. Importantly, even if '/' or '/details' is passed as a URI to `canonicalUri`, `uri.path` will still return '/' or '/details', ensuring consistent behavior.

The second part of the fix modifies the `parseRouteInformationWithDependencies` function in `parser.dart` to correctly handle URLs with query parameters or fragments. Previously, the trailing slash was added after the query parameters or fragments, which is incorrect. The fix ensures that the trailing slash is added immediately after the base URL, before any query parameters or fragments.

Preserving the trailing slash for root URLs is crucial because the `_matchByNavigatorKey` method in `match.dart` uses `uri.path` as the `remainingLocation` parameter. (why is that method relevant? because `parseRouteInformationWithDependencies` uses `findMatch`, which calls `_getLocRouteMatches`, which calls `match` and it calls `_matchByNavigatorKey`). If the trailing slash is removed from the root deep link URL, `uri.path` will not return '/', and the URL will not match any routes in the Go router.

To validate these changes, new tests have been added. In `path_utils_test.dart`, tests have been added to verify that the trailing slash is not removed from a URL that is not just the path. In `parser_test.dart`, a new test has been added to verify that a URI with an empty path is correctly parsed. This test covers the case where `routeInformation.uri.hasEmptyPath` is true, which was not previously tested.

Issues fixed:
 - flutter/flutter#133928

Author: kforjan

packages/go_router/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 13.2.3
+
+- Fixes an issue where deep links without path caused an exception
+
 ## 13.2.2
 
 - Fixes restoreRouteInformation issue when GoRouter.optionURLReflectsImperativeAPIs is true and the last match is ShellRouteMatch

packages/go_router/lib/src/parser.dart

@@ -79,11 +79,24 @@ class GoRouteInformationParser extends RouteInformationParser<RouteMatchList> {
     }
 
     late final RouteMatchList initialMatches;
-    initialMatches = configuration.findMatch(
-        routeInformation.uri.path.isEmpty
-            ? '${routeInformation.uri}/'
-            : routeInformation.uri.toString(),
-        extra: state.extra);
+    if (routeInformation.uri.hasEmptyPath) {
+      String newUri = '${routeInformation.uri.origin}/';
+      if (routeInformation.uri.hasQuery) {
+        newUri += '?${routeInformation.uri.query}';
+      }
+      if (routeInformation.uri.hasFragment) {
+        newUri += '#${routeInformation.uri.fragment}';
+      }
+      initialMatches = configuration.findMatch(
+        newUri,
+        extra: state.extra,
+      );
+    } else {
+      initialMatches = configuration.findMatch(
+        routeInformation.uri.toString(),
+        extra: state.extra,
+      );
+    }
     if (initialMatches.isError) {
       log('No initial matches: ${routeInformation.uri.path}');
     }

packages/go_router/lib/src/path_utils.dart

@@ -126,19 +126,22 @@ String canonicalUri(String loc) {
   }
   String canon = Uri.parse(loc).toString();
   canon = canon.endsWith('?') ? canon.substring(0, canon.length - 1) : canon;
+  final Uri uri = Uri.parse(canon);
 
   // remove trailing slash except for when you shouldn't, e.g.
   // /profile/ => /profile
   // / => /
-  // /login?from=/ => login?from=/
-  canon = canon.endsWith('/') && canon != '/' && !canon.contains('?')
+  // /login?from=/ => /login?from=/
+  canon = uri.path.endsWith('/') &&
+          uri.path != '/' &&
+          !uri.hasQuery &&
+          !uri.hasFragment
       ? canon.substring(0, canon.length - 1)
       : canon;
 
   // replace '/?', except for first occurrence, from path only
   // /login/?from=/ => /login?from=/
   // /?from=/ => /?from=/
-  final Uri uri = Uri.parse(canon);
   final int pathStartIndex = uri.host.isNotEmpty
       ? uri.toString().indexOf(uri.host) + uri.host.length
       : uri.hasScheme

packages/go_router/pubspec.yaml

@@ -1,7 +1,7 @@
 name: go_router
 description: A declarative router for Flutter based on Navigation 2 supporting
   deep linking, data-driven routes and more
-version: 13.2.2
+version: 13.2.3
 repository: https://github.com/flutter/packages/tree/main/packages/go_router
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router%22
 

packages/go_router/test/parser_test.dart

@@ -82,13 +82,54 @@ void main() {
   });
 
   testWidgets(
-      "GoRouteInformationParser can parse deeplink route and maintain uri's scheme and host",
+      "GoRouteInformationParser can parse deeplink root route and maintain uri's scheme, host, query and fragment",
+      (WidgetTester tester) async {
+    const String expectedScheme = 'https';
+    const String expectedHost = 'www.example.com';
+    const String expectedQuery = 'abc=def';
+    const String expectedFragment = 'abc';
+    const String expectedUriString =
+        '$expectedScheme://$expectedHost/?$expectedQuery#$expectedFragment';
+    final List<GoRoute> routes = <GoRoute>[
+      GoRoute(
+        path: '/',
+        builder: (_, __) => const Placeholder(),
+      ),
+    ];
+    final GoRouteInformationParser parser = await createParser(
+      tester,
+      routes: routes,
+      redirectLimit: 100,
+      redirect: (_, __) => null,
+    );
+
+    final BuildContext context = tester.element(find.byType(Router<Object>));
+
+    final RouteMatchList matchesObj =
+        await parser.parseRouteInformationWithDependencies(
+            createRouteInformation(expectedUriString), context);
+    final List<RouteMatchBase> matches = matchesObj.matches;
+    expect(matches.length, 1);
+    expect(matchesObj.uri.toString(), expectedUriString);
+    expect(matchesObj.uri.scheme, expectedScheme);
+    expect(matchesObj.uri.host, expectedHost);
+    expect(matchesObj.uri.query, expectedQuery);
+    expect(matchesObj.uri.fragment, expectedFragment);
+
+    expect(matches[0].matchedLocation, '/');
+    expect(matches[0].route, routes[0]);
+  });
+
+  testWidgets(
+      "GoRouteInformationParser can parse deeplink route with a path and maintain uri's scheme, host, query and fragment",
       (WidgetTester tester) async {
     const String expectedScheme = 'https';
     const String expectedHost = 'www.example.com';
     const String expectedPath = '/abc';
+    const String expectedQuery = 'abc=def';
+    const String expectedFragment = 'abc';
     const String expectedUriString =
-        '$expectedScheme://$expectedHost$expectedPath';
+        '$expectedScheme://$expectedHost$expectedPath?$expectedQuery#$expectedFragment';
     final List<GoRoute> routes = <GoRoute>[
       GoRoute(
         path: '/',
@@ -119,6 +160,8 @@ void main() {
     expect(matchesObj.uri.scheme, expectedScheme);
     expect(matchesObj.uri.host, expectedHost);
     expect(matchesObj.uri.path, expectedPath);
+    expect(matchesObj.uri.query, expectedQuery);
+    expect(matchesObj.uri.fragment, expectedFragment);
 
     expect(matches[0].matchedLocation, '/');
     expect(matches[0].route, routes[0]);

packages/go_router/test/path_utils_test.dart

@@ -100,6 +100,15 @@ void main() {
     verify('/a/', '/a');
     verify('/', '/');
     verify('/a/b/', '/a/b');
+    verify('https://www.example.com/', 'https://www.example.com/');
+    verify('https://www.example.com/a', 'https://www.example.com/a');
+    verify('https://www.example.com/a/', 'https://www.example.com/a');
+    verify('https://www.example.com/a/b/', 'https://www.example.com/a/b');
+    verify('https://www.example.com/?', 'https://www.example.com/');
+    verify('https://www.example.com/?a=b', 'https://www.example.com/?a=b');
+    verify('https://www.example.com/?a=/', 'https://www.example.com/?a=/');
+    verify('https://www.example.com/a/?b=c', 'https://www.example.com/a?b=c');
+    verify('https://www.example.com/#a/', 'https://www.example.com/#a/');
 
     expect(() => canonicalUri('::::'), throwsA(isA<FormatException>()));
     expect(() => canonicalUri(''), throwsA(anything));