Bugfix: Infinite recursion sanity checks with more helpful error messages

[nathanbrauer]

Issue #213

The following snippet in ShareDraftController.php at L161 includes no sanity checks to prevent infinite recursions when it attempts to follow redirects internally via recursions.

silverstripe-sharedraftcontent/src/Controllers/ShareDraftController.php

Lines 137 to 165 in 7c3f7e7

	private function getRenderedPageByURL(string $url): HTTPResponse
	{
	// Clean and update live global variables. This is how
	// HTTPRequestBuilder::createFromEnvironment works internally.
	$variables = HTTPRequestBuilder::cleanEnvironment(Environment::getVariables());
	$variables['_SERVER']['REQUEST_URI'] = $url;
	$variables['_SERVER']['REQUEST_METHOD'] = 'GET';
	$variables['_SERVER']['HTTP_USER_AGENT'] =
	isset($variables['_SERVER']['HTTP_USER_AGENT']) &&
	$variables['_SERVER']['HTTP_USER_AGENT']
	? $variables['_SERVER']['HTTP_USER_AGENT']
	: 'CLI';
	Environment::setVariables($variables);
	// Health-check prior to creating environment
	$pageRequest = HTTPRequestBuilder::createFromVariables($variables, @file_get_contents('php://input'));
	$response = Director::singleton()->handleRequest($pageRequest);
	if ($response->isRedirect()) {
	// The redirect will probably be Absolute URL so just want the path
	$newUrl = parse_url($response->getHeader('location') ?? '', PHP_URL_PATH);
	return $this->getRenderedPageByURL($newUrl);
	}
	return $response;
	}

Infinite recursions, such as those experienced in #213, would previously cause fatal time out or memory limit errors.

This pull request resolves this problem by adding sanity checks, preventing unanticipated internal infinite recursions via "logging" a minimal redirect stack to a private array on ShareDraftController and throws an Exception (with helpful information for developers) if....

1. ...a URL has already been processed for redirection (infinite recursion detected), OR
2. ...30 redirects have been logged
   (30 is an arbitrary number that seems to be more than what is typically necessary; this could, in a separate PR, be pulled into a config variable if desired)

Examples of Exception outputs

Infinite recursion detected

Max redirect recursions reached

Related

• Bug: Nginx/PHP-FSM environments subject to infinite recursions (internal redirects) #213

Info

• Edits for maintainers are enabled
• Set to merge into 2 and can be rebased onto 3 without issue

---

@muskie9

[nathanbrauer]

Extracted some duplicate code to satisfy previously failed workflow

[emteknetnz]

I've merged this, though I noticed that it was targetting the 2 branch instead of the 2.9 branch, sorry I should have picked up as part of peer review

I've created a backport PR to get this into 2.9 where I've cherry-picked the squahed merge commit. Once that is merged it will be automatically tagged as 2.9.4

[nathanbrauer]

Thanks for all your work to get this across the finish line, @emteknetnz!