Possible to cause endless recursion in PagePathHistory module
Short description of the issue
With particular combinations of redirect URLs, entered through the ProcessWire admin interface, it's possible to send the PagePathHistory module into an 'infinite loop' or endless recursion until it uses up all memory. The effect of this is that a user clicking on "What other URLs redirect to this page?" on the settings tab will just get the spinning icon and no results. Also calling $page->urls() from the API will not complete – the script will use memory until killed by the operating system.
Expected behavior
Ideally the user should be prevented from entering a redirect URL which will cause this issue, but also the code should have some facility for recognising the endless recursion and bailing out.
Actual behavior
getVirtualHistory() method is called repeatedly, alternating between 2 page ids, until memory is exhausted. It's necessary to delete the problem paths from the page_path_history history table in MySQL to restore proper functioning.
Steps to reproduce the issue
An example that caused this problem for us was when we had 2 pages like the following:
Page 1 URL: /one/two/three/ Old/redirect URL: /one/four/
Page 2 URL: /one/two/ Old/redirect URL: /one/four/five/
Setup/Environment
- ProcessWire version: 3.0.226
Thanks @16th-earl I've pushed a fix for this. One thing I'm not yet certain one is whether the contents "What URLs should redirect to this page" is right, or if it should exclude some of the URLs it currently shows for the /one/two/ and /one/two/three/ pages? It creates a bit of a conundrum when pages have swapped their parent/child relationships in this manner. If you want to try it with the extra URLs excluded, uncomment this section. I'd be curious what you find to be correct for an actual site, as I'm just testing with the /one/two/three/ theoretical examples where it's a little hard to tell.
@16th-earl could you verify the fix and provide more info for Ryan?
G'day – sorry for the delay. The fix works, thanks! No crash anymore.
I think overall it would be better to uncomment the block of code to exclude the extra pages, because otherwise there end up being extra redirect URLs that we never intended to have. I haven't found anywhere on our sites where excluding these extra URLs would cause a problem (by excluding too many things).
Either way, the fix allows us to sort out any circular references from the administrator interface if we need to. So that's great, thanks.