From 613e2c994bc4c57d49dc22cceddfb187d5294542 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Pe=CC=81rez?= <jaime.perez@uninett.no> Date: Wed, 29 Jun 2016 16:20:19 +0200 Subject: [PATCH] Stop intercepting exceptions in www/module.php. The module.php file is the way we allow modules to have their own pages. All those are executed and presented to the user via this script. However, if an exception is thrown by a module, that exception will be captured directly by the module.php script. This prevents us from adding more logic to exception handling, adds code duplication, and makes the exception handling non-uniform, since we could end up handling the same exception differently depending on whether it was thrown by a module or by a regular page. Now we no longer intercept exceptions in module.php, allowing the exception handler to kick in. That way exceptions are always handled uniformly, and we can also implement additional logic that we may want (i.e. adding a hook to the exception handler so that modules could handle exceptions the way they want). --- www/module.php | 217 +++++++++++++++++++++++-------------------------- 1 file changed, 103 insertions(+), 114 deletions(-) diff --git a/www/module.php b/www/module.php index 03409fe72..cb6b64ec6 100644 --- a/www/module.php +++ b/www/module.php @@ -38,145 +38,134 @@ $mimeTypes = array( 'xhtml' => 'application/xhtml+xml', ); -try { - - if (empty($_SERVER['PATH_INFO'])) { - throw new SimpleSAML_Error_NotFound('No PATH_INFO to module.php'); - } +if (empty($_SERVER['PATH_INFO'])) { + throw new SimpleSAML_Error_NotFound('No PATH_INFO to module.php'); +} - $url = $_SERVER['PATH_INFO']; - assert('substr($url, 0, 1) === "/"'); +$url = $_SERVER['PATH_INFO']; +assert('substr($url, 0, 1) === "/"'); - /* clear the PATH_INFO option, so that a script can detect whether it is called with anything following the - *'.php'-ending. - */ - unset($_SERVER['PATH_INFO']); +/* clear the PATH_INFO option, so that a script can detect whether it is called with anything following the + *'.php'-ending. + */ +unset($_SERVER['PATH_INFO']); - $modEnd = strpos($url, '/', 1); - if ($modEnd === false) { - // the path must always be on the form /module/ - throw new SimpleSAML_Error_NotFound('The URL must at least contain a module name followed by a slash.'); - } +$modEnd = strpos($url, '/', 1); +if ($modEnd === false) { + // the path must always be on the form /module/ + throw new SimpleSAML_Error_NotFound('The URL must at least contain a module name followed by a slash.'); +} - $module = substr($url, 1, $modEnd - 1); - $url = substr($url, $modEnd + 1); - if ($url === false) { - $url = ''; - } +$module = substr($url, 1, $modEnd - 1); +$url = substr($url, $modEnd + 1); +if ($url === false) { + $url = ''; +} - if (!SimpleSAML\Module::isModuleEnabled($module)) { - throw new SimpleSAML_Error_NotFound('The module \''.$module.'\' was either not found, or wasn\'t enabled.'); - } +if (!SimpleSAML\Module::isModuleEnabled($module)) { + throw new SimpleSAML_Error_NotFound('The module \''.$module.'\' was either not found, or wasn\'t enabled.'); +} - /* Make sure that the request isn't suspicious (contains references to current directory or parent directory or - * anything like that. Searching for './' in the URL will detect both '../' and './'. Searching for '\' will detect - * attempts to use Windows-style paths. - */ - if (strpos($url, '\\') !== false) { - throw new SimpleSAML_Error_BadRequest('Requested URL contained a backslash.'); - } elseif (strpos($url, './') !== false) { - throw new SimpleSAML_Error_BadRequest('Requested URL contained \'./\'.'); - } +/* Make sure that the request isn't suspicious (contains references to current directory or parent directory or + * anything like that. Searching for './' in the URL will detect both '../' and './'. Searching for '\' will detect + * attempts to use Windows-style paths. + */ +if (strpos($url, '\\') !== false) { + throw new SimpleSAML_Error_BadRequest('Requested URL contained a backslash.'); +} elseif (strpos($url, './') !== false) { + throw new SimpleSAML_Error_BadRequest('Requested URL contained \'./\'.'); +} - $moduleDir = SimpleSAML\Module::getModuleDir($module).'/www/'; +$moduleDir = SimpleSAML\Module::getModuleDir($module).'/www/'; - // check for '.php/' in the path, the presence of which indicates that another php-script should handle the request - for ($phpPos = strpos($url, '.php/'); $phpPos !== false; $phpPos = strpos($url, '.php/', $phpPos + 1)) { +// check for '.php/' in the path, the presence of which indicates that another php-script should handle the request +for ($phpPos = strpos($url, '.php/'); $phpPos !== false; $phpPos = strpos($url, '.php/', $phpPos + 1)) { - $newURL = substr($url, 0, $phpPos + 4); - $param = substr($url, $phpPos + 4); + $newURL = substr($url, 0, $phpPos + 4); + $param = substr($url, $phpPos + 4); - if (is_file($moduleDir.$newURL)) { - /* $newPath points to a normal file. Point execution to that file, and - * save the remainder of the path in PATH_INFO. - */ - $url = $newURL; - $_SERVER['PATH_INFO'] = $param; - break; - } + if (is_file($moduleDir.$newURL)) { + /* $newPath points to a normal file. Point execution to that file, and + * save the remainder of the path in PATH_INFO. + */ + $url = $newURL; + $_SERVER['PATH_INFO'] = $param; + break; } +} - $path = $moduleDir.$url; +$path = $moduleDir.$url; - if ($path[strlen($path) - 1] === '/') { - // path ends with a slash - directory reference. Attempt to find index file in directory - foreach ($indexFiles as $if) { - if (file_exists($path.$if)) { - $path .= $if; - break; - } +if ($path[strlen($path) - 1] === '/') { + // path ends with a slash - directory reference. Attempt to find index file in directory + foreach ($indexFiles as $if) { + if (file_exists($path.$if)) { + $path .= $if; + break; } } +} - if (is_dir($path)) { - /* Path is a directory - maybe no index file was found in the previous step, or maybe the path didn't end with - * a slash. Either way, we don't do directory listings. - */ - throw new SimpleSAML_Error_NotFound('Directory listing not available.'); - } - - if (!file_exists($path)) { - // file not found - SimpleSAML\Logger::info('Could not find file \''.$path.'\'.'); - throw new SimpleSAML_Error_NotFound('The URL wasn\'t found in the module.'); - } +if (is_dir($path)) { + /* Path is a directory - maybe no index file was found in the previous step, or maybe the path didn't end with + * a slash. Either way, we don't do directory listings. + */ + throw new SimpleSAML_Error_NotFound('Directory listing not available.'); +} - if (preg_match('#\.php$#D', $path)) { - // PHP file - attempt to run it +if (!file_exists($path)) { + // file not found + SimpleSAML\Logger::info('Could not find file \''.$path.'\'.'); + throw new SimpleSAML_Error_NotFound('The URL wasn\'t found in the module.'); +} - /* In some environments, $_SERVER['SCRIPT_NAME'] is already set with $_SERVER['PATH_INFO']. Check for that case, - * and append script name only if necessary. - * - * Contributed by Travis Hegner. - */ - $script = "/$module/$url"; - if (stripos($_SERVER['SCRIPT_NAME'], $script) === false) { - $_SERVER['SCRIPT_NAME'] .= '/'.$module.'/'.$url; - } +if (preg_match('#\.php$#D', $path)) { + // PHP file - attempt to run it - require($path); - exit(); + /* In some environments, $_SERVER['SCRIPT_NAME'] is already set with $_SERVER['PATH_INFO']. Check for that case, + * and append script name only if necessary. + * + * Contributed by Travis Hegner. + */ + $script = "/$module/$url"; + if (stripos($_SERVER['SCRIPT_NAME'], $script) === false) { + $_SERVER['SCRIPT_NAME'] .= '/'.$module.'/'.$url; } - // some other file type - attempt to serve it + require($path); + exit(); +} + +// some other file type - attempt to serve it - // find MIME type for file, based on extension - $contentType = null; - if (preg_match('#\.([^/\.]+)$#D', $path, $type)) { - $type = strtolower($type[1]); - if (array_key_exists($type, $mimeTypes)) { - $contentType = $mimeTypes[$type]; - } +// find MIME type for file, based on extension +$contentType = null; +if (preg_match('#\.([^/\.]+)$#D', $path, $type)) { + $type = strtolower($type[1]); + if (array_key_exists($type, $mimeTypes)) { + $contentType = $mimeTypes[$type]; } +} - if ($contentType === null) { - /* We were unable to determine the MIME type from the file extension. Fall back to mime_content_type (if it - * exists). - */ - if (function_exists('mime_content_type')) { - $contentType = mime_content_type($path); - } else { - // mime_content_type doesn't exist. Return a default MIME type - SimpleSAML\Logger::warning('Unable to determine mime content type of file: '.$path); - $contentType = 'application/octet-stream'; - } +if ($contentType === null) { + /* We were unable to determine the MIME type from the file extension. Fall back to mime_content_type (if it + * exists). + */ + if (function_exists('mime_content_type')) { + $contentType = mime_content_type($path); + } else { + // mime_content_type doesn't exist. Return a default MIME type + SimpleSAML\Logger::warning('Unable to determine mime content type of file: '.$path); + $contentType = 'application/octet-stream'; } +} - $contentLength = sprintf('%u', filesize($path)); // force filesize to an unsigned number - - header('Content-Type: '.$contentType); - header('Content-Length: '.$contentLength); - header('Cache-Control: public,max-age=86400'); - header('Expires: '.gmdate('D, j M Y H:i:s \G\M\T', time() + 10 * 60)); - header('Last-Modified: '.gmdate('D, j M Y H:i:s \G\M\T', filemtime($path))); +$contentLength = sprintf('%u', filesize($path)); // force filesize to an unsigned number - readfile($path); - exit(); -} catch (SimpleSAML_Error_Error $e) { +header('Content-Type: '.$contentType); +header('Content-Length: '.$contentLength); +header('Cache-Control: public,max-age=86400'); +header('Expires: '.gmdate('D, j M Y H:i:s \G\M\T', time() + 10 * 60)); +header('Last-Modified: '.gmdate('D, j M Y H:i:s \G\M\T', filemtime($path))); - $e->show(); -} catch (Exception $e) { - - $e = new SimpleSAML_Error_Error('UNHANDLEDEXCEPTION', $e); - $e->show(); -} +readfile($path); -- GitLab