From e49037024c5a92ddb76ee59d4ca90e2fb84327af Mon Sep 17 00:00:00 2001 From: "Nathan J. Brauer" Date: Thu, 9 Nov 2023 14:53:21 -0800 Subject: [PATCH 1/2] FIX Infinite recursion sanity checks with more helpful error messages (#214) * Infinite recursion protections Adds sanity checks to prevent unanticipated internal infinite recursions (now throws Exceptions with helpful messages; previously timed out or hit memory limits) * Accepting coding standards Co-authored-by: Steve Boyd * MNT Linting --------- Co-authored-by: Steve Boyd --- src/Controllers/ShareDraftController.php | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/Controllers/ShareDraftController.php b/src/Controllers/ShareDraftController.php index 3ea1914..883a228 100644 --- a/src/Controllers/ShareDraftController.php +++ b/src/Controllers/ShareDraftController.php @@ -52,6 +52,8 @@ class ShareDraftController extends Controller */ protected static $isViewingPreview = false; + private array $redirectRecursionIterations = []; + /** * @return bool */ @@ -172,8 +174,8 @@ private function getRenderedPageByURL(string $url): HTTPResponse $variables['_SERVER']['HTTP_USER_AGENT'] = isset($variables['_SERVER']['HTTP_USER_AGENT']) && $variables['_SERVER']['HTTP_USER_AGENT'] - ? $variables['_SERVER']['HTTP_USER_AGENT'] - : 'CLI'; + ? $variables['_SERVER']['HTTP_USER_AGENT'] + : 'CLI'; Environment::setVariables($variables); @@ -183,6 +185,15 @@ private function getRenderedPageByURL(string $url): HTTPResponse $response = Director::singleton()->handleRequest($pageRequest); if ($response->isRedirect()) { + if (in_array($url, $this->redirectRecursionIterations)) { + throw new \Exception("Infinite recursion detected." . $this->getRedirectRecursionIterationsLog($url)); + } + + $this->redirectRecursionIterations[] = $url; + if (count($this->redirectRecursionIterations) >= 30) { + throw new \Exception("Max redirect recursions reached." . $this->getRedirectRecursionIterationsLog()); + } + // The redirect will probably be Absolute URL so just want the path $newUrl = parse_url($response->getHeader('location') ?? '', PHP_URL_PATH); @@ -192,6 +203,13 @@ private function getRenderedPageByURL(string $url): HTTPResponse return $response; } + private function getRedirectRecursionIterationsLog(string $appendUrl = ''): string + { + return "\n\nRedirected URLs stack: \n" + . implode("\n", $this->redirectRecursionIterations) + . ($appendUrl ? "\n$appendUrl" : ''); + } + /** * @return DBHTMLText */ From 58a795e50a940c29bbdd8761ce50c010830f79ec Mon Sep 17 00:00:00 2001 From: "Nathan J. Brauer" Date: Sun, 15 Oct 2023 17:50:19 -0700 Subject: [PATCH 2/2] Fixes internal infinite recursion on Nginx/PHP-FPM environments --- src/Controllers/ShareDraftController.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Controllers/ShareDraftController.php b/src/Controllers/ShareDraftController.php index 883a228..51870de 100644 --- a/src/Controllers/ShareDraftController.php +++ b/src/Controllers/ShareDraftController.php @@ -179,6 +179,11 @@ private function getRenderedPageByURL(string $url): HTTPResponse Environment::setVariables($variables); + // This extra step required for PHP-FPM/Nginx implementations + Environment::setEnv('REQUEST_URI', $url); + Environment::setEnv('REQUEST_METHOD', 'GET'); + Environment::setEnv('HTTP_USER_AGENT', $variables['_SERVER']['HTTP_USER_AGENT']); + // Health-check prior to creating environment $pageRequest = HTTPRequestBuilder::createFromVariables($variables, @file_get_contents('php://input'));