From 7b3f6cb219cac448ba470f016ed5068bdc7ffc56 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sun, 18 Sep 2011 00:55:24 +0200 Subject: [PATCH 1/5] [ticket/10369] Always include errfile and errline in format_errors(). We remove the phpBB root path from errfile. This is consistent with how msg_handler handles E_WARNING messages etc. PHPBB3-10369 --- phpBB/includes/error_collector.php | 14 +++++++----- tests/error_collector_test.php | 35 ++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 tests/error_collector_test.php diff --git a/phpBB/includes/error_collector.php b/phpBB/includes/error_collector.php index 55834f354c..534df27ece 100644 --- a/phpBB/includes/error_collector.php +++ b/phpBB/includes/error_collector.php @@ -42,6 +42,8 @@ class phpbb_error_collector function format_errors() { + $phpbb_root_path = phpbb_realpath(dirname(__FILE__) . '/../'); + $text = ''; foreach ($this->errors as $error) { @@ -49,13 +51,15 @@ class phpbb_error_collector { $text .= "
\n"; } + list($errno, $msg_text, $errfile, $errline) = $error; - $text .= "Errno $errno: $msg_text"; - if (defined('DEBUG_EXTRA') || defined('IN_INSTALL')) - { - $text .= " at $errfile line $errline"; - } + + // Prevent leakage of local path to phpBB install + $errfile = str_replace(array($phpbb_root_path, '\\'), array('', '/'), $errfile); + + $text .= "Errno $errno: $msg_text at $errfile line $errline"; } + return $text; } } diff --git a/tests/error_collector_test.php b/tests/error_collector_test.php new file mode 100644 index 0000000000..e1ac32f5ac --- /dev/null +++ b/tests/error_collector_test.php @@ -0,0 +1,35 @@ +install(); + + // Cause a warning + 1/0; $line = __LINE__; + + $collector->uninstall(); + + list($errno, $msg_text, $errfile, $errline) = $collector->errors[0]; + $error_contents = $collector->format_errors(); + + $this->assertEquals($errno, 2); + + // Unfortunately $error_contents will contain the full path here, + // because the tests directory is outside of phpbb root path. + $this->assertStringStartsWith('Errno 2: Division by zero at ', $error_contents); + $this->assertStringEndsWith(" line $line", $error_contents); + } +} From 9006984d5ab7478e9187d14c5ab331057b1af9a4 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sun, 18 Sep 2011 22:20:20 +0200 Subject: [PATCH 2/5] [ticket/10369] DRY code to remove phpbb path from errfile. PHPBB3-10369 --- phpBB/includes/error_collector.php | 4 +--- phpBB/includes/functions.php | 26 +++++++++++++++++++++++--- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/phpBB/includes/error_collector.php b/phpBB/includes/error_collector.php index 534df27ece..040be4dd13 100644 --- a/phpBB/includes/error_collector.php +++ b/phpBB/includes/error_collector.php @@ -42,8 +42,6 @@ class phpbb_error_collector function format_errors() { - $phpbb_root_path = phpbb_realpath(dirname(__FILE__) . '/../'); - $text = ''; foreach ($this->errors as $error) { @@ -55,7 +53,7 @@ class phpbb_error_collector list($errno, $msg_text, $errfile, $errline) = $error; // Prevent leakage of local path to phpBB install - $errfile = str_replace(array($phpbb_root_path, '\\'), array('', '/'), $errfile); + $errfile = phpbb_filter_errfile($errfile); $text .= "Errno $errno: $msg_text at $errfile line $errline"; } diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 628f8ee123..4c1bfb4360 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -3816,9 +3816,8 @@ function msg_handler($errno, $msg_text, $errfile, $errline) if (strpos($errfile, 'cache') === false && strpos($errfile, 'template.') === false) { - // remove complete path to installation, with the risk of changing backslashes meant to be there - $errfile = str_replace(array(phpbb_realpath($phpbb_root_path), '\\'), array('', '/'), $errfile); - $msg_text = str_replace(array(phpbb_realpath($phpbb_root_path), '\\'), array('', '/'), $msg_text); + $errfile = phpbb_filter_errfile($errfile); + $msg_text = phpbb_filter_errfile($msg_text); $error_name = ($errno === E_WARNING) ? 'PHP Warning' : 'PHP Notice'; echo '[phpBB Debug] ' . $error_name . ': in file ' . $errfile . ' on line ' . $errline . ': ' . $msg_text . '
' . "\n"; @@ -3996,6 +3995,27 @@ function msg_handler($errno, $msg_text, $errfile, $errline) return false; } +/** +* Removes absolute path to phpBB root directory from error messages +* and converts backslashes to forward slashes. +* +* @param string $errfile Absolute file path +* (e.g. /var/www/phpbb3/phpBB/includes/functions.php) +* @return string Relative file path +* (e.g. /includes/functions.php) +*/ +function phpbb_filter_errfile($errfile) +{ + static $root_path; + + if (empty($root_path)) + { + $root_path = phpbb_realpath(dirname(__FILE__) . '/../'); + } + + return str_replace(array($root_path, '\\'), array('', '/'), $errfile); +} + /** * Queries the session table to get information about online guests * @param int $item_id Limits the search to the item with this id From 1ad97424a469fe2e36a3c3a616b5e49def292779 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sun, 18 Sep 2011 22:32:25 +0200 Subject: [PATCH 3/5] [ticket/10369] Rename filter_errfile() to filter_root_path(). PHPBB3-10369 --- phpBB/includes/error_collector.php | 2 +- phpBB/includes/functions.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/phpBB/includes/error_collector.php b/phpBB/includes/error_collector.php index 040be4dd13..3c0a89a1f3 100644 --- a/phpBB/includes/error_collector.php +++ b/phpBB/includes/error_collector.php @@ -53,7 +53,7 @@ class phpbb_error_collector list($errno, $msg_text, $errfile, $errline) = $error; // Prevent leakage of local path to phpBB install - $errfile = phpbb_filter_errfile($errfile); + $errfile = phpbb_filter_root_path($errfile); $text .= "Errno $errno: $msg_text at $errfile line $errline"; } diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 4c1bfb4360..cd856f55a7 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -3816,8 +3816,8 @@ function msg_handler($errno, $msg_text, $errfile, $errline) if (strpos($errfile, 'cache') === false && strpos($errfile, 'template.') === false) { - $errfile = phpbb_filter_errfile($errfile); - $msg_text = phpbb_filter_errfile($msg_text); + $errfile = phpbb_filter_root_path($errfile); + $msg_text = phpbb_filter_root_path($msg_text); $error_name = ($errno === E_WARNING) ? 'PHP Warning' : 'PHP Notice'; echo '[phpBB Debug] ' . $error_name . ': in file ' . $errfile . ' on line ' . $errline . ': ' . $msg_text . '
' . "\n"; @@ -4004,7 +4004,7 @@ function msg_handler($errno, $msg_text, $errfile, $errline) * @return string Relative file path * (e.g. /includes/functions.php) */ -function phpbb_filter_errfile($errfile) +function phpbb_filter_root_path($errfile) { static $root_path; From c8564e6ce98980143c42a67b51b0c8327cfc12b5 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sun, 18 Sep 2011 22:41:02 +0200 Subject: [PATCH 4/5] [ticket/10369] Add warning about paths outside of phpBB root not being filtered PHPBB3-10369 --- phpBB/includes/functions.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index cd856f55a7..2e445192ae 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -4001,6 +4001,8 @@ function msg_handler($errno, $msg_text, $errfile, $errline) * * @param string $errfile Absolute file path * (e.g. /var/www/phpbb3/phpBB/includes/functions.php) +* Please note that if $errfile is outside of the phpBB root, +* the root path will not be found and can not be filtered. * @return string Relative file path * (e.g. /includes/functions.php) */ From 1b390f0b498f1eb977dd62dc06dd5753b0c7ea65 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sun, 18 Sep 2011 23:03:28 +0200 Subject: [PATCH 5/5] [ticket/10369] Replace root path with "[ROOT]" as per IRC. PHPBB3-10369 --- phpBB/includes/functions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 2e445192ae..e01bbe36d1 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -4015,7 +4015,7 @@ function phpbb_filter_root_path($errfile) $root_path = phpbb_realpath(dirname(__FILE__) . '/../'); } - return str_replace(array($root_path, '\\'), array('', '/'), $errfile); + return str_replace(array($root_path, '\\'), array('[ROOT]', '/'), $errfile); } /**