Project

General

Profile

Actions

Bug #103844

closed

URL generation broken

Added by Robert Vock about 2 months ago. Updated about 1 month ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Link Handling, Site Handling & Routing
Target version:
-
Start date:
2024-05-15
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
11
PHP Version:
Tags:
Complexity:
Is Regression:
Yes
Sprint Focus:

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 :(


Related issues 2 (1 open1 closed)

Related to TYPO3 Core - Bug #103400: Avoid mapping route values that are out of scopeResolved2024-03-14

Actions
Related to TYPO3 Core - Feature #93100: ExtbasePluginEnhancer produces unnecessary cHashUnder ReviewOliver Hader2020-12-17

Actions
Actions #1

Updated by Robert Vock about 2 months ago

  • Related to Bug #103400: Avoid mapping route values that are out of scope added
Actions #2

Updated by Oliver Hader about 2 months ago

  • Related to Feature #93100: ExtbasePluginEnhancer produces unnecessary cHash added
Actions #3

Updated by Oliver Hader about 2 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;
Actions #4

Updated by Robert Vock about 2 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;
    }
}
Actions #5

Updated by Oliver Hader about 2 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?

Actions #6

Updated by Robert Vock about 1 month 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 :)

Actions #7

Updated by Christian Kuhn about 1 month ago

  • Status changed from Needs Feedback to Closed
Actions

Also available in: Atom PDF