Bug #78002

Require cHash for cached plugin actions in Extbase

Added by Stefan Froemken over 2 years ago. Updated 8 months ago.

Status:
Closed
Priority:
Must have
Assignee:
-
Category:
Caching
Target version:
Start date:
2016-09-20
Due date:
% Done:

100%

TYPO3 Version:
8
PHP Version:
7.0
Tags:
Complexity:
Is Regression:
No
Sprint Focus:

Description

As you have seen the discussion with Dmitry Dulepov at twitter reqCHash is only valid for cached plugins: pi_base and also extbase.

Kasper has documented it here:
https://github.com/TYPO3/TYPO3.CMS/blob/master/typo3/sysext/frontend/Classes/Plugin/AbstractPlugin.php#L250-L261

In TYPO3 4.5 there was no reqCHash in makeCacheHash. In TYPO3 4.7 it was introduced:
https://review.typo3.org/#/c/4704/

I wonder a bit, because Dmitry wrote "Looks ok to me", so it would be interessting to here why it's not OK today.

Stefan


Related issues

Related to TYPO3.Fluid - Feature #4704: Improve parsing exception messages New 2009-09-20
Related to TYPO3 Core - Bug #81297: Extbase record preview leads to 404 due to missing cHash Closed 2017-05-22
Related to TYPO3 Core - Bug #81293: cacheHash fails to be generated because of missing id although id is present in request-string Closed 2017-05-22
Precedes TYPO3 Core - Task #85942: Fix a typo and add an example for requireCHashArgumentForActionArguments Closed 2016-09-21 2016-09-21

Associated revisions

Revision 2769e508 (diff)
Added by Helmut Hummel over 2 years ago

[FEATURE] Enforce cHash argument for Extbase actions

TypoScriptFrontendController::reqCHash() is now called for Extbase
frontend plugin actions just like they were usually called for the
legacy AbstractPlugin.

This throws a 404, if plugin arguments are present, but cHash is not,
which would also happen, if the plugin arguments were added
to "cHashRequiredParameters" configuration.

This provides a more reliable page caching behavior
by default and with zero configuration for extension authors.

With the feature switch "requireCHashArgumentForActionArguments" this
behavior can be disabled, which could be useful, if all actions in a plugin
are uncached or one wants to manually control the cHash behavior.

Resolves: #78002
Releases: master
Change-Id: Ic6f71c3e5c8a94a0d422372a08c944aef5663f06
Reviewed-on: https://review.typo3.org/49976
Tested-by: TYPO3com <>
Reviewed-by: Oliver Hader <>
Tested-by: Oliver Hader <>
Reviewed-by: Susanne Moog <>
Tested-by: Susanne Moog <>

History

#1 Updated by Gerrit Code Review over 2 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/49976

#2 Updated by Dmitry Dulepov over 2 years ago

In TYPO3 4.5 there was no reqCHash in makeCacheHash. In TYPO3 4.7 it was introduced:

It was added because of doParametersRequireCacheHash. This is a new thing, it allows to set some parameters to always require cHash. This is for special applications, not for regular plugins. Do not remove that piece of code or you will break it.

See my comments in the pull request. It needs some changes.

#3 Updated by Gerrit Code Review over 2 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/49976

#4 Updated by Gerrit Code Review over 2 years ago

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

#5 Updated by Gerrit Code Review over 2 years ago

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

#6 Updated by Gerrit Code Review over 2 years ago

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

#7 Updated by Gerrit Code Review over 2 years ago

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

#8 Updated by Gerrit Code Review over 2 years ago

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

#9 Updated by Gerrit Code Review over 2 years ago

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

#10 Updated by Gerrit Code Review over 2 years ago

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

#11 Updated by Dmitry Dulepov over 2 years ago

  • Subject changed from Wrong position of reqCHash() to Require cHash for cached plugin actions in Extbase
  • Priority changed from Should have to Must have

#12 Updated by Gerrit Code Review over 2 years ago

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

#13 Updated by Gerrit Code Review over 2 years ago

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

#14 Updated by Gerrit Code Review over 2 years ago

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

#15 Updated by Gerrit Code Review over 2 years ago

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

#16 Updated by Gerrit Code Review over 2 years ago

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

#17 Updated by Gerrit Code Review over 2 years ago

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

#18 Updated by Gerrit Code Review over 2 years ago

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

#19 Updated by Benni Mack over 2 years ago

  • Target version changed from 8.4 to 8.5

#20 Updated by Gerrit Code Review over 2 years ago

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

#21 Updated by Gerrit Code Review over 2 years ago

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

#22 Updated by Gerrit Code Review over 2 years ago

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

#23 Updated by Anonymous over 2 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#24 Updated by Dmitry Dulepov about 2 years ago

The change will add lots of identical page cache entries for every non-cached action. Very inefficient for the cache and database space in general.

Also I wonder how this works for method="get" and forms. I think it does not.

Anyway, it is already done, so nothing to do now...

#25 Updated by Mathias Brodala almost 2 years ago

  • Related to Bug #81297: Extbase record preview leads to 404 due to missing cHash added

#26 Updated by Markus Klein almost 2 years ago

  • Related to Bug #81293: cacheHash fails to be generated because of missing id although id is present in request-string added

#27 Updated by Lukas Weiss over 1 year ago

I don't know where to report this, but there's an error in the documentation:
https://docs.typo3.org/typo3cms/extensions/core/8.7/Changelog/8.5/Breaking-78002-EnforceCHashArgumentForExtbaseActions.html

feature.requireCHashArgumentForActionArguments

should be

features.requireCHashArgumentForActionArguments

#28 Updated by Riccardo De Contardi over 1 year ago

  • Status changed from Resolved to Closed

#29 Updated by Stephan Großberndt 8 months ago

The documentation error was fixed in https://review.typo3.org/#/c/58002/

#30 Updated by Stephan Großberndt 8 months ago

  • Precedes Task #85942: Fix a typo and add an example for requireCHashArgumentForActionArguments added

Also available in: Atom PDF