From c5b286fb789550947408991a4cfb0bcae4fed595 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Wed, 19 Aug 2026 14:09:49 -0300 Subject: [PATCH] SAPI: Convert sapi_getenv to zend_string The old char* return was very ambigious at the SAPI level about ownership per platform (unix returned environ buffer, Win32 returned converted buffer), which it then estrduped at the SAPI frontend level. To clarify this, instead return an allocated zend_string in the SAPIs which we bubble up to consumers. There are some annoyances still; input filters have to shuffle some buffers because input filters expect to work on raw buffers, and phar does some mildly harrowing string mangling without zend_string. These should be cleaned up. There is also the question if SAPIs just bubbling up getenv when they lack a web server specific context i.e. from fcgi or Apache (CGI, FPM, Litespeed), as this seems not quite the right intent; CLI doesn't implement a getenv function for instance. --- Zend/zend.c | 2 +- Zend/zend.h | 4 ++-- Zend/zend_ini_parser.y | 8 ++++--- ext/opcache/ZendAccelerator.c | 2 +- ext/phar/phar_object.c | 23 +++++++++--------- ext/standard/basic_functions.c | 8 ++----- main/SAPI.c | 23 +++++++++--------- main/SAPI.h | 4 ++-- sapi/apache2handler/sapi_apache2.c | 4 ++-- sapi/cgi/cgi_main.c | 34 ++++++++++++++------------ sapi/fpm/fpm/fpm_main.c | 38 +++++++++++++++++++----------- sapi/litespeed/lsapi_main.c | 20 +++++++++------- 12 files changed, 92 insertions(+), 78 deletions(-) diff --git a/Zend/zend.c b/Zend/zend.c index 8643c248e6be..f3affe120a74 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -94,7 +94,7 @@ ZEND_API void (*zend_interrupt_function)(zend_execute_data *execute_data); ZEND_API void (*zend_error_cb)(int type, zend_string *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)(const char *name, size_t name_len); +ZEND_API zend_string *(*zend_getenv)(const char *name, size_t name_len); ZEND_API zend_string *(*zend_resolve_path)(zend_string *filename); ZEND_API zend_result (*zend_post_startup_cb)(void) = NULL; ZEND_API void (*zend_post_shutdown_cb)(void) = NULL; diff --git a/Zend/zend.h b/Zend/zend.h index a0f094324426..1abc5f892eaa 100644 --- a/Zend/zend.h +++ b/Zend/zend.h @@ -258,7 +258,7 @@ typedef struct _zend_utility_functions { zend_result (*stream_open_function)(zend_file_handle *handle); void (*printf_to_smart_string_function)(smart_string *buf, const char *format, va_list ap); void (*printf_to_smart_str_function)(smart_str *buf, const char *format, va_list ap); - char *(*getenv_function)(const char *name, size_t name_len); + zend_string *(*getenv_function)(const char *name, size_t name_len); zend_string *(*resolve_path_function)(zend_string *filename); zend_result (*random_bytes_function)(void *bytes, size_t size, char *errstr, size_t errstr_size); void (*random_bytes_insecure_function)(zend_random_bytes_insecure_state *state, void *bytes, size_t size); @@ -369,7 +369,7 @@ extern ZEND_API void (*zend_on_timeout)(int seconds); extern ZEND_API zend_result (*zend_stream_open_function)(zend_file_handle *handle); extern void (*zend_printf_to_smart_string)(smart_string *buf, const char *format, va_list ap); extern void (*zend_printf_to_smart_str)(smart_str *buf, const char *format, va_list ap); -extern ZEND_API char *(*zend_getenv)(const char *name, size_t name_len); +extern ZEND_API zend_string *(*zend_getenv)(const char *name, size_t name_len); extern ZEND_API zend_string *(*zend_resolve_path)(zend_string *filename); /* Generate 'size' random bytes into 'bytes' with the OS CSPRNG. */ extern ZEND_ATTRIBUTE_NONNULL ZEND_API zend_result (*zend_random_bytes)( diff --git a/Zend/zend_ini_parser.y b/Zend/zend_ini_parser.y index 47b05de4e692..6c375d3b414d 100644 --- a/Zend/zend_ini_parser.y +++ b/Zend/zend_ini_parser.y @@ -172,15 +172,17 @@ static void zend_ini_get_constant(zval *result, zval *name) static void zend_ini_get_var(zval *result, zval *name, zval *fallback) { zval *curval; + zend_string *sapi_envvar; char *envvar; /* Fetch configuration option value */ if ((curval = zend_get_configuration_directive(Z_STR_P(name))) != NULL) { ZVAL_NEW_STR(result, zend_string_init(Z_STRVAL_P(curval), Z_STRLEN_P(curval), ZEND_SYSTEM_INI)); /* ..or if not found, try ENV */ - } else if ((envvar = zend_getenv(Z_STRVAL_P(name), Z_STRLEN_P(name))) != NULL) { - ZVAL_NEW_STR(result, zend_string_init(envvar, strlen(envvar), ZEND_SYSTEM_INI)); - efree(envvar); + } else if ((sapi_envvar = zend_getenv(Z_STRVAL_P(name), Z_STRLEN_P(name))) != NULL) { + /* dup because persistent value may not be the same */ + ZVAL_NEW_STR(result, zend_string_dup(sapi_envvar, ZEND_SYSTEM_INI)); + zend_string_release(sapi_envvar); } else if ((envvar = getenv(Z_STRVAL_P(name))) != NULL) { ZVAL_NEW_STR(result, zend_string_init(envvar, strlen(envvar), ZEND_SYSTEM_INI)); /* ..or if not defined, try fallback value */ diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index f4bdcf3e7f7a..55b70e2352a7 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -5094,7 +5094,7 @@ static zend_result accel_finish_startup_preload(bool in_child) int (*orig_header_handler)(sapi_header_struct *sapi_header, sapi_header_op_enum op, sapi_headers_struct *sapi_headers) = sapi_module.header_handler; int (*orig_send_headers)(sapi_headers_struct *sapi_headers) = sapi_module.send_headers; void (*orig_send_header)(sapi_header_struct *sapi_header, void *server_context)= sapi_module.send_header; - char *(*orig_getenv)(const char *name, size_t name_len) = sapi_module.getenv; + zend_string *(*orig_getenv)(const char *name, size_t name_len) = sapi_module.getenv; size_t (*orig_ub_write)(const char *str, size_t str_length) = sapi_module.ub_write; void (*orig_flush)(void *server_context) = sapi_module.flush; #ifdef ZEND_SIGNALS diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c index 2fc17e7b9cea..0ce008f097bf 100644 --- a/ext/phar/phar_object.c +++ b/ext/phar/phar_object.c @@ -646,33 +646,34 @@ PHP_METHOD(Phar, webPhar) pt = estrndup(Z_STRVAL_P(z_script_name), Z_STRLEN_P(z_script_name)); } else { - char *testit = sapi_getenv("SCRIPT_NAME", sizeof("SCRIPT_NAME")-1); + zend_string *testit = sapi_getenv("SCRIPT_NAME", sizeof("SCRIPT_NAME")-1); if (!testit) { goto finish; } - pt = strstr(testit, basename); + pt = strstr(ZSTR_VAL(testit), basename); if (!pt) { - efree(testit); + zend_string_release(testit); goto finish; } - path_info = sapi_getenv("PATH_INFO", sizeof("PATH_INFO")-1); + zend_string *zs_path_info = sapi_getenv("PATH_INFO", sizeof("PATH_INFO")-1); - if (path_info) { - entry = path_info; - entry_len = strlen(entry); - spprintf(&path_info, 0, "%s%s", testit, path_info); + if (zs_path_info) { + entry = estrdup(ZSTR_VAL(zs_path_info)); + entry_len = strlen(entry); /* not ZSTR_LEN, b/c strdup truncates on nul */ + spprintf(&path_info, 0, "%s%s", ZSTR_VAL(testit), ZSTR_VAL(zs_path_info)); free_pathinfo = 1; + zend_string_release(zs_path_info); } else { - path_info = testit; + path_info = estrdup(ZSTR_VAL(testit)); free_pathinfo = 1; entry = estrndup("", 0); entry_len = 0; } - pt = estrndup(testit, (pt - testit) + (fname_len - (basename - fname))); - efree(testit); + pt = estrndup(ZSTR_VAL(testit), (pt - ZSTR_VAL(testit)) + (fname_len - (basename - fname))); + zend_string_release(testit); } not_cgi = 0; } else { diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index a87ab1b5ad62..645147ba7359 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -723,13 +723,9 @@ PHP_FUNCTION(getenv) } if (!local_only) { - /* SAPI method returns an emalloc()'d string */ - char *ptr = sapi_getenv(str, str_len); + zend_string *ptr = sapi_getenv(str, str_len); if (ptr) { - // TODO: avoid reallocation ??? - RETVAL_STRING(ptr); - efree(ptr); - return; + RETURN_STR(ptr); } } diff --git a/main/SAPI.c b/main/SAPI.c index 7de36af440c3..a20029979dfb 100644 --- a/main/SAPI.c +++ b/main/SAPI.c @@ -1008,9 +1008,9 @@ SAPI_API zend_stat_t *sapi_get_stat(void) } } -SAPI_API char *sapi_getenv(const char *name, size_t name_len) +SAPI_API zend_string *sapi_getenv(const char *name, size_t name_len) { - char *value, *tmp; + zend_string *value = NULL; if (!sapi_module.getenv) { return NULL; @@ -1019,19 +1019,18 @@ SAPI_API char *sapi_getenv(const char *name, size_t name_len) /* Ugly fix for HTTP_PROXY issue, see bug #72573 */ return NULL; } - tmp = sapi_module.getenv(name, name_len); - if (!tmp) { + value = sapi_module.getenv(name, name_len); + if (!value) { return NULL; } - value = estrdup(tmp); -#ifdef PHP_WIN32 - if (strlen(sapi_module.name) == sizeof("cgi-fcgi") - 1 && !strcmp(sapi_module.name, "cgi-fcgi")) { - /* XXX more modules to go, if needed. */ - free(tmp); - } -#endif if (sapi_module.input_filter) { - sapi_module.input_filter(PARSE_STRING, name, &value, strlen(value), NULL); + /* XXX: pretty ugly because input filters aren't zend_string yet. */ + char *tmp = estrdup(ZSTR_VAL(value)); + size_t tmp_len = 0; + sapi_module.input_filter(PARSE_STRING, name, &tmp, ZSTR_LEN(value), &tmp_len); + zend_string_release(value); + value = zend_string_init(tmp, tmp_len, 0); + efree(tmp); } return value; } diff --git a/main/SAPI.h b/main/SAPI.h index 2621b9184d1b..6d5496184132 100644 --- a/main/SAPI.h +++ b/main/SAPI.h @@ -217,7 +217,7 @@ SAPI_API zend_result sapi_register_input_filter(unsigned int (*input_filter)(int SAPI_API zend_result sapi_flush(void); SAPI_API zend_stat_t *sapi_get_stat(void); -SAPI_API char *sapi_getenv(const char *name, size_t name_len); +SAPI_API zend_string *sapi_getenv(const char *name, size_t name_len); SAPI_API char *sapi_get_default_content_type(void); SAPI_API void sapi_get_default_content_type_header(sapi_header_struct *default_header); @@ -246,7 +246,7 @@ struct _sapi_module_struct { size_t (*ub_write)(const char *str, size_t str_length); void (*flush)(void *server_context); zend_stat_t *(*get_stat)(void); - char *(*getenv)(const char *name, size_t name_len); + zend_string *(*getenv)(const char *name, size_t name_len); void (*sapi_error)(int type, const char *error_msg, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3); diff --git a/sapi/apache2handler/sapi_apache2.c b/sapi/apache2handler/sapi_apache2.c index 83b3f02fb743..cebd13093109 100644 --- a/sapi/apache2handler/sapi_apache2.c +++ b/sapi/apache2handler/sapi_apache2.c @@ -244,7 +244,7 @@ php_apache_sapi_read_cookies(void) return (char *) http_cookie; } -static char * +static zend_string * php_apache_sapi_getenv(const char *name, size_t name_len) { php_struct *ctx = SG(server_context); @@ -256,7 +256,7 @@ php_apache_sapi_getenv(const char *name, size_t name_len) env_var = apr_table_get(ctx->r->subprocess_env, name); - return (char *) env_var; + return env_var ? zend_string_init(env_var, strlen(env_var), 0) : NULL; } static void diff --git a/sapi/cgi/cgi_main.c b/sapi/cgi/cgi_main.c index a79eada2728b..ddb232f45eb6 100644 --- a/sapi/cgi/cgi_main.c +++ b/sapi/cgi/cgi_main.c @@ -525,10 +525,10 @@ static size_t sapi_fcgi_read_post(char *buffer, size_t count_bytes) } #ifdef PHP_WIN32 -/* The result needs to be freed! See sapi_getenv(). */ -static char *cgi_getenv_win32(const char *name, size_t name_len) +static zend_string *cgi_getenv_win32(const char *name, size_t name_len) { - char *ret = NULL; + zend_string *ret = NULL; + char *tmp = NULL; wchar_t *keyw, *valw; size_t size; int rc; @@ -548,9 +548,13 @@ static char *cgi_getenv_win32(const char *name, size_t name_len) rc = _wgetenv_s(&size, valw, size, keyw); if (!rc) { - ret = php_win32_cp_w_to_any(valw); + tmp = php_win32_cp_w_to_any(valw); + } + if (tmp) { + ret = zend_string_init(tmp, strlen(tmp), 0); } + free(tmp); free(keyw); efree(valw); @@ -558,16 +562,17 @@ static char *cgi_getenv_win32(const char *name, size_t name_len) } #endif -static char *sapi_cgi_getenv(const char *name, size_t name_len) +static zend_string *sapi_cgi_getenv(const char *name, size_t name_len) { #ifndef PHP_WIN32 - return getenv(name); + char *ret = getenv(name); + return ret ? zend_string_init(ret, strlen(ret), 0) : NULL; #else return cgi_getenv_win32(name, name_len); #endif } -static char *sapi_fcgi_getenv(const char *name, size_t name_len) +static zend_string *sapi_fcgi_getenv(const char *name, size_t name_len) { /* when php is started by mod_fastcgi, no regular environment * is provided to PHP. It is always sent to PHP at the start @@ -577,16 +582,15 @@ static char *sapi_fcgi_getenv(const char *name, size_t name_len) char *ret = fcgi_getenv(request, name, (int)name_len); #ifndef PHP_WIN32 - if (ret) return ret; - /* if cgi, or fastcgi and not found in fcgi env - check the regular environment */ - return getenv(name); + if (!ret) { + /* if cgi, or fastcgi and not found in fcgi env + check the regular environment */ + ret = getenv(name); + } + return ret ? zend_string_init(ret, strlen(ret), 0) : NULL; #else if (ret) { - /* The functions outside here don't know, where does it come - from. They'll need to free the returned memory as it's - not necessary from the fcgi env. */ - return strdup(ret); + return zend_string_init(ret, strlen(ret), 0); } /* if cgi, or fastcgi and not found in fcgi env check the regular environment */ diff --git a/sapi/fpm/fpm/fpm_main.c b/sapi/fpm/fpm/fpm_main.c index 655d66ce5f06..bf8884c87536 100644 --- a/sapi/fpm/fpm/fpm_main.c +++ b/sapi/fpm/fpm/fpm_main.c @@ -100,7 +100,7 @@ static int parent = 1; static int request_body_fd; static int fpm_is_running = 0; -static char *sapi_cgibin_getenv(const char *name, size_t name_len); +static zend_string *sapi_cgibin_getenv(const char *name, size_t name_len); static void fastcgi_ini_parser(zval *arg1, zval *arg2, zval *arg3, int callback_type, void *arg); #define PHP_MODE_STANDARD 1 @@ -461,16 +461,19 @@ static size_t sapi_cgi_read_post(char *buffer, size_t count_bytes) /* {{{ */ } /* }}} */ -static char *sapi_cgibin_getenv(const char *name, size_t name_len) /* {{{ */ +static zend_string *sapi_cgibin_getenv(const char *name, size_t name_len) /* {{{ */ { + char *var = NULL; /* if fpm has started, use fcgi env */ if (fpm_is_running) { fcgi_request *request = (fcgi_request*) SG(server_context); - return fcgi_getenv(request, name, name_len); + var = fcgi_getenv(request, name, name_len); } - - /* if fpm has not started yet, use std env */ - return getenv(name); + if (!var) { + /* if fpm has not started yet, use std env */ + var = getenv(name); /* XXX: should we? */ + } + return var ? zend_string_init(var, strlen(var), 0) : NULL; } /* }}} */ @@ -564,20 +567,27 @@ static void sapi_cgi_register_variables(zval *track_vars_array) /* {{{ */ if (CGIG(fix_pathinfo)) { char *script_name = SG(request_info).request_uri; unsigned int script_name_len = script_name ? strlen(script_name) : 0; - char *path_info = sapi_cgibin_getenv("PATH_INFO", sizeof("PATH_INFO") - 1); - unsigned int path_info_len = path_info ? strlen(path_info) : 0; - - php_self_len = script_name_len + path_info_len; - /* Concat script_name and path_info into php_self */ - php_self = zend_cstr_concat( - script_name, script_name_len, - path_info, path_info_len); + zend_string *path_info = sapi_cgibin_getenv("PATH_INFO", sizeof("PATH_INFO") - 1); + + if (path_info) { + php_self_len = script_name_len + ZSTR_LEN(path_info); + /* Concat script_name and path_info into php_self */ + php_self = zend_cstr_concat( + script_name, script_name_len, + ZSTR_VAL(path_info), ZSTR_LEN(path_info)); + } else { + php_self_len = script_name_len; + php_self = estrdup(script_name ? script_name : ""); + } /* Build the special-case PHP_SELF variable for the CGI version */ if (sapi_module.input_filter(PARSE_SERVER, "PHP_SELF", &php_self, php_self_len, &php_self_len)) { php_register_variable_safe("PHP_SELF", php_self, php_self_len, track_vars_array); } efree(php_self); + if (path_info) { + zend_string_release(path_info); + } } else { php_self = SG(request_info).request_uri ? SG(request_info).request_uri : ""; php_self_len = strlen(php_self); diff --git a/sapi/litespeed/lsapi_main.c b/sapi/litespeed/lsapi_main.c index 2acbd432ee84..ac28b2d8d04f 100644 --- a/sapi/litespeed/lsapi_main.c +++ b/sapi/litespeed/lsapi_main.c @@ -184,13 +184,15 @@ static int sapi_lsapi_deactivate(void) /* {{{ sapi_lsapi_getenv */ -static char *sapi_lsapi_getenv(const char * name, size_t name_len ) +static zend_string *sapi_lsapi_getenv(const char * name, size_t name_len ) { + char *var = NULL; if ( lsapi_mode ) { - return LSAPI_GetEnv( name ); + var = LSAPI_GetEnv( name ); } else { - return getenv( name ); + var = getenv( name ); /* XXX: Should a SAPI just reflect getenv? */ } + return var ? zend_string_init( var, strlen( var ), 0 ) : NULL; } /* }}} */ @@ -538,7 +540,8 @@ static int lsapi_activate_user_ini(void); static int sapi_lsapi_activate(void) { - char *path, *server_name; + char *path; + zend_string *server_name; size_t path_len, server_name_len; /* PATH_TRANSLATED should be defined at this stage but better safe than sorry :) */ @@ -550,11 +553,10 @@ static int sapi_lsapi_activate(void) server_name = sapi_lsapi_getenv("SERVER_NAME", 0); /* SERVER_NAME should also be defined at this stage..but better check it anyway */ if (server_name) { - server_name_len = strlen(server_name); - server_name = estrndup(server_name, server_name_len); - zend_str_tolower(server_name, server_name_len); - php_ini_activate_per_host_config(server_name, server_name_len); - efree(server_name); + zend_string *lowered = zend_string_tolower(server_name); + php_ini_activate_per_host_config(ZSTR_VAL(lowered), ZSTR_LEN(lowered)); + zend_string_release(lowered); + zend_string_release(server_name); } }