From 3c788b9279abca46e93becdf3100eb56c9e39ca6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kre=C5=A1imir=20Forjan?= Date: Mon, 1 Apr 2024 18:06:55 +0200 Subject: [PATCH 1/9] Preserve trailing slash for root URLs in canonicalUri --- packages/go_router/lib/src/path_utils.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/go_router/lib/src/path_utils.dart b/packages/go_router/lib/src/path_utils.dart index 37e76853e311..7039bbf00c93 100644 --- a/packages/go_router/lib/src/path_utils.dart +++ b/packages/go_router/lib/src/path_utils.dart @@ -125,20 +125,20 @@ String canonicalUri(String loc) { throw GoException('Location cannot be empty.'); } String canon = Uri.parse(loc).toString(); + final Uri uri = Uri.parse(canon); canon = canon.endsWith('?') ? canon.substring(0, canon.length - 1) : canon; // remove trailing slash except for when you shouldn't, e.g. // /profile/ => /profile // / => / // /login?from=/ => login?from=/ - canon = canon.endsWith('/') && canon != '/' && !canon.contains('?') + canon = uri.path.endsWith('/') && uri.path != '/' && !uri.hasQuery ? 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 From 7df6b6cdb1df4a47a5bce84ef523e3061554b67b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kre=C5=A1imir=20Forjan?= Date: Mon, 1 Apr 2024 18:07:57 +0200 Subject: [PATCH 2/9] Add tests for preserving trailing slash in root URLs --- packages/go_router/test/path_utils_test.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/go_router/test/path_utils_test.dart b/packages/go_router/test/path_utils_test.dart index 1c883c451559..ea45cf43ae24 100644 --- a/packages/go_router/test/path_utils_test.dart +++ b/packages/go_router/test/path_utils_test.dart @@ -100,6 +100,10 @@ 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'); expect(() => canonicalUri('::::'), throwsA(isA())); expect(() => canonicalUri(''), throwsA(anything)); From 1735d96b2419bcb016c28c51d4c227543a9b6ee7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kre=C5=A1imir=20Forjan?= Date: Mon, 1 Apr 2024 18:12:05 +0200 Subject: [PATCH 3/9] Ensure trailing slash for empty paths with query and/or fragment --- packages/go_router/lib/src/parser.dart | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/go_router/lib/src/parser.dart b/packages/go_router/lib/src/parser.dart index 9cd92c1848b6..b466d560b217 100644 --- a/packages/go_router/lib/src/parser.dart +++ b/packages/go_router/lib/src/parser.dart @@ -79,11 +79,19 @@ class GoRouteInformationParser extends RouteInformationParser { } 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}'); } From d2f8e529a51f0c843410f67bee8d02571be1f0c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kre=C5=A1imir=20Forjan?= Date: Mon, 1 Apr 2024 18:14:25 +0200 Subject: [PATCH 4/9] Add and modify tests to include empty path, query and fragment parameters --- packages/go_router/test/parser_test.dart | 47 +++++++++++++++++++++++- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/packages/go_router/test/parser_test.dart b/packages/go_router/test/parser_test.dart index 2d42de859e7e..31b8a50e5699 100644 --- a/packages/go_router/test/parser_test.dart +++ b/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 routes = [ + 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)); + + final RouteMatchList matchesObj = + await parser.parseRouteInformationWithDependencies( + createRouteInformation(expectedUriString), context); + final List 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 routes = [ 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]); From f576b70655c8d09f95cbbff9d40e5c4f5dce1805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kre=C5=A1imir=20Forjan?= Date: Mon, 1 Apr 2024 19:53:35 +0200 Subject: [PATCH 5/9] Update changelog.md and pubspec.yaml --- packages/go_router/CHANGELOG.md | 4 ++++ packages/go_router/pubspec.yaml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index d268648720c3..9bc93d1d3d8c 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,3 +1,7 @@ +## 13.2.3 + +- Fixes an issue where links without path caused an exception + ## 13.2.2 - Fixes restoreRouteInformation issue when GoRouter.optionURLReflectsImperativeAPIs is true and the last match is ShellRouteMatch diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index ac7945e62ece..1fc99858822f 100644 --- a/packages/go_router/pubspec.yaml +++ b/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 From cd5f14bc5b17945aa9679dafa838ef906ea16f02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kre=C5=A1imir=20Forjan?= Date: Mon, 1 Apr 2024 23:11:48 +0200 Subject: [PATCH 6/9] Update changelog for clarification --- packages/go_router/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 9bc93d1d3d8c..774d71c756e8 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,6 +1,6 @@ ## 13.2.3 -- Fixes an issue where links without path caused an exception +- Fixes an issue where deep links without path caused an exception ## 13.2.2 From e9b0887dc5f33c13cd32137a59ea2824811163d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kre=C5=A1imir=20Forjan?= Date: Fri, 5 Apr 2024 01:19:57 +0200 Subject: [PATCH 7/9] minore fixes, fix parsing placement, add fragment check --- packages/go_router/lib/src/parser.dart | 6 ++++-- packages/go_router/lib/src/path_utils.dart | 9 ++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/go_router/lib/src/parser.dart b/packages/go_router/lib/src/parser.dart index b466d560b217..1b9a3a3b06dc 100644 --- a/packages/go_router/lib/src/parser.dart +++ b/packages/go_router/lib/src/parser.dart @@ -89,8 +89,10 @@ class GoRouteInformationParser extends RouteInformationParser { } initialMatches = configuration.findMatch(newUri, extra: state.extra); } else { - initialMatches = configuration.findMatch(routeInformation.uri.toString(), - extra: state.extra); + initialMatches = configuration.findMatch( + routeInformation.uri.toString(), + extra: state.extra, + ); } if (initialMatches.isError) { log('No initial matches: ${routeInformation.uri.path}'); diff --git a/packages/go_router/lib/src/path_utils.dart b/packages/go_router/lib/src/path_utils.dart index 7039bbf00c93..49a7b33b3ebc 100644 --- a/packages/go_router/lib/src/path_utils.dart +++ b/packages/go_router/lib/src/path_utils.dart @@ -125,14 +125,17 @@ String canonicalUri(String loc) { throw GoException('Location cannot be empty.'); } String canon = Uri.parse(loc).toString(); - final Uri uri = Uri.parse(canon); 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 = uri.path.endsWith('/') && uri.path != '/' && !uri.hasQuery + // /login?from=/ => /login?from=/ + canon = uri.path.endsWith('/') && + uri.path != '/' && + !uri.hasQuery && + !uri.hasFragment ? canon.substring(0, canon.length - 1) : canon; From 10d34c889b1bdea486f049148d24a48de2b723fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kre=C5=A1imir=20Forjan?= Date: Fri, 5 Apr 2024 01:22:51 +0200 Subject: [PATCH 8/9] cover canonical url edge cases --- packages/go_router/test/path_utils_test.dart | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/go_router/test/path_utils_test.dart b/packages/go_router/test/path_utils_test.dart index ea45cf43ae24..5a656df135eb 100644 --- a/packages/go_router/test/path_utils_test.dart +++ b/packages/go_router/test/path_utils_test.dart @@ -104,6 +104,11 @@ void main() { 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())); expect(() => canonicalUri(''), throwsA(anything)); From 80ef1265043b3c974094aebb017bb9d2d6083797 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kre=C5=A1imir=20Forjan?= Date: Fri, 5 Apr 2024 01:57:47 +0200 Subject: [PATCH 9/9] fix formatting --- packages/go_router/lib/src/parser.dart | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/go_router/lib/src/parser.dart b/packages/go_router/lib/src/parser.dart index 1b9a3a3b06dc..bffcd1b15b62 100644 --- a/packages/go_router/lib/src/parser.dart +++ b/packages/go_router/lib/src/parser.dart @@ -87,7 +87,10 @@ class GoRouteInformationParser extends RouteInformationParser { if (routeInformation.uri.hasFragment) { newUri += '#${routeInformation.uri.fragment}'; } - initialMatches = configuration.findMatch(newUri, extra: state.extra); + initialMatches = configuration.findMatch( + newUri, + extra: state.extra, + ); } else { initialMatches = configuration.findMatch( routeInformation.uri.toString(),