Bug #78002

Require cHash for cached plugin actions in Extbase

Added by Stefan Froemken over 4 years ago. Updated over 2 years ago.

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

100%

Estimated time:
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 messagesNew2009-09-20

Actions
Related to TYPO3 Core - Bug #81297: Extbase record preview leads to 404 due to missing cHashClosed2017-05-22

Actions
Related to TYPO3 Core - Bug #81293: cacheHash fails to be generated because of missing id although id is present in request-stringClosedDavid Bascom2017-05-22

Actions
Precedes TYPO3 Core - Task #85942: Fix a typo and add an example for requireCHashArgumentForActionArgumentsClosed2016-09-212016-09-21

Actions
#1

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

#2

Updated by Dmitry Dulepov over 4 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 4 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 4 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 4 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 4 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 4 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 4 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 4 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 4 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 4 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 4 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 4 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 4 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 4 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 4 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 4 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 4 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 4 years ago

  • Target version changed from 8.4 to 8.5
#20

Updated by Gerrit Code Review over 4 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 4 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 4 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 4 years ago

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

Updated by Dmitry Dulepov about 4 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 4 years ago

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

Updated by Markus Klein almost 4 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 3 years 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 3 years ago

  • Status changed from Resolved to Closed
#29

Updated by Stephan Großberndt over 2 years ago

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

#30

Updated by Stephan Großberndt over 2 years ago

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

Also available in: Atom PDF