Voting starts in March for the Drupal Association Board election.
f you try and add a redirect to an existing URL alias, the redirect module correctly prevents you, with the message: "You are attempting to redirect the page to itself. This will result in an infinite loop."
However, nothing prevents you from editing an entity and setting its URL alias to be the same as an existing redirect's source for that entity. Since 7.0-1.0-rc1 of redirect, this causes the following Drupal message when you then view the page:
"Oops, looks like this request tried to create an infinite loop. We do not allow such things here. We are a professional website!"
This situation is quite common in the wild - a user merely needs to change the URL of a page (causing a redirect to be added automatically), then either change their mind and set it back, or turn pathauto's Generate automatic URL alias back on and Pathauto will do that for them.
Steps to Reproduce
The following steps will reproduce the problem from scratch:
- Create and save a test node and set its URL alias to "test" under URL Path Settings.
- Edit your node and in the "URL Redirects" vertical tab press Add URL redirect to this node and
fill in "test2" as the From field, then hit Save. You will be returned to the node edit form.
- Now visit the URL path settings vertical tab and change the URL alias to also be "test2" and hit Save.
- Your node will be loaded at path /test2 but you will see the error message: Oops, looks like this request tried to create an infinite loop. We do not allow such things here. We are a professional website!
I propose we fix this problem by having Redirect implement hook_entity_update(). From there we can automatically delete duplicate redirect rows and prevent the circular redirect from occurring. This automated behaviour is similar to the automatic alias added to old node and term paths, and would involve minimal new code, not impact existing code, and involve no UI changes.
We should also properly remove old duplicate redirect rows in a Database update, using redirect_delete_multiple().
- Determine best approach for both preventing entity URL aliases from duplicating Redirect sources.
- Determine best approach for fixing existing bad data (or whether to leave that as a manual process).
- Create patch combining the above.
- Testing, including with multi-language data and multiple url_alias entries for a path.
Add proper test coverage to the Simpletest at RedirectFunctionalTest::testPathChangeRedirects(). Currently that test doesn't appear to be doing any asserting, but it looks to me like it's performing the exact steps that would cause this error so it would be easy to add test coverage for this bug by asserting that the Oops!... error message was not shown after the submits.
Edit: Test coverage here: redirect-detect_prevent_circular_redirects_test_only-1796596-18.patch from #18 of [#1796596] .
User interface changes
This issue has been forked from, which aims to detect circular redirects in progress and stop them, and originally that issue was forked from which discusses whether to roll back the new flood protection features in rc1. This is not a duplicate of either of those issues, but involved in preventing the problem from happening in future, and fixing the bad data from when it has happened in the past.