Unverified Commit 764f4cc8 authored by Mouad Debbar's avatar Mouad Debbar Committed by GitHub

[web] Assert route names start with / if using PathUrlStrategy (#127986)

To avoid confusion and unpredictable errors (e.g. https://github.com/flutter/flutter/issues/127462), I'm adding this assert to disallow this pattern.

Fixes https://github.com/flutter/flutter/issues/127462
parent 13db2e4a
...@@ -63,9 +63,15 @@ class PathUrlStrategy extends ui_web.HashUrlStrategy { ...@@ -63,9 +63,15 @@ class PathUrlStrategy extends ui_web.HashUrlStrategy {
@override @override
String prepareExternalUrl(String internalUrl) { String prepareExternalUrl(String internalUrl) {
if (internalUrl.isNotEmpty && !internalUrl.startsWith('/')) { if (internalUrl.isEmpty) {
internalUrl = '/$internalUrl'; internalUrl = '/';
} }
assert(
internalUrl.startsWith('/'),
"When using PathUrlStrategy, all route names must start with '/' because "
"the browser's pathname always starts with '/'. "
"Found route name: '$internalUrl'",
);
return '$_basePath$internalUrl'; return '$_basePath$internalUrl';
} }
} }
...@@ -16,17 +16,16 @@ void main() { ...@@ -16,17 +16,16 @@ void main() {
setUp(() { setUp(() {
location = TestPlatformLocation(); location = TestPlatformLocation();
location.baseHref = '/';
}); });
test('allows null state', () { test('allows null state', () {
location.baseHref = '/';
final PathUrlStrategy strategy = PathUrlStrategy(location); final PathUrlStrategy strategy = PathUrlStrategy(location);
expect(() => strategy.pushState(null, '', '/'), returnsNormally); expect(() => strategy.pushState(null, '', '/'), returnsNormally);
expect(() => strategy.replaceState(null, '', '/'), returnsNormally); expect(() => strategy.replaceState(null, '', '/'), returnsNormally);
}); });
test('validates base href', () { test('validates base href', () {
location.baseHref = '/';
expect( expect(
() => PathUrlStrategy(location), () => PathUrlStrategy(location),
returnsNormally, returnsNormally,
...@@ -58,7 +57,6 @@ void main() { ...@@ -58,7 +57,6 @@ void main() {
}); });
test('leading slash is always prepended', () { test('leading slash is always prepended', () {
location.baseHref = '/';
final PathUrlStrategy strategy = PathUrlStrategy(location); final PathUrlStrategy strategy = PathUrlStrategy(location);
location.pathname = ''; location.pathname = '';
...@@ -95,13 +93,35 @@ void main() { ...@@ -95,13 +93,35 @@ void main() {
expect(strategy.getPath(), '/bar?q=1&t=r'); expect(strategy.getPath(), '/bar?q=1&t=r');
}); });
test('empty route name is ok', () {
final PathUrlStrategy strategy = PathUrlStrategy(location);
expect(strategy.prepareExternalUrl(''), '/');
expect(() => strategy.pushState(null, '', ''), returnsNormally);
expect(() => strategy.replaceState(null, '', ''), returnsNormally);
});
test('route names must start with /', () {
final PathUrlStrategy strategy = PathUrlStrategy(location);
expect(() => strategy.prepareExternalUrl('foo'), throwsAssertionError);
expect(() => strategy.prepareExternalUrl('foo/'), throwsAssertionError);
expect(() => strategy.prepareExternalUrl('foo/bar'), throwsAssertionError);
expect(() => strategy.pushState(null, '', 'foo'), throwsAssertionError);
expect(() => strategy.pushState(null, '', 'foo/'), throwsAssertionError);
expect(() => strategy.pushState(null, '', 'foo/bar'), throwsAssertionError);
expect(() => strategy.replaceState(null, '', 'foo'), throwsAssertionError);
expect(() => strategy.replaceState(null, '', 'foo/'), throwsAssertionError);
expect(() => strategy.replaceState(null, '', 'foo/bar'), throwsAssertionError);
});
test('generates external path correctly in the presence of basePath', () { test('generates external path correctly in the presence of basePath', () {
location.baseHref = 'https://example.com/foo/'; location.baseHref = 'https://example.com/foo/';
final PathUrlStrategy strategy = PathUrlStrategy(location); final PathUrlStrategy strategy = PathUrlStrategy(location);
expect(strategy.prepareExternalUrl(''), '/foo'); expect(strategy.prepareExternalUrl(''), '/foo/');
expect(strategy.prepareExternalUrl('/'), '/foo/'); expect(strategy.prepareExternalUrl('/'), '/foo/');
expect(strategy.prepareExternalUrl('bar'), '/foo/bar');
expect(strategy.prepareExternalUrl('/bar'), '/foo/bar'); expect(strategy.prepareExternalUrl('/bar'), '/foo/bar');
expect(strategy.prepareExternalUrl('/bar/'), '/foo/bar/'); expect(strategy.prepareExternalUrl('/bar/'), '/foo/bar/');
}); });
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment