From 95c67c676cf86d0e54a3e35a1a6afd35dea2375f Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 29 May 2020 10:33:22 +0200 Subject: [PATCH 1/3] Pass zend_string message to zend_error_cb --- Zend/zend.c | 24 ++++++------- Zend/zend.h | 4 +-- Zend/zend_exceptions.c | 37 ++++++++++---------- Zend/zend_exceptions.h | 2 +- ext/opcache/ZendAccelerator.c | 24 ++++--------- ext/soap/soap.c | 63 +++++++++------------------------- ext/standard/basic_functions.c | 5 +-- main/main.c | 41 +++++++++------------- main/php_globals.h | 2 +- sapi/cli/php_cli_server.c | 3 +- 10 files changed, 78 insertions(+), 127 deletions(-) diff --git a/Zend/zend.c b/Zend/zend.c index 4a9c939aa0033..d6d699cb5acdd 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -74,7 +74,7 @@ ZEND_API FILE *(*zend_fopen)(const char *filename, zend_string **opened_path); ZEND_API int (*zend_stream_open_function)(const char *filename, zend_file_handle *handle); ZEND_API void (*zend_ticks_function)(int ticks); ZEND_API void (*zend_interrupt_function)(zend_execute_data *execute_data); -ZEND_API void (*zend_error_cb)(int type, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args); +ZEND_API void (*zend_error_cb)(int type, const char *error_filename, const uint32_t error_lineno, zend_string *message); void (*zend_printf_to_smart_string)(smart_string *buf, const char *format, va_list ap); void (*zend_printf_to_smart_str)(smart_str *buf, const char *format, va_list ap); ZEND_API char *(*zend_getenv)(char *name, size_t name_len); @@ -1263,7 +1263,6 @@ static ZEND_COLD void zend_error_va_list( int orig_type, const char *error_filename, uint32_t error_lineno, const char *format, va_list args) { - va_list usr_copy; zval params[4]; zval retval; zval orig_user_error_handler; @@ -1273,6 +1272,7 @@ static ZEND_COLD void zend_error_va_list( zend_stack delayed_oplines_stack; zend_class_entry *orig_fake_scope; int type = orig_type & E_ALL; + zend_string *message = zend_vstrpprintf(0, format, args); /* Report about uncaught exception in case of fatal errors */ if (EG(exception)) { @@ -1308,10 +1308,7 @@ static ZEND_COLD void zend_error_va_list( #ifdef HAVE_DTRACE if (DTRACE_ERROR_ENABLED()) { - char *dtrace_error_buffer; - zend_vspprintf(&dtrace_error_buffer, 0, format, args); - DTRACE_ERROR(dtrace_error_buffer, (char *)error_filename, error_lineno); - efree(dtrace_error_buffer); + DTRACE_ERROR(ZSTR_VAL(message), (char *)error_filename, error_lineno); } #endif /* HAVE_DTRACE */ @@ -1319,7 +1316,7 @@ static ZEND_COLD void zend_error_va_list( if (Z_TYPE(EG(user_error_handler)) == IS_UNDEF || !(EG(user_error_handler_error_reporting) & type) || EG(error_handling) != EH_NORMAL) { - zend_error_cb(orig_type, error_filename, error_lineno, format, args); + zend_error_cb(orig_type, error_filename, error_lineno, message); } else switch (type) { case E_ERROR: case E_PARSE: @@ -1328,14 +1325,11 @@ static ZEND_COLD void zend_error_va_list( case E_COMPILE_ERROR: case E_COMPILE_WARNING: /* The error may not be safe to handle in user-space */ - zend_error_cb(orig_type, error_filename, error_lineno, format, args); + zend_error_cb(orig_type, error_filename, error_lineno, message); break; default: /* Handle the error in user space */ - va_copy(usr_copy, args); - ZVAL_STR(¶ms[1], zend_vstrpprintf(0, format, usr_copy)); - va_end(usr_copy); - + ZVAL_STR_COPY(¶ms[1], message); ZVAL_LONG(¶ms[0], type); if (error_filename) { @@ -1369,13 +1363,13 @@ static ZEND_COLD void zend_error_va_list( if (call_user_function(CG(function_table), NULL, &orig_user_error_handler, &retval, 4, params) == SUCCESS) { if (Z_TYPE(retval) != IS_UNDEF) { if (Z_TYPE(retval) == IS_FALSE) { - zend_error_cb(orig_type, error_filename, error_lineno, format, args); + zend_error_cb(orig_type, error_filename, error_lineno, message); } zval_ptr_dtor(&retval); } } else if (!EG(exception)) { /* The user error handler failed, use built-in error handler */ - zend_error_cb(orig_type, error_filename, error_lineno, format, args); + zend_error_cb(orig_type, error_filename, error_lineno, message); } EG(fake_scope) = orig_fake_scope; @@ -1408,6 +1402,8 @@ static ZEND_COLD void zend_error_va_list( EG(exit_status) = 255; } } + + zend_string_release(message); } /* }}} */ diff --git a/Zend/zend.h b/Zend/zend.h index cd41cde5ef1b7..6e43b296f4091 100644 --- a/Zend/zend.h +++ b/Zend/zend.h @@ -186,7 +186,7 @@ struct _zend_class_entry { }; typedef struct _zend_utility_functions { - void (*error_function)(int type, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args) ZEND_ATTRIBUTE_PTR_FORMAT(printf, 4, 0); + void (*error_function)(int type, const char *error_filename, const uint32_t error_lineno, zend_string *message); size_t (*printf_function)(const char *format, ...) ZEND_ATTRIBUTE_PTR_FORMAT(printf, 1, 2); size_t (*write_function)(const char *str, size_t str_length); FILE *(*fopen_function)(const char *filename, zend_string **opened_path); @@ -280,7 +280,7 @@ extern ZEND_API zend_write_func_t zend_write; extern ZEND_API FILE *(*zend_fopen)(const char *filename, zend_string **opened_path); extern ZEND_API void (*zend_ticks_function)(int ticks); extern ZEND_API void (*zend_interrupt_function)(zend_execute_data *execute_data); -extern ZEND_API void (*zend_error_cb)(int type, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args) ZEND_ATTRIBUTE_PTR_FORMAT(printf, 4, 0); +extern ZEND_API void (*zend_error_cb)(int type, const char *error_filename, const uint32_t error_lineno, zend_string *message); extern ZEND_API void (*zend_on_timeout)(int seconds); extern ZEND_API int (*zend_stream_open_function)(const char *filename, zend_file_handle *handle); extern void (*zend_printf_to_smart_string)(smart_string *buf, const char *format, va_list ap); diff --git a/Zend/zend_exceptions.c b/Zend/zend_exceptions.c index a2bdbdad746fb..0204e14a58f6a 100644 --- a/Zend/zend_exceptions.c +++ b/Zend/zend_exceptions.c @@ -820,7 +820,7 @@ ZEND_API zend_class_entry *zend_get_error_exception(void) } /* }}} */ -ZEND_API ZEND_COLD zend_object *zend_throw_exception(zend_class_entry *exception_ce, const char *message, zend_long code) /* {{{ */ +static zend_object *zend_throw_exception_zstr(zend_class_entry *exception_ce, zend_string *message, zend_long code) /* {{{ */ { zval ex, tmp; @@ -836,9 +836,8 @@ ZEND_API ZEND_COLD zend_object *zend_throw_exception(zend_class_entry *exception if (message) { - ZVAL_STRING(&tmp, message); + ZVAL_STR(&tmp, message); zend_update_property_ex(exception_ce, &ex, ZSTR_KNOWN(ZEND_STR_MESSAGE), &tmp); - zval_ptr_dtor(&tmp); } if (code) { ZVAL_LONG(&tmp, code); @@ -850,6 +849,15 @@ ZEND_API ZEND_COLD zend_object *zend_throw_exception(zend_class_entry *exception } /* }}} */ +ZEND_API ZEND_COLD zend_object *zend_throw_exception(zend_class_entry *exception_ce, const char *message, zend_long code) /* {{{ */ +{ + zend_string *msg_str = zend_string_init(message, strlen(message), 0); + zend_object *ex = zend_throw_exception_zstr(exception_ce, msg_str, code); + zend_string_release(msg_str); + return ex; +} +/* }}} */ + ZEND_API ZEND_COLD zend_object *zend_throw_exception_ex(zend_class_entry *exception_ce, zend_long code, const char *format, ...) /* {{{ */ { va_list arg; @@ -865,10 +873,10 @@ ZEND_API ZEND_COLD zend_object *zend_throw_exception_ex(zend_class_entry *except } /* }}} */ -ZEND_API ZEND_COLD zend_object *zend_throw_error_exception(zend_class_entry *exception_ce, const char *message, zend_long code, int severity) /* {{{ */ +ZEND_API ZEND_COLD zend_object *zend_throw_error_exception(zend_class_entry *exception_ce, zend_string *message, zend_long code, int severity) /* {{{ */ { zval ex, tmp; - zend_object *obj = zend_throw_exception(exception_ce, message, code); + zend_object *obj = zend_throw_exception_zstr(exception_ce, message, code); ZVAL_OBJ(&ex, obj); ZVAL_LONG(&tmp, severity); zend_update_property_ex(zend_ce_error_exception, &ex, ZSTR_KNOWN(ZEND_STR_SEVERITY), &tmp); @@ -879,23 +887,14 @@ ZEND_API ZEND_COLD zend_object *zend_throw_error_exception(zend_class_entry *exc static void zend_error_va(int type, const char *file, uint32_t lineno, const char *format, ...) /* {{{ */ { va_list args; - va_start(args, format); - zend_error_cb(type, file, lineno, format, args); + zend_string *message = zend_vstrpprintf(0, format, args); + zend_error_cb(type, file, lineno, message); + zend_string_release(message); va_end(args); } /* }}} */ -static void zend_error_helper(int type, const char *filename, const uint32_t lineno, const char *format, ...) /* {{{ */ -{ - va_list va; - - va_start(va, format); - zend_error_cb(type, filename, lineno, format, va); - va_end(va); -} -/* }}} */ - /* This function doesn't return if it uses E_ERROR */ ZEND_API ZEND_COLD void zend_exception_error(zend_object *ex, int severity) /* {{{ */ { @@ -910,9 +909,9 @@ ZEND_API ZEND_COLD void zend_exception_error(zend_object *ex, int severity) /* { zend_string *file = zval_get_string(GET_PROPERTY_SILENT(&exception, ZEND_STR_FILE)); zend_long line = zval_get_long(GET_PROPERTY_SILENT(&exception, ZEND_STR_LINE)); - zend_error_helper( + zend_error_cb( (ce_exception == zend_ce_parse_error ? E_PARSE : E_COMPILE_ERROR) | E_DONT_BAIL, - ZSTR_VAL(file), line, "%s", ZSTR_VAL(message)); + ZSTR_VAL(file), line, message); zend_string_release_ex(file, 0); zend_string_release_ex(message, 0); diff --git a/Zend/zend_exceptions.h b/Zend/zend_exceptions.h index ec6f0a0201111..ced74bf9f163c 100644 --- a/Zend/zend_exceptions.h +++ b/Zend/zend_exceptions.h @@ -61,7 +61,7 @@ ZEND_API ZEND_COLD zend_object *zend_throw_exception_ex(zend_class_entry *except ZEND_API ZEND_COLD void zend_throw_exception_object(zval *exception); ZEND_API void zend_clear_exception(void); -ZEND_API zend_object *zend_throw_error_exception(zend_class_entry *exception_ce, const char *message, zend_long code, int severity); +ZEND_API zend_object *zend_throw_error_exception(zend_class_entry *exception_ce, zend_string *message, zend_long code, int severity); extern ZEND_API void (*zend_throw_exception_hook)(zval *ex); diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index 25c76c4a5e4af..ef8f39f7a9ae2 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -121,7 +121,7 @@ zend_bool fallback_process = 0; /* process uses file cache fallback */ static zend_op_array *(*accelerator_orig_compile_file)(zend_file_handle *file_handle, int type); static int (*accelerator_orig_zend_stream_open_function)(const char *filename, zend_file_handle *handle ); static zend_string *(*accelerator_orig_zend_resolve_path)(const char *filename, size_t filename_len); -static void (*accelerator_orig_zend_error_cb)(int type, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args); +static void (*accelerator_orig_zend_error_cb)(int type, const char *error_filename, const uint32_t error_lineno, zend_string *message); static zif_handler orig_chdir = NULL; static ZEND_INI_MH((*orig_include_path_on_modify)) = NULL; static int (*orig_post_startup_cb)(void); @@ -1669,37 +1669,27 @@ static void zend_accel_init_auto_globals(void) } } -static void persistent_error_cb(int type, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args) { +static void persistent_error_cb(int type, const char *error_filename, const uint32_t error_lineno, zend_string *message) { if (ZCG(record_warnings)) { zend_recorded_warning *warning = emalloc(sizeof(zend_recorded_warning)); - va_list args_copy; warning->type = type; warning->error_lineno = error_lineno; warning->error_filename = zend_string_init(error_filename, strlen(error_filename), 0); - va_copy(args_copy, args); - warning->error_message = zend_vstrpprintf(0, format, args_copy); - va_end(args_copy); + warning->error_message = zend_string_copy(message); ZCG(num_warnings)++; ZCG(warnings) = erealloc(ZCG(warnings), sizeof(zend_recorded_warning) * ZCG(num_warnings)); ZCG(warnings)[ZCG(num_warnings)-1] = warning; } - accelerator_orig_zend_error_cb(type, error_filename, error_lineno, format, args); -} - -/* Hack to get us a va_list to pass to zend_error_cb. */ -static void replay_warning_helper(const zend_recorded_warning *warning, ...) { - va_list va; - va_start(va, warning); - accelerator_orig_zend_error_cb( - warning->type, ZSTR_VAL(warning->error_filename), warning->error_lineno, "%s", va); - va_end(va); + accelerator_orig_zend_error_cb(type, error_filename, error_lineno, message); } static void replay_warnings(zend_persistent_script *script) { for (uint32_t i = 0; i < script->num_warnings; i++) { zend_recorded_warning *warning = script->warnings[i]; - replay_warning_helper(warning, ZSTR_VAL(warning->error_message)); + accelerator_orig_zend_error_cb( + warning->type, ZSTR_VAL(warning->error_filename), warning->error_lineno, + warning->error_message); } } diff --git a/ext/soap/soap.c b/ext/soap/soap.c index 57b912fd0e0ff..2444f4b2354a8 100644 --- a/ext/soap/soap.c +++ b/ext/soap/soap.c @@ -67,7 +67,7 @@ static void delete_service(void *service); static void delete_url(void *handle); static void delete_hashtable(void *hashtable); -static void soap_error_handler(int error_num, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args); +static void soap_error_handler(int error_num, const char *error_filename, const uint32_t error_lineno, zend_string *message); #define SOAP_SERVER_BEGIN_CODE() \ zend_bool _old_handler = SOAP_GLOBAL(use_soap_error_handler);\ @@ -163,15 +163,7 @@ zend_class_entry* soap_var_class_entry; ZEND_DECLARE_MODULE_GLOBALS(soap) -static void (*old_error_handler)(int, const char *, const uint32_t, const char*, va_list); - -#define call_old_error_handler(error_num, error_filename, error_lineno, format, args) \ -{ \ - va_list copy; \ - va_copy(copy, args); \ - old_error_handler(error_num, error_filename, error_lineno, format, copy); \ - va_end(copy); \ -} +static void (*old_error_handler)(int, const char *, const uint32_t, zend_string *); #define PHP_SOAP_SERVER_CLASSNAME "SoapServer" #define PHP_SOAP_CLIENT_CLASSNAME "SoapClient" @@ -1837,7 +1829,7 @@ static ZEND_NORETURN void soap_server_fault(char* code, char* string, char *acto } /* }}} */ -static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args) /* {{{ */ +static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, const char *error_filename, const uint32_t error_lineno, zend_string *message) /* {{{ */ { zend_bool _old_in_compilation; zend_execute_data *_old_current_execute_data; @@ -1866,24 +1858,12 @@ static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, c error_num == E_PARSE) && use_exceptions) { zval fault; - char* code = SOAP_GLOBAL(error_code); - char buffer[1024]; - size_t buffer_len; - va_list argcopy; - - va_copy(argcopy, args); - buffer_len = vslprintf(buffer, sizeof(buffer)-1, format, argcopy); - va_end(argcopy); - - buffer[sizeof(buffer)-1]=0; - if (buffer_len > sizeof(buffer) - 1 || buffer_len == (size_t)-1) { - buffer_len = sizeof(buffer) - 1; - } - + char *code = SOAP_GLOBAL(error_code); if (code == NULL) { code = "Client"; } - add_soap_fault_ex(&fault, &SOAP_GLOBAL(error_object), code, buffer, NULL, NULL); + + add_soap_fault_ex(&fault, &SOAP_GLOBAL(error_object), code, ZSTR_VAL(message), NULL, NULL); Z_ADDREF(fault); zend_throw_exception_object(&fault); zend_bailout(); @@ -1891,13 +1871,12 @@ static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, c !SOAP_GLOBAL(error_code) || strcmp(SOAP_GLOBAL(error_code),"WSDL") != 0) { /* Ignore libxml warnings during WSDL parsing */ - call_old_error_handler(error_num, error_filename, error_lineno, format, args); + old_error_handler(error_num, error_filename, error_lineno, message); } } else { int old = PG(display_errors); int fault = 0; zval fault_obj; - va_list argcopy; if (error_num == E_USER_ERROR || error_num == E_COMPILE_ERROR || @@ -1906,7 +1885,7 @@ static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, c error_num == E_PARSE) { char* code = SOAP_GLOBAL(error_code); - char buffer[1024]; + zend_string *buffer; zval outbuf; zval *tmp; soapServicePtr service; @@ -1920,21 +1899,12 @@ static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, c (tmp = zend_hash_str_find(Z_OBJPROP(SOAP_GLOBAL(error_object)), "service", sizeof("service")-1)) != NULL && (service = (soapServicePtr)zend_fetch_resource_ex(tmp, "service", le_service)) && !service->send_errors) { - strcpy(buffer, "Internal Error"); + buffer = zend_string_init("Internal Error", sizeof("Internal Error")-1, 0); } else { - size_t buffer_len; - zval outbuflen; - - va_copy(argcopy, args); - buffer_len = vslprintf(buffer, sizeof(buffer)-1, format, argcopy); - va_end(argcopy); - - buffer[sizeof(buffer)-1]=0; - if (buffer_len > sizeof(buffer) - 1 || buffer_len == (size_t)-1) { - buffer_len = sizeof(buffer) - 1; - } + buffer = zend_string_copy(message); /* Get output buffer and send as fault detials */ + zval outbuflen; if (php_output_get_length(&outbuflen) != FAILURE && Z_LVAL(outbuflen) != 0) { php_output_get_contents(&outbuf); } @@ -1942,14 +1912,15 @@ static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, c } ZVAL_NULL(&fault_obj); - set_soap_fault(&fault_obj, NULL, code, buffer, NULL, &outbuf, NULL); + set_soap_fault(&fault_obj, NULL, code, ZSTR_VAL(buffer), NULL, &outbuf, NULL); + zend_string_release(buffer); fault = 1; } PG(display_errors) = 0; SG(sapi_headers).http_status_line = NULL; zend_try { - call_old_error_handler(error_num, error_filename, error_lineno, format, args); + old_error_handler(error_num, error_filename, error_lineno, message); } zend_catch { CG(in_compilation) = _old_in_compilation; EG(current_execute_data) = _old_current_execute_data; @@ -1969,12 +1940,12 @@ static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, c } /* }}} */ -static void soap_error_handler(int error_num, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args) /* {{{ */ +static void soap_error_handler(int error_num, const char *error_filename, const uint32_t error_lineno, zend_string *message) /* {{{ */ { if (EXPECTED(!SOAP_GLOBAL(use_soap_error_handler))) { - call_old_error_handler(error_num, error_filename, error_lineno, format, args); + old_error_handler(error_num, error_filename, error_lineno, message); } else { - soap_real_error_handler(error_num, error_filename, error_lineno, format, args); + soap_real_error_handler(error_num, error_filename, error_lineno, message); } } /* }}} */ diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index 7e0f4e3da7898..ac407108e0159 100755 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -1533,7 +1533,8 @@ PHP_FUNCTION(error_get_last) if (PG(last_error_message)) { array_init(return_value); add_assoc_long_ex(return_value, "type", sizeof("type")-1, PG(last_error_type)); - add_assoc_string_ex(return_value, "message", sizeof("message")-1, PG(last_error_message)); + add_assoc_str_ex(return_value, "message", sizeof("message")-1, + zend_string_copy(PG(last_error_message))); add_assoc_string_ex(return_value, "file", sizeof("file")-1, PG(last_error_file)?PG(last_error_file):"-"); add_assoc_long_ex(return_value, "line", sizeof("line")-1, PG(last_error_lineno)); } @@ -1550,7 +1551,7 @@ PHP_FUNCTION(error_clear_last) PG(last_error_type) = 0; PG(last_error_lineno) = 0; - free(PG(last_error_message)); + zend_string_release(PG(last_error_message)); PG(last_error_message) = NULL; if (PG(last_error_file)) { diff --git a/main/main.c b/main/main.c index 6143718d05d42..83d450f8307b9 100644 --- a/main/main.c +++ b/main/main.c @@ -1190,19 +1190,16 @@ PHPAPI void php_html_puts(const char *str, size_t size) /* {{{ php_error_cb extended error handling function */ -static ZEND_COLD void php_error_cb(int orig_type, const char *error_filename, const uint32_t error_lineno, const char *format, va_list args) +static ZEND_COLD void php_error_cb(int orig_type, const char *error_filename, const uint32_t error_lineno, zend_string *message) { - char *buffer; - int buffer_len, display; + zend_bool display; int type = orig_type & E_ALL; - buffer_len = (int)vspprintf(&buffer, PG(log_errors_max_len), format, args); - /* check for repeated errors to be ignored */ if (PG(ignore_repeated_errors) && PG(last_error_message)) { /* no check for PG(last_error_file) is needed since it cannot * be NULL if PG(last_error_message) is not NULL */ - if (strcmp(PG(last_error_message), buffer) + if (zend_string_equals(PG(last_error_message), message) || (!PG(ignore_repeated_source) && ((PG(last_error_lineno) != (int)error_lineno) || strcmp(PG(last_error_file), error_filename)))) { @@ -1238,9 +1235,8 @@ static ZEND_COLD void php_error_cb(int orig_type, const char *error_filename, co * but DO NOT overwrite a pending exception */ if (!EG(exception)) { - zend_throw_error_exception(EG(exception_class), buffer, 0, type); + zend_throw_error_exception(EG(exception_class), message, 0, type); } - efree(buffer); return; } } @@ -1248,9 +1244,9 @@ static ZEND_COLD void php_error_cb(int orig_type, const char *error_filename, co /* store the error if it has changed */ if (display) { if (PG(last_error_message)) { - char *s = PG(last_error_message); + zend_string *s = PG(last_error_message); PG(last_error_message) = NULL; - free(s); + zend_string_release(s); } if (PG(last_error_file)) { char *s = PG(last_error_file); @@ -1261,7 +1257,7 @@ static ZEND_COLD void php_error_cb(int orig_type, const char *error_filename, co error_filename = "Unknown"; } PG(last_error_type) = type; - PG(last_error_message) = strdup(buffer); + PG(last_error_message) = zend_string_copy(message); PG(last_error_file) = strdup(error_filename); PG(last_error_lineno) = error_lineno; } @@ -1318,40 +1314,40 @@ static ZEND_COLD void php_error_cb(int orig_type, const char *error_filename, co char *log_buffer; #ifdef PHP_WIN32 if (type == E_CORE_ERROR || type == E_CORE_WARNING) { - syslog(LOG_ALERT, "PHP %s: %s (%s)", error_type_str, buffer, GetCommandLine()); + syslog(LOG_ALERT, "PHP %s: %s (%s)", error_type_str, ZSTR_VAL(message), GetCommandLine()); } #endif - spprintf(&log_buffer, 0, "PHP %s: %s in %s on line %" PRIu32, error_type_str, buffer, error_filename, error_lineno); + spprintf(&log_buffer, 0, "PHP %s: %s in %s on line %" PRIu32, error_type_str, ZSTR_VAL(message), error_filename, error_lineno); php_log_err_with_severity(log_buffer, syslog_type_int); efree(log_buffer); } if (PG(display_errors) && ((module_initialized && !PG(during_request_startup)) || (PG(display_startup_errors)))) { if (PG(xmlrpc_errors)) { - php_printf("faultCode" ZEND_LONG_FMT "faultString%s:%s in %s on line %" PRIu32 "", PG(xmlrpc_error_number), error_type_str, buffer, error_filename, error_lineno); + php_printf("faultCode" ZEND_LONG_FMT "faultString%s:%s in %s on line %" PRIu32 "", PG(xmlrpc_error_number), error_type_str, ZSTR_VAL(message), error_filename, error_lineno); } else { char *prepend_string = INI_STR("error_prepend_string"); char *append_string = INI_STR("error_append_string"); if (PG(html_errors)) { if (type == E_ERROR || type == E_PARSE) { - zend_string *buf = escape_html(buffer, buffer_len); + zend_string *buf = escape_html(ZSTR_VAL(message), ZSTR_LEN(message)); php_printf("%s
\n%s: %s in %s on line %" PRIu32 "
\n%s", STR_PRINT(prepend_string), error_type_str, ZSTR_VAL(buf), error_filename, error_lineno, STR_PRINT(append_string)); zend_string_free(buf); } else { - php_printf("%s
\n%s: %s in %s on line %" PRIu32 "
\n%s", STR_PRINT(prepend_string), error_type_str, buffer, error_filename, error_lineno, STR_PRINT(append_string)); + php_printf("%s
\n%s: %s in %s on line %" PRIu32 "
\n%s", STR_PRINT(prepend_string), error_type_str, ZSTR_VAL(message), error_filename, error_lineno, STR_PRINT(append_string)); } } else { /* Write CLI/CGI errors to stderr if display_errors = "stderr" */ if ((!strcmp(sapi_module.name, "cli") || !strcmp(sapi_module.name, "cgi") || !strcmp(sapi_module.name, "phpdbg")) && PG(display_errors) == PHP_DISPLAY_ERRORS_STDERR ) { - fprintf(stderr, "%s: %s in %s on line %" PRIu32 "\n", error_type_str, buffer, error_filename, error_lineno); + fprintf(stderr, "%s: %s in %s on line %" PRIu32 "\n", error_type_str, ZSTR_VAL(message), error_filename, error_lineno); #ifdef PHP_WIN32 fflush(stderr); #endif } else { - php_printf("%s\n%s: %s in %s on line %" PRIu32 "\n%s", STR_PRINT(prepend_string), error_type_str, buffer, error_filename, error_lineno, STR_PRINT(append_string)); + php_printf("%s\n%s: %s in %s on line %" PRIu32 "\n%s", STR_PRINT(prepend_string), error_type_str, ZSTR_VAL(message), error_filename, error_lineno, STR_PRINT(append_string)); } } } @@ -1371,7 +1367,7 @@ static ZEND_COLD void php_error_cb(int orig_type, const char *error_filename, co trigger_break=0; break; } - zend_output_debug_string(trigger_break, "%s(%" PRIu32 ") : %s - %s", error_filename, error_lineno, error_type_str, buffer); + zend_output_debug_string(trigger_break, "%s(%" PRIu32 ") : %s - %s", error_filename, error_lineno, error_type_str, ZSTR_VAL(message)); } #endif } @@ -1405,7 +1401,6 @@ static ZEND_COLD void php_error_cb(int orig_type, const char *error_filename, co if (!(orig_type & E_DONT_BAIL)) { /* restore memory limit */ zend_set_memory_limit(PG(memory_limit)); - efree(buffer); zend_objects_store_mark_destructed(&EG(objects_store)); zend_bailout(); return; @@ -1413,8 +1408,6 @@ static ZEND_COLD void php_error_cb(int orig_type, const char *error_filename, co } break; } - - efree(buffer); } /* }}} */ @@ -1595,7 +1588,7 @@ static zval *php_get_configuration_directive_for_zend(zend_string *name) static void php_free_request_globals(void) { if (PG(last_error_message)) { - free(PG(last_error_message)); + zend_string_release(PG(last_error_message)); PG(last_error_message) = NULL; } if (PG(last_error_file)) { @@ -1961,7 +1954,7 @@ static void core_globals_ctor(php_core_globals *core_globals) static void core_globals_dtor(php_core_globals *core_globals) { if (core_globals->last_error_message) { - free(core_globals->last_error_message); + zend_string_release(core_globals->last_error_message); } if (core_globals->last_error_file) { free(core_globals->last_error_file); diff --git a/main/php_globals.h b/main/php_globals.h index e5709ddc67444..3f34aac951d6c 100644 --- a/main/php_globals.h +++ b/main/php_globals.h @@ -133,7 +133,7 @@ struct _php_core_globals { zend_bool report_zend_debug; int last_error_type; - char *last_error_message; + zend_string *last_error_message; char *last_error_file; int last_error_lineno; diff --git a/sapi/cli/php_cli_server.c b/sapi/cli/php_cli_server.c index dbee30700f834..b9c5aa523b38b 100644 --- a/sapi/cli/php_cli_server.c +++ b/sapi/cli/php_cli_server.c @@ -1214,7 +1214,8 @@ static void php_cli_server_log_response(php_cli_server_client *client, int statu /* error */ if (append_error_message) { - spprintf(&error_buf, 0, " - %s in %s on line %d", PG(last_error_message), PG(last_error_file), PG(last_error_lineno)); + spprintf(&error_buf, 0, " - %s in %s on line %d", + ZSTR_VAL(PG(last_error_message)), PG(last_error_file), PG(last_error_lineno)); if (!error_buf) { efree(basic_buf); if (message) { From 695396f86bcb9ec767b0c86feecf548a6dbfe106 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 29 May 2020 10:58:53 +0200 Subject: [PATCH 2/3] Avoid double formatting in php_error_docref Export a zend_error_zstr API for that purpose. --- Zend/zend.c | 24 +++++++++++++++++------- Zend/zend.h | 1 + main/main.c | 12 ++++++------ 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/Zend/zend.c b/Zend/zend.c index d6d699cb5acdd..0303e99b60ba7 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -1259,9 +1259,8 @@ ZEND_API zval *zend_get_configuration_directive(zend_string *name) /* {{{ */ } \ } while (0) -static ZEND_COLD void zend_error_va_list( - int orig_type, const char *error_filename, uint32_t error_lineno, - const char *format, va_list args) +static ZEND_COLD void zend_error_impl( + int orig_type, const char *error_filename, uint32_t error_lineno, zend_string *message) { zval params[4]; zval retval; @@ -1272,7 +1271,6 @@ static ZEND_COLD void zend_error_va_list( zend_stack delayed_oplines_stack; zend_class_entry *orig_fake_scope; int type = orig_type & E_ALL; - zend_string *message = zend_vstrpprintf(0, format, args); /* Report about uncaught exception in case of fatal errors */ if (EG(exception)) { @@ -1402,10 +1400,17 @@ static ZEND_COLD void zend_error_va_list( EG(exit_status) = 255; } } +} +/* }}} */ +static ZEND_COLD void zend_error_va_list( + int orig_type, const char *error_filename, uint32_t error_lineno, + const char *format, va_list args) +{ + zend_string *message = zend_vstrpprintf(0, format, args); + zend_error_impl(orig_type, error_filename, error_lineno, message); zend_string_release(message); } -/* }}} */ static ZEND_COLD void get_filename_lineno(int type, const char **filename, uint32_t *lineno) { /* Obtain relevant filename and lineno */ @@ -1495,7 +1500,6 @@ ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_at_noreturn( /* Should never reach this. */ abort(); } -/* }}} */ ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_noreturn(int type, const char *format, ...) { @@ -1510,7 +1514,13 @@ ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_noreturn(int type, const char * /* Should never reach this. */ abort(); } -/* }}} */ + +ZEND_API ZEND_COLD void zend_error_zstr(int type, zend_string *message) { + const char *filename; + uint32_t lineno; + get_filename_lineno(type, &filename, &lineno); + zend_error_impl(type, filename, lineno, message); +} ZEND_API ZEND_COLD void zend_throw_error(zend_class_entry *exception_ce, const char *format, ...) /* {{{ */ { diff --git a/Zend/zend.h b/Zend/zend.h index 6e43b296f4091..4b7307a0ce9f4 100644 --- a/Zend/zend.h +++ b/Zend/zend.h @@ -300,6 +300,7 @@ ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_noreturn(int type, const char * /* If filename is NULL the default filename is used. */ ZEND_API ZEND_COLD void zend_error_at(int type, const char *filename, uint32_t lineno, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 4, 5); ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_at_noreturn(int type, const char *filename, uint32_t lineno, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 4, 5); +ZEND_API ZEND_COLD void zend_error_zstr(int type, zend_string *message); ZEND_API ZEND_COLD void zend_throw_error(zend_class_entry *exception_ce, const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3); ZEND_API ZEND_COLD void zend_type_error(const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 1, 2); diff --git a/main/main.c b/main/main.c index 83d450f8307b9..0fe3334be5ce8 100644 --- a/main/main.c +++ b/main/main.c @@ -960,7 +960,7 @@ PHPAPI ZEND_COLD void php_verror(const char *docref, const char *params, int typ const char *function; int origin_len; char *origin; - char *message; + zend_string *message; int is_function = 0; /* get error text into buffer and escape for html if necessary */ @@ -1095,15 +1095,15 @@ PHPAPI ZEND_COLD void php_verror(const char *docref, const char *params, int typ } /* display html formatted or only show the additional links */ if (PG(html_errors)) { - spprintf(&message, 0, "%s [%s]: %s", origin, docref_root, docref, docref_target, docref, buffer); + message = zend_strpprintf(0, "%s [%s]: %s", origin, docref_root, docref, docref_target, docref, buffer); } else { - spprintf(&message, 0, "%s [%s%s%s]: %s", origin, docref_root, docref, docref_target, buffer); + message = zend_strpprintf(0, "%s [%s%s%s]: %s", origin, docref_root, docref, docref_target, buffer); } if (target) { efree(target); } } else { - spprintf(&message, 0, "%s: %s", origin, buffer); + message = zend_strpprintf(0, "%s: %s", origin, buffer); } if (replace_origin) { zend_string_free(replace_origin); @@ -1120,8 +1120,8 @@ PHPAPI ZEND_COLD void php_verror(const char *docref, const char *params, int typ efree(buffer); } - php_error(type, "%s", message); - efree(message); + zend_error_zstr(type, message); + zend_string_release(message); } /* }}} */ From 186a6a9377eed6aec53a291f444589733d6b068a Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 29 May 2020 12:32:51 +0200 Subject: [PATCH 3/3] Clear last error in more places Avoid leaking the last error between persistent and per-request phases. --- main/main.c | 51 +++++++++++++++++++++++---------------------------- 1 file changed, 23 insertions(+), 28 deletions(-) diff --git a/main/main.c b/main/main.c index 0fe3334be5ce8..0f6061f8e035d 100644 --- a/main/main.c +++ b/main/main.c @@ -1188,6 +1188,17 @@ PHPAPI void php_html_puts(const char *str, size_t size) } /* }}} */ +static void clear_last_error() { + if (PG(last_error_message)) { + zend_string_release(PG(last_error_message)); + PG(last_error_message) = NULL; + } + if (PG(last_error_file)) { + free(PG(last_error_file)); + PG(last_error_file) = NULL; + } +} + /* {{{ php_error_cb extended error handling function */ static ZEND_COLD void php_error_cb(int orig_type, const char *error_filename, const uint32_t error_lineno, zend_string *message) @@ -1243,16 +1254,7 @@ static ZEND_COLD void php_error_cb(int orig_type, const char *error_filename, co /* store the error if it has changed */ if (display) { - if (PG(last_error_message)) { - zend_string *s = PG(last_error_message); - PG(last_error_message) = NULL; - zend_string_release(s); - } - if (PG(last_error_file)) { - char *s = PG(last_error_file); - PG(last_error_file) = NULL; - free(s); - } + clear_last_error(); if (!error_filename) { error_filename = "Unknown"; } @@ -1587,14 +1589,7 @@ static zval *php_get_configuration_directive_for_zend(zend_string *name) */ static void php_free_request_globals(void) { - if (PG(last_error_message)) { - zend_string_release(PG(last_error_message)); - PG(last_error_message) = NULL; - } - if (PG(last_error_file)) { - free(PG(last_error_file)); - PG(last_error_file) = NULL; - } + clear_last_error(); if (PG(php_sys_temp_dir)) { efree(PG(php_sys_temp_dir)); PG(php_sys_temp_dir) = NULL; @@ -1876,12 +1871,12 @@ void php_request_shutdown(void *dummy) } } zend_end_try(); - /* 9. free request-bound globals */ - php_free_request_globals(); - - /* 10. Shutdown scanner/executor/compiler and restore ini entries */ + /* 9. Shutdown scanner/executor/compiler and restore ini entries */ zend_deactivate(); + /* 10. free request-bound globals */ + php_free_request_globals(); + /* 11. Call all extensions post-RSHUTDOWN functions */ zend_try { zend_post_deactivate_modules(); @@ -1953,12 +1948,10 @@ static void core_globals_ctor(php_core_globals *core_globals) */ static void core_globals_dtor(php_core_globals *core_globals) { - if (core_globals->last_error_message) { - zend_string_release(core_globals->last_error_message); - } - if (core_globals->last_error_file) { - free(core_globals->last_error_file); - } + /* These should have been freed earlier. */ + ZEND_ASSERT(!core_globals->last_error_message); + ZEND_ASSERT(!core_globals->last_error_file); + if (core_globals->disable_functions) { free(core_globals->disable_functions); } @@ -2391,6 +2384,8 @@ int php_module_startup(sapi_module_struct *sf, zend_module_entry *additional_mod sapi_deactivate(); module_startup = 0; + /* Don't leak errors from startup into the per-request phase. */ + clear_last_error(); shutdown_memory_manager(1, 0); virtual_cwd_activate();