Path wildcards are too wild (on D9/D10)
Overview
The routing integration for Civi-D9/D10 includes wildcard support. It's a bit too wild.
Steps to Reproduce
This issue was identified by an E2E unit test (AfformRoutingTest
), which provides one way to reproduce it.
cd vendor/civicrm/civicrm-core/afform/mock
phpunit9 tests/phpunit/api/v4/Afform/AfformRoutingTest.php
api\v4\Afform\AfformRoutingTest::testChangingPath
Failed asserting that 200 matches expected 403.
/home/homer/buildkit/build/build-3/vendor/civicrm/civicrm-core/ext/afform/mock/tests/phpunit/api/v4/Afform/AfformRoutingTest.php:86
/home/homer/buildkit/build/build-3/vendor/civicrm/civicrm-core/ext/afform/mock/tests/phpunit/api/v4/Afform/AfformRoutingTest.php:68
/home/homer/buildkit/extern/phpunit9/phpunit9.phar:2307
You can do the same steps manually:
- Original name
- Make a dummy page
civicrm/mock-page
. - Check that
civicrm/mock-page
is valid. - Check that
civicrm/mock-page-renamed
is not valid.
- Make a dummy page
- New name
- Rename
civicrm/mock-page
tocivicrm/mock-page-renamed
. - Check that
civicrm/mock-page
is not valid. - Check that
civicrm/mock-page-renamed
is valid.
- Rename
(These steps use Afform, but that's not necessary. Routes defined by extensions or by core have the same issues.)
Current behavior
The problem appears at step 1.3. On Civi-D9/D10, a request for civicrm/mock-page-renamed
matches a wildcard rule that allows it to fallback to civicrm/mock-page
.
This appears to be generally true of all routes on Civi-D9/D10, e.g.
- The route
civicrm/admin
has wildcard support. It matchescivicrm/adminhahafunny
orcivicrm/administrative-page
Expected behavior
The normal behavior (on other UFs) is for the wildcard to allow subpaths, e.g.
- The route
civicrm/mock-page
has wildcard support. It matchescivicrm/mock-page/haha/funny
but notcivicrm/mock-page-haha-funny
Edited by totten