diff --git a/Zend/tests/display_error_function_args.phpt b/Zend/tests/display_error_function_args.phpt new file mode 100644 index 0000000000000..72590d1fff4b5 --- /dev/null +++ b/Zend/tests/display_error_function_args.phpt @@ -0,0 +1,28 @@ +--TEST-- +Displaying function arguments in errors +--INI-- +error_include_args=On +--FILE-- + "123456789012345678901" . chr(0))); + +ini_set("error_include_args", "Off"); + +unlink('/'); +password_hash("test", PASSWORD_BCRYPT, array("salt" => "123456789012345678901" . chr(0))); + +?> +--EXPECTF-- +Warning: unlink('/'): %s in %s on line %d + +Warning: password_hash(Object(SensitiveParameterValue), '2y', Array): The "salt" option has been ignored, since providing a custom salt is no longer supported in %s on line %d + +Warning: unlink(/): %s in %s on line %d + +Warning: password_hash(): The "salt" option has been ignored, since providing a custom salt is no longer supported in %s on line %d diff --git a/Zend/zend_exceptions.c b/Zend/zend_exceptions.c index bc794fa3b902c..ba16690396038 100644 --- a/Zend/zend_exceptions.c +++ b/Zend/zend_exceptions.c @@ -543,6 +543,31 @@ static void _build_trace_args(zval *arg, smart_str *str) /* {{{ */ } /* }}} */ +static void _build_trace_args_list(zval *tmp, smart_str *str) /* {{{ */ +{ + if (EXPECTED(Z_TYPE_P(tmp) == IS_ARRAY)) { + size_t last_len = ZSTR_LEN(str->s); + zend_string *name; + zval *arg; + + ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(tmp), name, arg) { + if (name) { + smart_str_append(str, name); + smart_str_appends(str, ": "); + } + _build_trace_args(arg, str); + } ZEND_HASH_FOREACH_END(); + + if (last_len != ZSTR_LEN(str->s)) { + ZSTR_LEN(str->s) -= 2; /* remove last ', ' */ + } + } else { + /* only happens w/ reflection abuse (Zend/tests/bug63762.phpt) */ + zend_error(E_WARNING, "args element is not an array"); + } +} +/* }}} */ + static void _build_trace_string(smart_str *str, const HashTable *ht, uint32_t num) /* {{{ */ { zval *file, *tmp; @@ -589,30 +614,49 @@ static void _build_trace_string(smart_str *str, const HashTable *ht, uint32_t nu smart_str_appendc(str, '('); tmp = zend_hash_find_known_hash(ht, ZSTR_KNOWN(ZEND_STR_ARGS)); if (tmp) { - if (EXPECTED(Z_TYPE_P(tmp) == IS_ARRAY)) { - size_t last_len = ZSTR_LEN(str->s); - zend_string *name; - zval *arg; - - ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(tmp), name, arg) { - if (name) { - smart_str_append(str, name); - smart_str_appends(str, ": "); - } - _build_trace_args(arg, str); - } ZEND_HASH_FOREACH_END(); - - if (last_len != ZSTR_LEN(str->s)) { - ZSTR_LEN(str->s) -= 2; /* remove last ', ' */ - } - } else { - zend_error(E_WARNING, "args element is not an array"); - } + _build_trace_args_list(tmp, str); } smart_str_appends(str, ")\n"); } /* }}} */ +/* {{{ Gets the function arguments printed as a string from a backtrace frame. */ +ZEND_API zend_string *zend_trace_function_args_to_string(const HashTable *frame) { + zval *tmp; + smart_str str = {0}; + /* init because ASan will complain */ + smart_str_appends(&str, ""); + + tmp = zend_hash_find_known_hash(frame, ZSTR_KNOWN(ZEND_STR_ARGS)); + if (tmp) { + _build_trace_args_list(tmp, &str); + } + + smart_str_0(&str); + return str.s ? str.s : ZSTR_EMPTY_ALLOC(); +} +/* }}} */ + +/* {{{ Gets the currently executing function's arguments as a string. Used by php_verror. */ +ZEND_API zend_string *zend_trace_current_function_args_string(void) { + zend_string *dynamic_params = NULL; + /* get a backtrace to snarf function args */ + zval backtrace, *first_frame; + zend_fetch_debug_backtrace(&backtrace, /* skip_last */ 0, /* options */ 0, /* limit */ 1); + /* can fail esp if low memory condition */ + if (Z_TYPE(backtrace) != IS_ARRAY) { + return NULL; /* don't need to free */ + } + first_frame = zend_hash_index_find(Z_ARRVAL(backtrace), 0); + if (first_frame) { + dynamic_params = zend_trace_function_args_to_string(Z_ARRVAL_P(first_frame)); + } + zval_ptr_dtor(&backtrace); + /* free the string after we use it */ + return dynamic_params; +} +/* }}} */ + ZEND_API zend_string *zend_trace_to_string(const HashTable *trace, bool include_main) { zend_ulong index; zval *frame; diff --git a/Zend/zend_exceptions.h b/Zend/zend_exceptions.h index e5a6be2f32feb..b52bb48d166ae 100644 --- a/Zend/zend_exceptions.h +++ b/Zend/zend_exceptions.h @@ -66,6 +66,8 @@ ZEND_API zend_result zend_update_exception_properties(zend_execute_data *execute /* show an exception using zend_error(severity,...), severity should be E_ERROR */ ZEND_API ZEND_COLD zend_result zend_exception_error(zend_object *exception, int severity); ZEND_NORETURN void zend_exception_uncaught_error(const char *prefix, ...) ZEND_ATTRIBUTE_FORMAT(printf, 1, 2); +ZEND_API zend_string *zend_trace_function_args_to_string(const HashTable *frame); +ZEND_API zend_string *zend_trace_current_function_args_string(void); ZEND_API zend_string *zend_trace_to_string(const HashTable *trace, bool include_main); ZEND_API ZEND_COLD zend_object *zend_create_unwind_exit(void); diff --git a/ext/openssl/tests/ServerClientTestCase.inc b/ext/openssl/tests/ServerClientTestCase.inc index 8eedbfdebee8b..25a1cc75e78af 100644 --- a/ext/openssl/tests/ServerClientTestCase.inc +++ b/ext/openssl/tests/ServerClientTestCase.inc @@ -100,7 +100,8 @@ class ServerClientTestCase $ini = php_ini_loaded_file(); $cmd = sprintf( '%s %s "%s" %s', - PHP_BINARY, $ini ? "-n -c $ini" : "", + // XXX: TEST_PHP_EXTRA_ARGS for run-test values won't work here? + PHP_BINARY, $ini ? "-n -c $ini -d error_include_args=0" : "", __FILE__, WORKER_ARGV_VALUE ); diff --git a/main/main.c b/main/main.c index bb5b700fe7ae1..723d53f7f66c5 100644 --- a/main/main.c +++ b/main/main.c @@ -64,6 +64,7 @@ #include "win32/php_registry.h" #include "ext/standard/flock_compat.h" #endif +#include "Zend/zend_builtin_functions.h" #include "Zend/zend_exceptions.h" #if PHP_SIGCHILD @@ -803,6 +804,7 @@ PHP_INI_BEGIN() STD_PHP_INI_ENTRY_EX("display_errors", "1", PHP_INI_ALL, OnUpdateDisplayErrors, display_errors, php_core_globals, core_globals, display_errors_mode) STD_PHP_INI_BOOLEAN("display_startup_errors", "1", PHP_INI_ALL, OnUpdateBool, display_startup_errors, php_core_globals, core_globals) STD_PHP_INI_BOOLEAN("enable_dl", "1", PHP_INI_SYSTEM, OnUpdateBool, enable_dl, php_core_globals, core_globals) + STD_PHP_INI_BOOLEAN("error_include_args", "1", PHP_INI_ALL, OnUpdateBool, error_include_args, php_core_globals, core_globals) STD_PHP_INI_BOOLEAN("expose_php", "1", PHP_INI_SYSTEM, OnUpdateBool, expose_php, php_core_globals, core_globals) STD_PHP_INI_ENTRY("docref_root", "", PHP_INI_ALL, OnUpdateString, docref_root, php_core_globals, core_globals) STD_PHP_INI_ENTRY("docref_ext", "", PHP_INI_ALL, OnUpdateString, docref_ext, php_core_globals, core_globals) @@ -1134,7 +1136,14 @@ PHPAPI ZEND_COLD void php_verror(const char *docref, const char *params, int typ /* if we still have memory then format the origin */ if (is_function) { - origin_len = spprintf(&origin, 0, "%s%s%s(%s)", class_name, space, function, params); + zend_string *dynamic_params = NULL; + if (PG(error_include_args)) { + dynamic_params = zend_trace_current_function_args_string(); + } + origin_len = spprintf(&origin, 0, "%s%s%s(%s)", class_name, space, function, dynamic_params ? ZSTR_VAL(dynamic_params) : params); + if (dynamic_params) { + zend_string_release(dynamic_params); + } } else { origin_len = strlen(function); origin = estrndup(function, origin_len); diff --git a/main/php_globals.h b/main/php_globals.h index 893bf25d26cb5..a1876ad1e3412 100644 --- a/main/php_globals.h +++ b/main/php_globals.h @@ -61,6 +61,7 @@ struct _php_core_globals { uint8_t display_errors; bool display_startup_errors; + bool error_include_args; bool log_errors; bool ignore_repeated_errors; bool ignore_repeated_source; diff --git a/php.ini-development b/php.ini-development index 6f93f440112ea..a6e3f683d1fbf 100644 --- a/php.ini-development +++ b/php.ini-development @@ -611,6 +611,15 @@ ignore_repeated_source = Off ; Production Value: On ;fatal_error_backtraces = On +; This directive controls whether PHP will print the actual arguments of a +; function upon an error. If this is off (or there was an error fetching the +; arguments), the function providing the error may optionally provide some +; additional information after the problem function's name. +; Default Value: Off +; Development Value: Off +; Production Value: Off +;error_include_args = Off + ;;;;;;;;;;;;;;;;; ; Data Handling ; ;;;;;;;;;;;;;;;;; diff --git a/php.ini-production b/php.ini-production index 9aafad21e9c74..d91f40b292c10 100644 --- a/php.ini-production +++ b/php.ini-production @@ -613,6 +613,15 @@ ignore_repeated_source = Off ; Production Value: On ;fatal_error_backtraces = On +; This directive controls whether PHP will print the actual arguments of a +; function upon an error. If this is off (or there was an error fetching the +; arguments), the function providing the error may optionally provide some +; additional information after the problem function's name. +; Default Value: Off +; Development Value: Off +; Production Value: Off +;error_include_args = Off + ;;;;;;;;;;;;;;;;; ; Data Handling ; ;;;;;;;;;;;;;;;;; diff --git a/run-tests.php b/run-tests.php index 68d2cb69256da..52df86f7411a1 100755 --- a/run-tests.php +++ b/run-tests.php @@ -275,6 +275,7 @@ function main(): void 'fatal_error_backtraces=Off', 'display_errors=1', 'display_startup_errors=1', + 'error_include_args=0', 'log_errors=0', 'html_errors=0', 'track_errors=0', diff --git a/sapi/cli/tests/php_cli_server.inc b/sapi/cli/tests/php_cli_server.inc index 3022022f894e9..3ad6ced5cb449 100644 --- a/sapi/cli/tests/php_cli_server.inc +++ b/sapi/cli/tests/php_cli_server.inc @@ -24,7 +24,8 @@ function php_cli_server_start( file_put_contents($doc_root . '/' . ($router ?: 'index.php'), ''); } - $cmd = [$php_executable, '-t', $doc_root, '-n', ...$cmd_args, '-S', 'localhost:0']; + // XXX: This should ideally use the same INI overrides as run-tests + $cmd = [$php_executable, '-d', 'error_include_args=0', '-t', $doc_root, '-n', ...$cmd_args, '-S', 'localhost:0']; if (!is_null($router)) { $cmd[] = $router; } diff --git a/scripts/dev/bless_tests.php b/scripts/dev/bless_tests.php index 7493a504729c4..a578903ef6dd8 100755 --- a/scripts/dev/bless_tests.php +++ b/scripts/dev/bless_tests.php @@ -74,6 +74,10 @@ function normalizeOutput(string $out): string { 'Resource ID#%d used as offset, casting to integer (%d)', $out); $out = preg_replace('/string\(\d+\) "([^"]*%d)/', 'string(%d) "$1', $out); + // Inside of strings, replace absolute paths that have been truncated with + // any string. These tend to contain homedirs with usernames, not good. + $out = preg_replace("/'\\/.*\.\\.\\.'/", "'%s'", $out); + $out = preg_replace("/'file:\/\\/.*\.\\.\\.'/", "'%s'", $out); $out = str_replace("\0", '%0', $out); return $out; }