Bug #88178

Page cache is deleted, when an uncached (Extbase) Plugin performs a redirect

Added by Helmut Hummel 4 months ago. Updated 4 months ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Frontend
Target version:
-
Start date:
2019-04-19
Due date:
% Done:

100%

TYPO3 Version:
7
PHP Version:
Tags:
Complexity:
Is Regression:
Yes
Sprint Focus:

Description

There is a legitimate case that a plugin performs a redirect (sends Location header).
Usually such plugins are defined as uncached.
Uncached plugins are called, even when the rest of the page remains cached.

With the change applied for #83755 any redirect performed by such plugins, leads to the complete page cache being deleted.

On a side note (and the reason I ran into this), rendering frontend pages on CLI with the goal to warm up the cache is not possible any more after this change,
because http_response_code() always returns false when called from cli SAPI.


Related issues

Related to TYPO3 Core - Bug #83755: Extbase TYPO3\CMS\Extbase\Mvc\Controller\AbstractController->redirectToUri() – remove HTML redirect? New 2018-02-02

Associated revisions

Revision 6c38e760 (diff)
Added by Helmut Hummel 4 months ago

[BUGFIX] Do not remove page cache for redirects issued by plugins

This reverts commit 3139b5608c87e7a5c2a49aa48cc683654db71e39.
"[BUGFIX] Do not cache content with different status code than 200"

At the same time we convert a plugin action to uncached,
in case a redirect is issued.

Resolves: #88178
Releases: 8.7, 9.5, master
Change-Id: Icaa8038d1fa4bf1c74dfb505c2b8fc97963ca4be
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/60518
Tested-by: TYPO3com <>
Tested-by: Markus Klein <>
Tested-by: Georg Ringer <>
Reviewed-by: Oliver Klee <>
Reviewed-by: Andreas Fernandez <>
Reviewed-by: Susanne Moog <>
Reviewed-by: Markus Klein <>
Reviewed-by: Georg Ringer <>

Revision 5c60f470 (diff)
Added by Helmut Hummel 4 months ago

[BUGFIX] Do not remove page cache for redirects issued by plugins

This reverts commit 3139b5608c87e7a5c2a49aa48cc683654db71e39.
"[BUGFIX] Do not cache content with different status code than 200"

At the same time we convert a plugin action to uncached,
in case a redirect is issued.

Resolves: #88178
Releases: 8.7, 9.5, master
Change-Id: Icaa8038d1fa4bf1c74dfb505c2b8fc97963ca4be
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/60684
Tested-by: TYPO3com <>
Tested-by: Georg Ringer <>
Reviewed-by: Georg Ringer <>

Revision e855f9cf (diff)
Added by Helmut Hummel 4 months ago

[BUGFIX] Do not remove page cache for redirects issued by plugins

This reverts commit 3139b5608c87e7a5c2a49aa48cc683654db71e39.
"[BUGFIX] Do not cache content with different status code than 200"

At the same time we convert a plugin action to uncached,
in case a redirect is issued.

Resolves: #88178
Releases: 8.7, 9.5, master
Change-Id: Icaa8038d1fa4bf1c74dfb505c2b8fc97963ca4be
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/60687
Tested-by: TYPO3com <>
Tested-by: Benni Mack <>
Reviewed-by: Benni Mack <>

History

#1 Updated by Helmut Hummel 4 months ago

My conclusion would be: Any plugin that needs a redirect, should be marked as not cachable.

If there is a valid case for giving cached plugins the possiblity to perform a permanent redirect,
then this redirect should be stored in the cache and be handled right after the cache is retrieved.

#2 Updated by Gerrit Code Review 4 months 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/+/60518

#3 Updated by Helmut Hummel 4 months ago

  • Related to Bug #83755: Extbase TYPO3\CMS\Extbase\Mvc\Controller\AbstractController->redirectToUri() – remove HTML redirect? added

#4 Updated by Markus Klein 4 months ago

A usecase, which contradicts this change request.

Consider a cachable show action for a seminar date. This view can be fully cached, it mostly never changes.
In case the seminar date has to be cancelled, the state of the seminar date is changed and the respective cache entry is flushed.
The show action then issues a redirect to an overview page, in case the page for this seminar date is still accessed by someone.
The redirect is so to say permanent, but since the core does not store the HTTP headers with the cache content, this redirect response must not be cached.

#5 Updated by Gerrit Code Review 4 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/+/60518

#6 Updated by Helmut Hummel 4 months ago

Thanks for the use case. There would have been multiple ways to achieve desired behavior without introducing such core change, which seems pretty intrusive to me.

  1. Using HttpUtility::redirect (or throw new DirectResponseException in 9.5 and higher)
  2. Disabling the page cache using TSFE->set_no_cache (which is done e.g. in News extension)
  3. Throw a 404 and handle the 404 accordingly in a custom 404 handler
  4. Convert the plugin to USER_INT before issuing the redirect

I added the last to Extbase redierctToUri as I don't see a use case for issuing a redirect and at the same time have the same plugin action cached (which only works currently at cost of always rendering the complete page, writing and then again removing the page cache)

#7 Updated by Gerrit Code Review 4 months ago

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

#8 Updated by Helmut Hummel 4 months ago

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

#9 Updated by Gerrit Code Review 4 months ago

  • Status changed from Resolved to Under Review

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

#10 Updated by Helmut Hummel 4 months ago

  • Status changed from Under Review to Resolved

#11 Updated by Benni Mack 4 months ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF