Problem/Motivation
When a redirect is detected, the redirect entity is added to the response as a cacheable dependency. The response then may be cache by, for example, Dynamic Page Cache. When that redirect is changed or deleted, the response is invalidated by the entity cache tags of the redirect.
However, because of the possibility of having redirect chains, a new redirect that affects the response may be created but the response may still be served from the cache - sending the user to the wrong page.
Steps to reproduce
Proposed resolution
Add the redirect_list cache tag to redirection responses, or analyze the cases and, for example, load redirects that could be affected by adding a new one in their chain and invalidate the entity cache tags for those redirects.
Remaining tasks
Ideally, implement the more accurate solution.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | redirect.response-cache-tags-3525917-2.patch | 1.26 KB | krystalcode |
Issue fork redirect-3525917
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
krystalcode commentedImplementing the quick - but less efficient - solution for now.
Comment #3
alexpottInteresting bug and yeah clearing all redirects out of the cache when generating or deleting redirects does feel sub-optimal. I feel that to fix this bug properly we should be adding test coverage to ensure we've got the bug and any edge cases we can imagine covered. Also adding test coverage will make it easier to move to a better solution.
Comment #4
alexpottGoing to see if I can write a failing test.
Comment #6
alexpottI've found the bug here. It's because we're not adding all the redirects involved when processing a redirect chain - we're only adding the last one. Therefore when we update another redirect in the chain the cache is not correctly invalidated. Adding all the redirects in makes the system work as expected.
Comment #7
berdirAdded a comment on the phpstan thing we discussed.
Comment #8
alexpottThanks @berdir
Comment #9
berdiractually fails on cspell now.
Comment #10
alexpott@berdir yay! No regressions :)
Comment #12
berdirYes, point for you in that argument I guess ;)