Bug #103844
closedURL generation broken
0%
Description
The last security update changed how URLs are generated and broke some URLs.
We have URLs, which use the PersistedPatternMapper. This mapper would generate a URL with the given parameter, even if the generate-Method could not find the object. This is now prevented with the fix for #103400
Example Routing-Configuration (simplified)¶
Dealer:
type: Extbase
extension: Example
plugin: dealer
defaultController: 'Dealer::index'
routes:
-
routePath: '/dealer/{dealer}'
_controller: 'Dealer::show'
_arguments:
dealer: dealer
requirements:
dealer: '(.*-)?[0-9]+'
aspects:
dealer:
type: PersistedPatternMapper
tableName: tx_example_domain_model_dealer
routeFieldPattern: '^(?P<uid>\d+)$'
routeFieldResult: '{uid}'
The generated URLs look like this: www.example.com/dealer/123
We have an API endpoint, which generates links for 6000 dealers. If we would generate those links using the TYPO3 API, we would have multiple SQL Queries for each dealer. To solve this issue, we only generate a "fake" URL and replace the Path-Part:
$baseLink = $this->getTSFE()->cObj->typoLink_URL([
'parameter' => 't3://page?uid=' . $dealerPage,
'additionalParams' => GeneralUtility::implodeArrayForUrl('', [
'tx_vierwdbosch_dealer' => [
'action' => 'show',
'dealer' => '0',
],
]),
]);
This will generate a URL www.example.com/dealer/123
Now we can generate all URLs with a simple string-replace:
foreach ($dealers as $dealer) {
$dealer['link'] = str_replace('/0', '/' . $dealer['uid'], $baseLink);
}
This does not work anymore, because the initial URL will not be correctly generated. The dealer with ID 0 does not exist and the URL will look like this:
www.example.com?tx_example_dealer%5Baction%5D=show&tx_example_dealer%5Bdealer%5D=0&cHash=0875exxxxxx
Generating the URLs for all 6000 dealers takes 7 Seconds. Using one fake link and replacing the slug with string replace takes 70ms.
Another use-case was generating a link with a slug when we only had the slug and not the object.
We had an API change and many URLs changed, because the underlying DB was changed. We know that the OLD slugs should be mapped to new URLs. We have a catchallAction for this.
defaultController: 'SelfService::index'
routes:
-
routePath: '/asset/{asset}'
_controller: 'Asset::show'
_arguments:
asset: asset
-
routePath: '/{matchAny}'
_controller: 'SelfService::catchall'
_arguments:
matchAny: path
requirements:
asset: 'asset-.*'
matchAny: '.*'
public function catchallAction(string $path): ResponseInterface {
if (preg_match('/-e(\d+)$/', $path, $matches)) {
$asset = 'asset-' . str_pad($matches[1], 5, '0', STR_PAD_LEFT);
$this->redirect('show', 'Asset', null, ['asset' => $asset], null, null, 301);
}
...
}
URLs in the form of /random-prefix-e123
are redirected to /asset/asset-00123
This does not work anymore, because the URL for the redirect is not generated correctly anymore.
I think to fix this issue, we would need to load and validate the asset in the catchall action to get the correct object and use this in the URL generation.
It's possible that this is as intended, but this is still a large breaking change for us and prevents us from installing the security update :(
Updated by Robert Vock 6 months ago
- Related to Bug #103400: Avoid mapping route values that are out of scope added
Updated by Oliver Hader 6 months ago
- Related to Feature #93100: ExtbasePluginEnhancer produces unnecessary cHash added
Updated by Oliver Hader 6 months ago · Edited
Issue #103400 fixed the bug of creating resources that actually did not exist and would have lead to a HTTP 404 error when being used.
As a feature it might be possible to consider having something like generatorFallbackValue=0
– in any way, allowing URIs for non-existing resources is the edge case and should not be the default behavior.
As a work-around for now, it would be possible and use a custom mapper which e.g. overrides \TYPO3\CMS\Core\Routing\Aspect\PersistedPatternMapper::generate
(I guess that the examples shown above shall use the persistence lookup when resolving URIs) and returns the desired value, e.g.
<?php namespace Vendor\Package; class MyMapper extends \TYPO3\CMS\Core\Routing\Aspect\PersistedPatternMapper { public function generate(string $value): ?string { // this generates URIs for any given value // and by-passes the persistence lookup completely return $value; } }
To be able to use that custom mapper in the YAML config as MyMapper
, it needs to be registered like this:
$GLOBALS['TYPO3_CONF_VARS']['SYS']['routing']['aspects']['MyMapper'] = \Vendor\Package\MyMapper;
Updated by Robert Vock 6 months ago
Thanks for your answer.
Yes, I believe as well that generating URLs for non-existing resources should not be allowed by default.
I like the approach with having a special mapper which returns the original value. I would still call the original PersistedPatternMapper, if a numeric value was passed:
<?php
namespace Vendor\Package;
class MyMapper extends \TYPO3\CMS\Core\Routing\Aspect\PersistedPatternMapper
{
public function generate(string $value): ?string
{
if (is_numeric($value)) {
$slug = parent::generate($value);
if ($slug) {
return $slug;
}
}
// this generates URIs for any given value
// and by-passes the persistence lookup completely
return $value;
}
}
Updated by Oliver Hader 6 months ago · Edited
- Status changed from New to Needs Feedback
Cool, that you seemed to like the idea of using a custom mapper, instead! ;-)
I guess we can close this ticket, correct?
Updated by Robert Vock 6 months ago
I'm not sure, if there should be an additional option for the default mapper. Or if the behaviour should be documented in the routing configuration:
https://docs.typo3.org/m/typo3/reference-coreapi/main/en-us/ApiOverview/Routing/AdvancedRoutingConfiguration.html
But you can close this ticket. I am using the custom mapper as a solution for our use-case :)
Updated by Christian Kuhn 6 months ago
- Status changed from Needs Feedback to Closed