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.

Issue fork redirect-3525917

Command icon 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

krystalcode created an issue. See original summary.

krystalcode’s picture

Status: Active » Needs review
StatusFileSize
new1.26 KB

Implementing the quick - but less efficient - solution for now.

alexpott’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Interesting 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.

alexpott’s picture

Going to see if I can write a failing test.

alexpott’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests

I'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.

berdir’s picture

Status: Needs review » Needs work

Added a comment on the phpstan thing we discussed.

alexpott’s picture

Status: Needs work » Needs review

Thanks @berdir

berdir’s picture

Status: Needs review » Needs work

actually fails on cspell now.

alexpott’s picture

Status: Needs work » Needs review

@berdir yay! No regressions :)

  • berdir committed b38998f8 on 8.x-1.x authored by alexpott
    Issue #3525917 by alexpott, krystalcode: Invalidate cached responses...
berdir’s picture

Status: Needs review » Fixed

Yes, point for you in that argument I guess ;)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.