Bug #91098

Routing and RouteEnhancer problems

Added by Aimeos no-lastname-given 3 months ago. Updated 13 days ago.

Status:
Needs Feedback
Priority:
Should have
Assignee:
-
Category:
Link Handling, Site Handling & Routing
Target version:
-
Start date:
2020-06-23
Due date:
% Done:

0%

TYPO3 Version:
9
PHP Version:
7.4
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

The 9.5 routing and the route enhancers seem to have some problems or at least don't behave as expected comparing to Symfony/Laravel.

I've used this configuration for testing:

routeEnhancers:
  Aimeos:
    type: Extbase
    namespace: ai
    limitToPages: [90]
    defaultController: 'Catalog::list'
    routes:
      - { routePath: '/{d_name}', _controller: 'Catalog::detail' }
      - { routePath: '/{d_name}/{d_pos}', _controller: 'Catalog::detail' }
    requirements:
      d_pos: '[0-9]*'

First observation: Routes are tested in reverse order, not from top to bottom. This doesn't make sense since the routes are not sorted by their path length and longer paths are not tried first.

When testing the rules with different parameters I get unexpected results:

URL: http://127.0.0.1:8000/your-shop/shop/detail?ai%5Bd_name%5D=Demo-selection-article&ai%5Bd_pos%5D=1
Expected: http://127.0.0.1:8000/your-shop/shop/detail/Demo-selection-article/1
Result: http://127.0.0.1:8000/your-shop/shop/detail/Demo-selection-article/1

URL: http://127.0.0.1:8000/your-shop/shop/detail?ai%5Bd_name%5D=Demo-selection-article
Expected: http://127.0.0.1:8000/your-shop/shop/detail/Demo-selection-article
Result: Exception that the ai[d_pos] parameter is missing because it doesn't test the next rule (/{d_name})

URL: http://127.0.0.1:8000/your-shop/shop/detail?ai%5Bd_name%5D=Demo-selection-article&ai%5Bd_pos%5D=
Expected: http://127.0.0.1:8000/your-shop/shop/detail/Demo-selection-article/
Result: Exception "Parameter "ai__d_pos" for route "ai_1" must match "[^/]++" ("" given) to generate a corresponding URL."


Subtasks

Task #91702: Evaluate possibility for optional route partsNew

Task #91703: Add configuration guard for ambiguous literals in route pathsNew


Related issues

Related to TYPO3 Core - Bug #90924: Maximum route parameters always appended Under Review 2020-04-01

History

#1 Updated by Oliver Hader 3 months ago

  • Category set to Link Handling, Site Handling & Routing

#2 Updated by Oliver Hader 2 months ago

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

#3 Updated by Oliver Hader 20 days ago

  • Status changed from New to Needs Feedback

Please have a look at issue #91098 and the corresponding patch at https://review.typo3.org/c/Packages/TYPO3.CMS/+/64877 which (hopefully) solves the problem.

#4 Updated by Aimeos no-lastname-given 15 days ago

One problem or at least the behavior is different to Laravel/Symfony:

routeEnhancers:
  Aimeos:
    type: Extbase
    namespace: ai
    limitToPages: [90]
    defaultController: 'Catalog::list'
    routes:
      - { routePath: '/{d_name}/{d_pos}', _controller: 'Catalog::detail' }
      - { routePath: '/{d_name}', _controller: 'Catalog::detail' }

If the parameter d_pos is added to the parameters but empty:

generateUri( $target, ['ai' => ['d_name' => 'test', 'd_pos' => '']] )

This exception occurs:

Symfony\\Component\\Routing\\Exception\\InvalidParameterException: Parameter \"ai__d_pos\" for route \"ai_0\" must match \"[^/]
++\" (\"\" given) to generate a corresponding URL. in /test/typo10/vendor/symfony/routing/Generator/UrlGenerator.php:193
Stack trace:
#0 /home/nose/Aimeos/test/typo10/public/typo3/sysext/core/Classes/Routing/UrlGenerator.php(55): Symfony\\Component\\Routing\\Generator\\UrlGenerator->doGenerate(Array, Array, Array, Array, Array, 'ai_0', 0, Array, Array)
#1 /test/typo10/vendor/symfony/routing/Generator/UrlGenerator.php(160): TYPO3\\CMS\\Core\\Routing\\UrlGenerator->doGenerate(Array, Array, Array, Array, Array, 'ai_0', 0, Array, Array)
#2 /test/typo10/public/typo3/sysext/core/Classes/Routing/PageRouter.php(336): Symfony\\Compone
nt\\Routing\\Generator\\UrlGenerator->generate('ai_0', Array, 0)
#3 /test/typo10/public/typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php(419): TYPO3\\CMS\\Core\\Routing\\PageRouter->generateUri(Object(TYPO3\\CMS\\Core\\Routing\\Route), Array, '', 'url')
#4 /test/typo10/public/typo3/sysext/frontend
/Classes/Typolink/PageLinkBuilder.php(232): TYPO3\\CMS\\Frontend\\Typolink\\PageLinkBuilder->generateUrlForPageWithSiteConfiguration(Array, Object(TYPO3\\CMS\\Core\\Site\\Entity\\Site), Array, '', Array)

