Project

General

Profile

Actions

Bug #90959

closed

Wrong URL applied for Route

Added by Stefan Froemken about 4 years ago. Updated about 1 year ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Link Handling, Site Handling & Routing
Start date:
2020-04-06
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
9
PHP Version:
7.2
Tags:
pending-close
Complexity:
Is Regression:
Yes
Sprint Focus:
On Location Sprint

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?


Related issues 1 (0 open1 closed)

Related to TYPO3 Core - Bug #90924: Maximum route parameters always appendedClosed2020-04-01

Actions
Actions #1

Updated by Benni Mack almost 4 years ago

  • Target version changed from next-patchlevel to Candidate for patchlevel
Actions #2

Updated by Benni Mack over 1 year ago

  • Sprint Focus set to On Location Sprint
Actions #3

Updated by Oliver Hader over 1 year 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 with zero|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.

Actions #4

Updated by Oliver Hader over 1 year ago

  • Related to Bug #90924: Maximum route parameters always appended added
Actions #5

Updated by Oliver Hader over 1 year 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!

Actions #6

Updated by Stefan Froemken about 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.

Actions

Also available in: Atom PDF