Feature #93100
closedExtbasePluginEnhancer produces unnecessary cHash
Added by Kevin Ditscheid almost 4 years ago. Updated 16 days ago.
100%
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.
Updated by Gerrit Code Review almost 4 years 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
Updated by Jonas Eberle almost 4 years ago
Could you add an example on how to test? It doesn't seem to happen in all circumstances, does it?
Updated by B. Kausch almost 4 years 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...
Updated by Jonas Eberle almost 4 years 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?
Updated by Michael Stopp over 3 years 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.
Updated by Gerrit Code Review about 3 years 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
Updated by Oliver Hader about 3 years 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
- create new aspect mapper (example https://github.com/TYPO3/typo3/blob/master/typo3/sysext/core/Classes/Routing/Aspect/StaticValueMapper.php)
- that static(!) mapper shall ensure that the first 15 characters in
editCode
have a valid signature
// !!! 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; }
Updated by Oliver Hader about 3 years ago
- Status changed from Under Review to Needs Feedback
- Tags set to pending-close
Updated by Oliver Hader almost 3 years 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}'
Updated by Oliver Hader over 2 years ago
- Tracker changed from Bug to Feature
- TYPO3 Version deleted (
10) - Tags deleted (
pending-close)
Updated by Gerrit Code Review over 2 years 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
Updated by Oliver Hader almost 2 years ago
- Related to Bug #92355: Routing always generates cHash if there are no aspects added
Updated by Gerrit Code Review almost 2 years ago
Patch set 2 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
Updated by Gerrit Code Review 8 months ago
Patch set 3 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
Updated by Gerrit Code Review 8 months ago
Patch set 4 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
Updated by Gerrit Code Review 8 months ago
Patch set 5 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
Updated by Gerrit Code Review 8 months ago
Patch set 6 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
Updated by Oliver Hader 6 months ago
- Related to Bug #103844: URL generation broken added
Updated by Gerrit Code Review 3 months ago
Patch set 7 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
Updated by Gerrit Code Review 3 months ago
Patch set 8 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
Updated by Gerrit Code Review 3 months ago
Patch set 9 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
Updated by Oliver Hader 3 months ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 973015bc20f5c9ba7189a9b693fc2cdc8de2df97.