If a default value is configured, it works:

defaults:
  d_pos: ''

#5 Updated by Oliver Hader 14 days ago

Aimeos no-lastname-given wrote:

One problem or at least the behavior is different to Laravel/Symfony:

Actually it's Symfony's native behaviour on compiling routes, including the default requirement of [^/]+ to ensure variables values are not empty and do not contain a slash. Laravel has a different routing strategy and different syntax.

Could you please attach the Symfony or even Laravel route declaration you used as a reference for comparison?

#6 Updated by Aimeos no-lastname-given 13 days ago

Here is the Laravel route definition:

Route::match( array( 'GET', 'POST' ), '{d_name}/{d_pos?}', array(
    'as' => 'aimeos_shop_detail',
    'uses' => 'Aimeos\Shop\Controller\CatalogController@detailAction'
) )->where( ['d_pos' => '[0-9]*'] );

Checked the same in TYPO3 with:
    routes:
      - { routePath: '/{d_name}/{d_pos}', _controller: 'Catalog::detail' }
    requirements:
      d_pos: '[0-9]*'

which works the same way.

One more observation:
Why are parameters often translated from "ai[abc]=1" to "ai__abc=1"? What's the reason behind? And why isn't this always the case?

#7 Updated by Oliver Hader 13 days ago

Aimeos no-lastname-given wrote:

Here is the Laravel route definition:

Route::match(['GET', 'POST'], '{d_name}/{d_pos?}', [
    'as' => 'aimeos_shop_detail',
    'uses' => 'Aimeos\Shop\Controller\CatalogController@detailAction'
])->where(['d_pos' => '[0-9]*']);

  • {d_pos?} marking an optional parameter would have to be defined with defaults in Symfony
  • where[] marking a condition would have to be defined with requirements (you mentioned that already)

Thus, the TYPO3-way (in most parts the Symfony-way) would indeed be

routes:
 - { routePath: '/{d_name}/{d_pos}', _controller: 'Catalog::detail' }
defaults:
  d_pos: ''
requirements:
  d_pos: '[0-9]*'

That being said, the default value would lead to the situation that the URL parameter is defined, but empty (e.g. &d_name=something&d_pos=), but could/should be omitted when comparing to Laravel. Sounds like an interesting enhancement, when the default value e.g. would be set like d_pos: null (untested, how Symfony behaves in this scenario).

One more observation:
Why are parameters often translated from "ai[abc]=1" to "ai__abc=1"? What's the reason behind? And why isn't this always the case?

In TYPO3 basically VariableProcessor is taking care of transforming given route variables (and their optionally given target arguments) for internal process with regular expressions. Once everything is done, these keys are transformed back to their original variables or target arguments.

The reasons back then in 2018 where

  • PCRE literals like [].+ would have to be escaped during the route compile process
    • handing over unescaped literals (e.g. ai[abc]), or even accidentally double escaped literals (e.g. ai\\\[abc\\\]) would lead to side effects
    • debugging (and understanding) of generated regular expressions during runtime having large expressions would become more difficult
    • it's however somehow a "comfort" feature
  • a couple of those literals like /,;.:-_~+*=|@ are used as separators by Symfony's route compiler
  • long variable names exceeding 32-characters are not supported in the route compiler (and PCRE)

The test cases for VariableProcess show most of the use cases and their expected results, please see https://github.com/TYPO3/TYPO3.CMS/blob/9.5/typo3/sysext/core/Tests/Unit/Routing/Enhancer/VariableProcessorTest.php

#8 Updated by Aimeos no-lastname-given 13 days ago

Thank you for your detailed explaination!

Tried to create a route for this URL:

/confirm?action=confirm&code=demo-invoice&controller=Checkout

using:
  AimeosConfirm:
    type: Extbase
    namespace: ai
    limitToPages: [28]
    routes:
      - { routePath: '/{code}', _controller: 'Checkout::confirm' }
    defaults:
      code: ''

This obviously fails because the parameter is "code", not "ai[code]". How to solve that?

Two more suggestions for later:
- It would be nice to have an option which applies the routes to subpages as well when using "limitToPages"
- We would be glad if we could ship routing definitions as part of the extension so they apply immediately after the extension is installed as many people using TYPO3 don't have the knowledge about routing (or read the docs)

Also available in: Atom PDF