Bug #90959
closedWrong URL applied for Route
0%
Description
Hello Core-Team,
a customer has following routeEnhancer configuration. I have adopted it to my local machine:
base: 'http://typo395.ddev.site/' baseVariants: { } errorHandling: { } languages: - title: English enabled: true languageId: '0' base: / typo3Language: default locale: en_US.UTF-8 iso-639-1: en navigationTitle: English hreflang: en-US direction: ltr flag: en-us-gb - title: German enabled: true languageId: '1' base: /de/ typo3Language: de locale: de_DE.UTF-8 iso-639-1: de navigationTitle: Deutsch hreflang: de-DE direction: ltr fallbackType: fallback fallbacks: '0' flag: de solr_core_read: '' rootPageId: 1 routeEnhancers: News: type: Extbase extension: News plugin: Pi1 routes: - routePath: '/page-{page}' _controller: 'News::list' _arguments: page: '@widget_0/currentPage' - routePath: '/{news-title}' _controller: 'News::detail' _arguments: news-title: news - routePath: '/{category-name}' _controller: 'News::list' _arguments: category-name: overwriteDemand/categories - routePath: '/{tag-name}' _controller: 'News::list' _arguments: tag-name: overwriteDemand/tags defaultController: 'News::list' defaults: page: '0' aspects: news-title: type: PersistedAliasMapper tableName: tx_news_domain_model_news routeFieldName: path_segment page: type: StaticRangeMapper start: '1' end: '100' category-name: type: PersistedAliasMapper tableName: sys_category routeFieldName: slug tag-name: type: PersistedAliasMapper tableName: tx_news_domain_model_tag routeFieldName: slug DateMenu: type: Extbase extension: News plugin: Pi1 routes: - routePath: '/page-{page}' _controller: 'News::list' _arguments: page: '@widget_0/currentPage' requirements: page: \d+ - routePath: '/{news-title}' _controller: 'News::detail' _arguments: news-title: news - routePath: '/{date-year}' _controller: 'News::list' _arguments: date-year: overwriteDemand/year requirements: date-year: \d+ - routePath: '/{date-year}/page-{page}' _controller: 'News::list' _arguments: date-year: overwriteDemand/year page: '@widget_0/currentPage' requirements: date-year: \d+ page: \d+ - routePath: '/{date-year}/{date-month}' _controller: 'News::list' _arguments: date-month: overwriteDemand/month date-year: overwriteDemand/year requirements: date-month: \d+ date-year: \d+ - routePath: '/{date-year}/{date-month}/page-{page}' _controller: 'News::list' _arguments: date-month: overwriteDemand/month date-year: overwriteDemand/year page: '@widget_0/currentPage' requirements: date-month: \d+ date-year: \d+ page: \d+ defaultController: 'News::list' defaults: page: '0' date-month: '' date-year: '' aspects: news-title: type: PersistedAliasMapper tableName: tx_news_domain_model_news routeFieldName: path_segment page: type: StaticRangeMapper start: '1' end: '25' date-month: type: StaticValueMapper map: '01': '01' '02': '02' '03': '03' '04': '04' '05': '05' '06': '06' '07': '07' '08': '08' '09': '09' 10: '10' 11: '11' 12: '12' date-year: type: StaticRangeMapper start: '2000' end: '2030'
When customer opens list view of news following error occurs:
Parameter "acec1fc0ac7e1adfbcd522115dc0792" for route "tx_news_pi1_4" must match "[^/]++" ("" given) to generate a corresponding URL.
It has nothing to do with one or more slashes in path_segment, as there are no slashes in there.
Please have a look at the Routes name "tx_news_pi1_4". The last part of it is a counter of found Routes. As it starts with 0, five valid routes have been found with the configuration above. A little bit too much IMO. This has something to do with the configured "defaults" named page, date-month and date-year. These defaults will be added to all Routes regardless, if Routepath has these variables or not.
There is not check anymore, if configured route matches the requested URI. You only check, if configured Variable is Part of the MergedParameters regardless, if URI matches "/page-{page}/" or "news/{title}/{page}". Both Routes will be added because they contain {page}.
In our customers case the first processed route is " /{date-year}/{date-month}/page-{page}" although only a page ID is given. The other path parts are configured as empty in defaults. That's why path is now "///page-2" which is IMO very wrong, but explains the error we get.
In ExtbasePluginEnhancer in method getVariant I see, that you don't set any Requirements to new Route:
$route = new Route(rtrim($defaultPageRoute->getPath(), '/') . '/' . ltrim($routePath, '/'), $defaults, [], $options);
Can you please tell me why?
Updated by Benni Mack over 4 years ago
- Target version changed from next-patchlevel to Candidate for patchlevel
Updated by Oliver Hader about 2 years ago
Preface¶
requirements
are should be checked for resolving only, and thus concern values in a URL- assume there's a mapper that transforms numbers into words, e.g. "0" to "zero", "1" to "one", "2" to "two", ...
- having a
requirements
setting for that variable withzero|one|two
would match URLs like/company-news/page-two
- having a
requirements
setting for that variable with\d+
would only match/company-news/page-2
(when used without mentioned mapper)
Analyzing configuration¶
The shown configuration has five ambiguous configurations for the same target controller-action News::list
. There are (invalid) defaults
for all parameters, thus all of these routes would be valid. As mentioned above, requirements
usually concerns values in the generated URL, but not the raw input parameter values.
routes: - routePath: '/page-{page}' _controller: 'News::list' - routePath: '/{date-year}' _controller: 'News::list' - routePath: '/{date-year}/page-{page}' _controller: 'News::list' - routePath: '/{date-year}/{date-month}' _controller: 'News::list' - routePath: '/{date-year}/{date-month}/page-{page}' _controller: 'News::list' defaultController: 'News::list' defaults: page: '0' date-month: '' date-year: ''
The mentioned exception route "tx_news_pi1_4" must match "[^/]++" ("" given)
could be resolved by using non-blank defaults
like data-month: '12'
and date-year: '2022'
. The generated route would be valid and look like company-news/2022/12/page-0
.
However, when generating a route with input params like ?id=123&tx_news_pi1[controller]=News&tx_news_pi1[action]=list
, I guess company-news/2022/12/page-0
is still not the expected behavior. By looking to the enhancer configuration above again, it probably might be company-news/page-0
or company-news/2022
- but that's still ambiguous.
Conclusion¶
The problems mentioned above can be mitigated by avoiding defaults
as much as possible (make it explicit). In case defaults
cannot be avoided, a new/better algorithmic approach would have to prioritize those routes that match best to the given URL parameters and use the least defaults
values as possible - which still cannot resolve mentioned ambiguity completely.
Prioritization would have to be applied in \TYPO3\CMS\Core\Routing\PageRouter::generateUri
.
Updated by Oliver Hader about 2 years ago
- Related to Bug #90924: Maximum route parameters always appended added
Updated by Oliver Hader about 2 years ago
- Status changed from New to Needs Feedback
- Tags set to pending-close
Seems like this might be solved already with issue #90924 (merged in July 2020). The current issue was reported earlier (April 2020).
Can you please check, whether those problems still exist? Thx!
Updated by Stefan Froemken over 1 year ago
- Status changed from Needs Feedback to Closed
Closed.
The configuration is not valid. There are two Enhancers with same Type, Extension and Plugin. In that case only the last Enhancer wins.
Further: requirements is not allowed within RoutePath. It's only allowed within the Enhancer itself.
If you're on Page Home->Extensions->News->CategoryView and you have 9 RoutePaths configured the RouteCollection will now contain 36 possible routes. Don't know, if that's the way to go. But sorting within FullCollection is OK. Routes for page CategoryView will be processed first. Perfect.
In new version, if there are multiple possible routes, there is a condition which checks, if record with variable from URL exists, if not, the matcher will try the next possible route.
The new approach is much better now. So: closed.