Feature #93100

ExtbasePluginEnhancer produces unnecessary cHash

Added by Kevin Ditscheid over 1 year ago. Updated about 2 months ago.

Status:
Under Review
Priority:
Should have
Assignee:
-
Category:
Link Handling, Site Handling & Routing
Target version:
-
Start date:
2020-12-17
Due date:
% Done:

0%

Estimated time:
PHP Version:
Tags:
Complexity:
Sprint Focus:

Description

The ExtbasePluginEnhancer produces a cHash parameter, besides that it is not needed, because all parameters have been mapped.
I think the issue lies in the inflateParameters function, that is setting an empty namespace parameter before the check, if the enhancer is needed at all, leading to a garbage value that messes up the remainingQueryParameters check later on.

#1

Updated by Gerrit Code Review over 1 year ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67176

#2

Updated by Jonas Eberle over 1 year ago

Could you add an example on how to test? It doesn't seem to happen in all circumstances, does it?

#3

Updated by B. Kausch over 1 year ago

I would like to see that fixed soon. These hashes does not look good in a prod environment ;)

Here is an example for an routeEnhancer, which will produce a cHash:

routeEnhancers:
  EventForm:
    type: Extbase
    extension: SomeExtension
    plugin: Eventform
    routes:
      -
        routePath: '/edit/{editCode}'
        _controller: 'Event::edit'
        _arguments: {'editCode': 'editCode'}
    defaultController: 'Event::new'
    requirements:
      editCode: '[A-Z0-9]{15}'

When I build an Uri for the edit action with a correct editCode, an cHash is prepended. I don't see any reason why...

#4

Updated by Jonas Eberle over 1 year ago

B. Kausch wrote in #note-3:

I would like to see that fixed soon. These hashes does not look good in a prod environment ;)

You could help by testing the patch linked in the first post.

When I build an Uri for the edit action with a correct editCode, an cHash is prepended. I don't see any reason why...

Maybe this is to be expected if the value space is as big as yours (34^15). There is a limit somewhere of 10000, at least for the StaticRangeMapper (https://github.com/TYPO3/TYPO3.CMS/blob/master/typo3/sysext/core/Classes/Routing/PageRouter.php#L566) - does it apply here, too?

#5

Updated by Michael Stopp about 1 year ago

Maybe this is to be expected if the value space is as big as yours (34^15). There is a limit somewhere of 10000, at least for the StaticRangeMapper (https://github.com/TYPO3/TYPO3.CMS/blob/master/typo3/sysext/core/Classes/Routing/PageRouter.php#L566) - does it apply here, too?

Well, does it? Can someone with in-depth knowledge shed light on this? Because if it were true, it would explain many similar cases I had, where I just couldn't get rid of the cHash parameter. For example if you have a personalized token in the URL, it is by definition unique and adding an extra cHash just makes the URL ugly.

I always found the official documentation lacking in specificity:

If the requirements are too loose, a URL signature parameter (“cHash”) is added to the end of the URL which cannot be removed.

Ok, but when are the requirements "too loose"? If the 10000 limit applies here as well, it would be a very helpful information and should be mentioned in the documentation. And if not: there must be some criteria that defines, whether a cHash gets added or not.

#6

Updated by Gerrit Code Review 7 months ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67176

#7

Updated by Oliver Hader 7 months ago

Can those having problems with it please share their configuration (partially available already) AND some examples of how links are build, what are the input parameters/values? Only this way it would be possible to see how it's used and understand the problem. Thanks.

Besides that, the configuration provided at https://forge.typo3.org/issues/93100#note-3 does not contain any "static aspect mapper" (see https://docs.typo3.org/m/typo3/reference-coreapi/master/en-us/ApiOverview/Routing/AdvancedRoutingConfiguration.html#aspects) to avoid cache-flooding - for instance [A-Z0-9]{15} is 36^15 combinations and possible cache entries for that - which is a lot more than default limit of 10,000. In order to avoid cache-flooding, the cHash parameter is added.

As an alternative I see two possibilities - given that editCode is somehow a random (PRNG) value:

a) Find a way to ensure editCode value has been generated by the system (and was not injected from the outside)

+ actual editCode:       0123456789abcde
+ extend with signature: 691e962069233fe1cbaf1618e49fb287f1f001a4
+ new signed editCode:   0123456789abcde691e962069233fe1cbaf1618e49fb287f1f001a4
// !!! UNTESTED SOURCE CODE !!!
class MyEditCodeMapper implements StaticMappableAspectInterface, \Countable
{
  public function __construct(array $settings) {}

  public function count(): int
  {
    return 1; // use this with care and only if your application can verify the parameter signature!!!
  }

  public function generate(string $value): ?string
  {
    // appends HMAC-SHA1 to given plain value
    // when generation a route
    return $value . GeneralUtility::hmac($value, __CLASS__);
  }

  public function resolve(string $value): ?string
  {
     if (!preg_match('#^(.+{15})(.+{40})$#', $value, $matches) {
       // did not match expected format (15 chars + 40 chars signature)
       return null;
     }
     $expectedHash = GeneralUtility::hmac($matches[1], __CLASS__);
     if ($expectedHash === $matches[2]) {
       // signature is valid, return/extract first 15 characters (the initial payload)
       return $matches[1];
     }
     // signature did not match, return nothing (`null`)
     return null;
  }
}

b) disable cache & use custom mapper

  • not the suggested way, but simpler
  • still: it's dirty, a hack, a work-around, you don't win any price with that, it's ugly
  • disable cache for page holding the application (the plugin)
  • use custom static aspect mapper (like above) that just pass the values (and tricks the system)
  • public function count(): int { return 1; } (this disables the cache-flooding protection mechanism - be aware of that)
  • public function generate(string $value): ?string { return $value; }
  • public function resolve(string $value): ?string { return $value; }
#8

Updated by Oliver Hader 7 months ago

  • Status changed from Under Review to Needs Feedback
  • Tags set to pending-close
#9

Updated by Oliver Hader 4 months ago

Just came to this issue again... it seems this actual can be a feature, that allows to declare a particular route variable is "static".

e.g.

...
routes:
 - routePath: '/edit/{editCode}'
   _controller: 'Event::edit'
   _arguments: {'editCode': 'editCode'}

# in case requirement for a particular variable is defined,
# allow to declare it as "static" as well...
# (avoiding to use a superfluous mapper for that)
isStatic:
  editCode: true
requirements:
  editCode: '[A-Z0-9]{15}'
#10

Updated by Oliver Hader 2 months ago

  • Tracker changed from Bug to Feature
  • TYPO3 Version deleted (10)
  • Tags deleted (pending-close)
#11

Updated by Gerrit Code Review about 2 months ago

  • Status changed from Needs Feedback to Under Review

Patch set 1 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74016

Also available in: Atom PDF