From e18a755ac71ea49f1f07254e8e163a3507bf22e6 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 27 May 2022 15:35:14 +0100 Subject: [PATCH 1/5] Session: use more appropriate types --- ext/session/mod_files.c | 6 +-- ext/session/mod_mm.c | 25 ++++++------ ext/session/mod_user.c | 2 +- ext/session/mod_user_class.c | 8 ++-- ext/session/php_session.h | 52 ++++++++++++------------ ext/session/session.c | 77 ++++++++++++++++-------------------- 6 files changed, 81 insertions(+), 89 deletions(-) diff --git a/ext/session/mod_files.c b/ext/session/mod_files.c index 0972171555350..a0401019a725a 100644 --- a/ext/session/mod_files.c +++ b/ext/session/mod_files.c @@ -155,7 +155,7 @@ static void ps_files_open(ps_files *data, const char *key) { char buf[MAXPATHLEN]; #if !defined(O_NOFOLLOW) || !defined(PHP_WIN32) - struct stat sbuf; + struct stat sbuf = {0}; #endif int ret; @@ -226,7 +226,7 @@ static void ps_files_open(ps_files *data, const char *key) } } -static int ps_files_write(ps_files *data, zend_string *key, zend_string *val) +static zend_result ps_files_write(ps_files *data, zend_string *key, zend_string *val) { size_t n = 0; @@ -337,7 +337,7 @@ static int ps_files_cleanup_dir(const char *dirname, zend_long maxlifetime) return (nrdels); } -static int ps_files_key_exists(ps_files *data, const char *key) +static zend_result ps_files_key_exists(ps_files *data, const char *key) { char buf[MAXPATHLEN]; zend_stat_t sbuf = {0}; diff --git a/ext/session/mod_mm.c b/ext/session/mod_mm.c index baec6f5a7fda0..9610fb75bb4cc 100644 --- a/ext/session/mod_mm.c +++ b/ext/session/mod_mm.c @@ -64,7 +64,7 @@ static ps_mm *ps_mm_instance = NULL; # define ps_mm_debug(a) #endif -static inline uint32_t ps_sd_hash(const char *data, int len) +static inline uint32_t ps_sd_hash(const char *data, size_t len) { uint32_t h; const char *e = data + len; @@ -110,7 +110,7 @@ static ps_sd *ps_sd_new(ps_mm *data, const char *key) { uint32_t hv, slot; ps_sd *sd; - int keylen; + size_t keylen; keylen = strlen(key); @@ -172,7 +172,7 @@ static void ps_sd_destroy(ps_mm *data, ps_sd *sd) mm_free(data->mm, sd); } -static ps_sd *ps_sd_lookup(ps_mm *data, const char *key, int rw) +static ps_sd *ps_sd_lookup(ps_mm *data, const char *key, bool rw) { uint32_t hv, slot; ps_sd *ret, *prev; @@ -201,7 +201,7 @@ static ps_sd *ps_sd_lookup(ps_mm *data, const char *key, int rw) return ret; } -static int ps_mm_key_exists(ps_mm *data, const char *key) +static zend_result ps_mm_key_exists(ps_mm *data, const char *key) { ps_sd *sd; @@ -221,7 +221,7 @@ const ps_module ps_mod_mm = { #define PS_MM_DATA ps_mm *data = PS_GET_MOD_DATA() -static int ps_mm_initialize(ps_mm *data, const char *path) +static zend_result ps_mm_initialize(ps_mm *data, const char *path) { data->owner = getpid(); data->mm = mm_create(0, path); @@ -242,7 +242,6 @@ static int ps_mm_initialize(ps_mm *data, const char *path) static void ps_mm_destroy(ps_mm *data) { - int h; ps_sd *sd, *next; /* This function is called during each module shutdown, @@ -252,7 +251,7 @@ static void ps_mm_destroy(ps_mm *data) return; } - for (h = 0; h < data->hash_max + 1; h++) { + for (int h = 0; h < data->hash_max + 1; h++) { for (sd = data->hash[h]; sd; sd = next) { next = sd->next; ps_sd_destroy(data, sd); @@ -266,11 +265,11 @@ static void ps_mm_destroy(ps_mm *data) PHP_MINIT_FUNCTION(ps_mm) { - int save_path_len = strlen(PS(save_path)); - int mod_name_len = strlen(sapi_module.name); - int euid_len; + size_t save_path_len = strlen(PS(save_path)); + size_t mod_name_len = strlen(sapi_module.name); + size_t euid_len; char *ps_mm_path, euid[30]; - int ret; + zend_result ret; ps_mm_instance = calloc(sizeof(*ps_mm_instance), 1); if (!ps_mm_instance) { @@ -302,7 +301,7 @@ PHP_MINIT_FUNCTION(ps_mm) efree(ps_mm_path); - if (ret != SUCCESS) { + if (ret == FAILURE) { free(ps_mm_instance); ps_mm_instance = NULL; return FAILURE; @@ -344,7 +343,7 @@ PS_READ_FUNC(mm) { PS_MM_DATA; ps_sd *sd; - int ret = FAILURE; + zend_result ret = FAILURE; mm_lock(data->mm, MM_LOCK_RD); diff --git a/ext/session/mod_user.c b/ext/session/mod_user.c index 214275dbac4c0..005e6d063918e 100644 --- a/ext/session/mod_user.c +++ b/ext/session/mod_user.c @@ -47,7 +47,7 @@ static void ps_call_handler(zval *func, int argc, zval *argv, zval *retval) #define STDVARS \ zval retval; \ - int ret = FAILURE + zend_result ret = FAILURE #define PSF(a) PS(mod_user_names).name.ps_##a diff --git a/ext/session/mod_user_class.c b/ext/session/mod_user_class.c index 1332ecddf9409..d5b71da52c282 100644 --- a/ext/session/mod_user_class.c +++ b/ext/session/mod_user_class.c @@ -39,7 +39,7 @@ PHP_METHOD(SessionHandler, open) { char *save_path = NULL, *session_name = NULL; size_t save_path_len, session_name_len; - int ret; + zend_result ret; if (zend_parse_parameters(ZEND_NUM_ARGS(), "ss", &save_path, &save_path_len, &session_name, &session_name_len) == FAILURE) { RETURN_THROWS(); @@ -56,14 +56,14 @@ PHP_METHOD(SessionHandler, open) zend_bailout(); } zend_end_try(); - RETVAL_BOOL(SUCCESS == ret); + RETURN_BOOL(SUCCESS == ret); } /* }}} */ /* {{{ Wraps the old close handler */ PHP_METHOD(SessionHandler, close) { - int ret; + zend_result ret; // don't return on failure, since not closing the default handler // could result in memory leaks or other nasties @@ -80,7 +80,7 @@ PHP_METHOD(SessionHandler, close) zend_bailout(); } zend_end_try(); - RETVAL_BOOL(SUCCESS == ret); + RETURN_BOOL(SUCCESS == ret); } /* }}} */ diff --git a/ext/session/php_session.h b/ext/session/php_session.h index ef139196ca900..aa4a8209507d4 100644 --- a/ext/session/php_session.h +++ b/ext/session/php_session.h @@ -39,29 +39,29 @@ typedef struct ps_module_struct { const char *s_name; - int (*s_open)(PS_OPEN_ARGS); - int (*s_close)(PS_CLOSE_ARGS); - int (*s_read)(PS_READ_ARGS); - int (*s_write)(PS_WRITE_ARGS); - int (*s_destroy)(PS_DESTROY_ARGS); + zend_result (*s_open)(PS_OPEN_ARGS); + zend_result (*s_close)(PS_CLOSE_ARGS); + zend_result (*s_read)(PS_READ_ARGS); + zend_result (*s_write)(PS_WRITE_ARGS); + zend_result (*s_destroy)(PS_DESTROY_ARGS); zend_long (*s_gc)(PS_GC_ARGS); zend_string *(*s_create_sid)(PS_CREATE_SID_ARGS); - int (*s_validate_sid)(PS_VALIDATE_SID_ARGS); - int (*s_update_timestamp)(PS_UPDATE_TIMESTAMP_ARGS); + zend_result (*s_validate_sid)(PS_VALIDATE_SID_ARGS); + zend_result (*s_update_timestamp)(PS_UPDATE_TIMESTAMP_ARGS); } ps_module; #define PS_GET_MOD_DATA() *mod_data #define PS_SET_MOD_DATA(a) *mod_data = (a) -#define PS_OPEN_FUNC(x) int ps_open_##x(PS_OPEN_ARGS) -#define PS_CLOSE_FUNC(x) int ps_close_##x(PS_CLOSE_ARGS) -#define PS_READ_FUNC(x) int ps_read_##x(PS_READ_ARGS) -#define PS_WRITE_FUNC(x) int ps_write_##x(PS_WRITE_ARGS) -#define PS_DESTROY_FUNC(x) int ps_delete_##x(PS_DESTROY_ARGS) +#define PS_OPEN_FUNC(x) zend_result ps_open_##x(PS_OPEN_ARGS) +#define PS_CLOSE_FUNC(x) zend_result ps_close_##x(PS_CLOSE_ARGS) +#define PS_READ_FUNC(x) zend_result ps_read_##x(PS_READ_ARGS) +#define PS_WRITE_FUNC(x) zend_result ps_write_##x(PS_WRITE_ARGS) +#define PS_DESTROY_FUNC(x) zend_result ps_delete_##x(PS_DESTROY_ARGS) #define PS_GC_FUNC(x) zend_long ps_gc_##x(PS_GC_ARGS) #define PS_CREATE_SID_FUNC(x) zend_string *ps_create_sid_##x(PS_CREATE_SID_ARGS) -#define PS_VALIDATE_SID_FUNC(x) int ps_validate_sid_##x(PS_VALIDATE_SID_ARGS) -#define PS_UPDATE_TIMESTAMP_FUNC(x) int ps_update_timestamp_##x(PS_UPDATE_TIMESTAMP_ARGS) +#define PS_VALIDATE_SID_FUNC(x) zend_result ps_validate_sid_##x(PS_VALIDATE_SID_ARGS) +#define PS_UPDATE_TIMESTAMP_FUNC(x) zend_result ps_update_timestamp_##x(PS_UPDATE_TIMESTAMP_ARGS) /* Legacy save handler module definitions */ #define PS_FUNCS(x) \ @@ -224,7 +224,7 @@ ZEND_TSRMLS_CACHE_EXTERN() typedef struct ps_serializer_struct { const char *name; zend_string *(*encode)(PS_SERIALIZER_ENCODE_ARGS); - int (*decode)(PS_SERIALIZER_DECODE_ARGS); + zend_result (*decode)(PS_SERIALIZER_DECODE_ARGS); } ps_serializer; #define PS_SERIALIZER_ENCODE_NAME(x) ps_srlzr_encode_##x @@ -233,7 +233,7 @@ typedef struct ps_serializer_struct { #define PS_SERIALIZER_ENCODE_FUNC(x) \ zend_string *PS_SERIALIZER_ENCODE_NAME(x)(PS_SERIALIZER_ENCODE_ARGS) #define PS_SERIALIZER_DECODE_FUNC(x) \ - int PS_SERIALIZER_DECODE_NAME(x)(PS_SERIALIZER_DECODE_ARGS) + zend_result PS_SERIALIZER_DECODE_NAME(x)(PS_SERIALIZER_DECODE_ARGS) #define PS_SERIALIZER_FUNCS(x) \ PS_SERIALIZER_ENCODE_FUNC(x); \ @@ -245,30 +245,30 @@ typedef struct ps_serializer_struct { /* default create id function */ PHPAPI zend_string *php_session_create_id(PS_CREATE_SID_ARGS); /* Dummy PS module functions */ -PHPAPI int php_session_validate_sid(PS_VALIDATE_SID_ARGS); -PHPAPI int php_session_update_timestamp(PS_UPDATE_TIMESTAMP_ARGS); +PHPAPI zend_result php_session_validate_sid(PS_VALIDATE_SID_ARGS); +PHPAPI zend_result php_session_update_timestamp(PS_UPDATE_TIMESTAMP_ARGS); PHPAPI void session_adapt_url(const char *url, size_t url_len, char **new_url, size_t *new_len); -PHPAPI int php_session_destroy(void); +PHPAPI zend_result php_session_destroy(void); PHPAPI void php_add_session_var(zend_string *name); PHPAPI zval *php_set_session_var(zend_string *name, zval *state_val, php_unserialize_data_t *var_hash); PHPAPI zval *php_get_session_var(zend_string *name); -PHPAPI int php_session_register_module(const ps_module *); +PHPAPI zend_result php_session_register_module(const ps_module *); -PHPAPI int php_session_register_serializer(const char *name, +PHPAPI zend_result php_session_register_serializer(const char *name, zend_string *(*encode)(PS_SERIALIZER_ENCODE_ARGS), - int (*decode)(PS_SERIALIZER_DECODE_ARGS)); + zend_result (*decode)(PS_SERIALIZER_DECODE_ARGS)); -PHPAPI int php_session_start(void); -PHPAPI int php_session_flush(int write); +PHPAPI zend_result php_session_start(void); +PHPAPI zend_result php_session_flush(int write); PHPAPI const ps_module *_php_find_ps_module(const char *name); PHPAPI const ps_serializer *_php_find_ps_serializer(const char *name); -PHPAPI int php_session_valid_key(const char *key); -PHPAPI int php_session_reset_id(void); +PHPAPI zend_result php_session_valid_key(const char *key); +PHPAPI zend_result php_session_reset_id(void); #define PS_ADD_VARL(name) do { \ php_add_session_var(name); \ diff --git a/ext/session/session.c b/ext/session/session.c index 5f3d3384d21ea..4f7dd5f5776d3 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -149,9 +149,9 @@ static inline void php_rshutdown_session_globals(void) /* {{{ */ } /* }}} */ -PHPAPI int php_session_destroy(void) /* {{{ */ +PHPAPI zend_result php_session_destroy(void) /* {{{ */ { - int retval = SUCCESS; + zend_result retval = SUCCESS; if (PS(session_status) != php_session_active) { php_error_docref(NULL, E_WARNING, "Trying to destroy uninitialized session"); @@ -240,7 +240,7 @@ static zend_string *php_session_encode(void) /* {{{ */ } /* }}} */ -static int php_session_decode(zend_string *data) /* {{{ */ +static zend_result php_session_decode(zend_string *data) /* {{{ */ { if (!PS(serializer)) { php_error_docref(NULL, E_WARNING, "Unknown session.serialize_handler. Failed to decode session object"); @@ -327,12 +327,12 @@ PHPAPI zend_string *php_session_create_id(PS_CREATE_SID_ARGS) /* {{{ */ /* Default session id char validation function allowed by ps_modules. * If you change the logic here, please also update the error message in * ps_modules appropriately */ -PHPAPI int php_session_valid_key(const char *key) /* {{{ */ +PHPAPI zend_result php_session_valid_key(const char *key) /* {{{ */ { size_t len; const char *p; char c; - int ret = SUCCESS; + zend_result ret = SUCCESS; for (p = key; (c = *p); p++) { /* valid characters are a..z,A..Z,0..9 */ @@ -378,7 +378,7 @@ static zend_long php_session_gc(bool immediate) /* {{{ */ return num; } /* }}} */ -static int php_session_initialize(void) /* {{{ */ +static zend_result php_session_initialize(void) /* {{{ */ { zend_string *val = NULL; @@ -468,7 +468,7 @@ static int php_session_initialize(void) /* {{{ */ static void php_session_save_current_state(int write) /* {{{ */ { - int ret = FAILURE; + zend_result ret = FAILURE; if (write) { IF_SESSION_VARS() { @@ -864,7 +864,7 @@ PS_SERIALIZER_DECODE_FUNC(php_serialize) /* {{{ */ const char *endptr = val + vallen; zval session_vars; php_unserialize_data_t var_hash; - int result; + bool result; zend_string *var_name = zend_string_init("_SESSION", sizeof("_SESSION") - 1, 0); ZVAL_NULL(&session_vars); @@ -921,7 +921,6 @@ PS_SERIALIZER_DECODE_FUNC(php_binary) /* {{{ */ { const char *p; const char *endptr = val + vallen; - int namelen; zend_string *name; php_unserialize_data_t var_hash; zval *current, rv; @@ -929,7 +928,8 @@ PS_SERIALIZER_DECODE_FUNC(php_binary) /* {{{ */ PHP_VAR_UNSERIALIZE_INIT(var_hash); for (p = val; p < endptr; ) { - namelen = ((unsigned char)(*p)) & (~PS_BIN_UNDEF); + // Can this be changed to size_t? + int namelen = ((unsigned char)(*p)) & (~PS_BIN_UNDEF); if (namelen < 0 || namelen > PS_BIN_MAX || (p + namelen) >= endptr) { PHP_VAR_UNSERIALIZE_DESTROY(var_hash); @@ -993,7 +993,7 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */ const char *endptr = val + vallen; ptrdiff_t namelen; zend_string *name; - int retval = SUCCESS; + zend_result retval = SUCCESS; php_unserialize_data_t var_hash; zval *current, rv; @@ -1045,12 +1045,11 @@ static ps_serializer ps_serializers[MAX_SERIALIZERS + 1] = { PS_SERIALIZER_ENTRY(php_binary) }; -PHPAPI int php_session_register_serializer(const char *name, zend_string *(*encode)(PS_SERIALIZER_ENCODE_ARGS), int (*decode)(PS_SERIALIZER_DECODE_ARGS)) /* {{{ */ +PHPAPI zend_result php_session_register_serializer(const char *name, zend_string *(*encode)(PS_SERIALIZER_ENCODE_ARGS), zend_result (*decode)(PS_SERIALIZER_DECODE_ARGS)) /* {{{ */ { - int ret = FAILURE; - int i; + zend_result ret = FAILURE; - for (i = 0; i < MAX_SERIALIZERS; i++) { + for (int i = 0; i < MAX_SERIALIZERS; i++) { if (ps_serializers[i].name == NULL) { ps_serializers[i].name = name; ps_serializers[i].encode = encode; @@ -1076,12 +1075,11 @@ static const ps_module *ps_modules[MAX_MODULES + 1] = { ps_user_ptr }; -PHPAPI int php_session_register_module(const ps_module *ptr) /* {{{ */ +PHPAPI zend_result php_session_register_module(const ps_module *ptr) /* {{{ */ { int ret = FAILURE; - int i; - for (i = 0; i < MAX_MODULES; i++) { + for (int i = 0; i < MAX_MODULES; i++) { if (!ps_modules[i]) { ps_modules[i] = ptr; ret = SUCCESS; @@ -1093,12 +1091,12 @@ PHPAPI int php_session_register_module(const ps_module *ptr) /* {{{ */ /* }}} */ /* Dummy PS module function */ -PHPAPI int php_session_validate_sid(PS_VALIDATE_SID_ARGS) { +PHPAPI zend_result php_session_validate_sid(PS_VALIDATE_SID_ARGS) { return SUCCESS; } /* Dummy PS module function */ -PHPAPI int php_session_update_timestamp(PS_UPDATE_TIMESTAMP_ARGS) { +PHPAPI zend_result php_session_update_timestamp(PS_UPDATE_TIMESTAMP_ARGS) { return SUCCESS; } @@ -1306,7 +1304,7 @@ static void php_session_remove_cookie(void) { efree(session_cookie); } -static int php_session_send_cookie(void) /* {{{ */ +static zend_result php_session_send_cookie(void) /* {{{ */ { smart_str ncookie = {0}; zend_string *date_fmt = NULL; @@ -1436,7 +1434,7 @@ static void ppid2sid(zval *ppid) { } -PHPAPI int php_session_reset_id(void) /* {{{ */ +PHPAPI zend_result php_session_reset_id(void) /* {{{ */ { int module_number = PS(module_number); zval *sid, *data, *ppid; @@ -1506,7 +1504,7 @@ PHPAPI int php_session_reset_id(void) /* {{{ */ /* }}} */ -PHPAPI int php_session_start(void) /* {{{ */ +PHPAPI zend_result php_session_start(void) /* {{{ */ { zval *ppid; zval *data; @@ -1629,7 +1627,7 @@ PHPAPI int php_session_start(void) /* {{{ */ } /* }}} */ -PHPAPI int php_session_flush(int write) /* {{{ */ +PHPAPI zend_result php_session_flush(int write) /* {{{ */ { if (PS(session_status) == php_session_active) { php_session_save_current_state(write); @@ -1640,7 +1638,7 @@ PHPAPI int php_session_flush(int write) /* {{{ */ } /* }}} */ -static int php_session_abort(void) /* {{{ */ +static zend_result php_session_abort(void) /* {{{ */ { if (PS(session_status) == php_session_active) { if (PS(mod_data) || PS(mod_user_implemented)) { @@ -1653,7 +1651,7 @@ static int php_session_abort(void) /* {{{ */ } /* }}} */ -static int php_session_reset(void) /* {{{ */ +static zend_result php_session_reset(void) /* {{{ */ { if (PS(session_status) == php_session_active && php_session_initialize() == SUCCESS) { @@ -1689,7 +1687,7 @@ PHP_FUNCTION(session_set_cookie_params) bool secure = 0, secure_null = 1; bool httponly = 0, httponly_null = 1; zend_string *ini_name; - int result; + zend_result result; int found = 0; if (!PS(use_cookies)) { @@ -1947,7 +1945,7 @@ PHP_FUNCTION(session_module_name) } /* }}} */ -static int save_handler_check_session(void) { +static zend_result save_handler_check_session(void) { if (PS(session_status) == php_session_active) { php_error_docref(NULL, E_WARNING, "Session save handler cannot be changed when a session is active"); return FAILURE; @@ -2169,9 +2167,8 @@ PHP_FUNCTION(session_save_path) PHP_FUNCTION(session_id) { zend_string *name = NULL; - int argc = ZEND_NUM_ARGS(); - if (zend_parse_parameters(argc, "|S!", &name) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "|S!", &name) == FAILURE) { RETURN_THROWS(); } @@ -2238,7 +2235,7 @@ PHP_FUNCTION(session_regenerate_id) RETURN_FALSE; } } else { - int ret; + zend_result ret; data = php_session_encode(); if (data) { ret = PS(mod)->s_write(&PS(mod_data), PS(id), data, PS(gc_maxlifetime)); @@ -2471,8 +2468,8 @@ PHP_FUNCTION(session_decode) } /* }}} */ -static int php_session_start_set_ini(zend_string *varname, zend_string *new_value) { - int ret; +static zend_result php_session_start_set_ini(zend_string *varname, zend_string *new_value) { + zend_result ret; smart_str buf ={0}; smart_str_appends(&buf, "session"); smart_str_appendc(&buf, '.'); @@ -2715,7 +2712,7 @@ PHP_FUNCTION(session_register_shutdown) * Module Setup and Destruction * ******************************** */ -static int php_rinit_session(bool auto_start) /* {{{ */ +static zend_result php_rinit_session(bool auto_start) /* {{{ */ { php_rinit_session_globals(); @@ -2759,8 +2756,6 @@ static PHP_RINIT_FUNCTION(session) /* {{{ */ static PHP_RSHUTDOWN_FUNCTION(session) /* {{{ */ { - int i; - if (PS(session_status) == php_session_active) { zend_try { php_session_flush(1); @@ -2769,7 +2764,7 @@ static PHP_RSHUTDOWN_FUNCTION(session) /* {{{ */ php_rshutdown_session_globals(); /* this should NOT be done in php_rshutdown_session_globals() */ - for (i = 0; i < PS_NUM_APIS; i++) { + for (int i = 0; i < PS_NUM_APIS; i++) { if (!Z_ISUNDEF(PS(mod_user_names).names[i])) { zval_ptr_dtor(&PS(mod_user_names).names[i]); ZVAL_UNDEF(&PS(mod_user_names).names[i]); @@ -2782,8 +2777,6 @@ static PHP_RSHUTDOWN_FUNCTION(session) /* {{{ */ static PHP_GINIT_FUNCTION(ps) /* {{{ */ { - int i; - #if defined(COMPILE_DL_SESSION) && defined(ZTS) ZEND_TSRMLS_CACHE_UPDATE(); #endif @@ -2801,7 +2794,7 @@ static PHP_GINIT_FUNCTION(ps) /* {{{ */ ps_globals->mod_user_is_open = 0; ps_globals->session_vars = NULL; ps_globals->set_handler = 0; - for (i = 0; i < PS_NUM_APIS; i++) { + for (int i = 0; i < PS_NUM_APIS; i++) { ZVAL_UNDEF(&ps_globals->mod_user_names.names[i]); } ZVAL_UNDEF(&ps_globals->http_session_vars); @@ -3020,10 +3013,10 @@ static void php_session_rfc1867_cleanup(php_session_rfc1867_progress *progress) php_session_flush(1); } /* }}} */ -static int php_session_rfc1867_callback(unsigned int event, void *event_data, void **extra) /* {{{ */ +static zend_result php_session_rfc1867_callback(unsigned int event, void *event_data, void **extra) /* {{{ */ { php_session_rfc1867_progress *progress; - int retval = SUCCESS; + zend_result retval = SUCCESS; if (php_session_rfc1867_orig_callback) { retval = php_session_rfc1867_orig_callback(event, event_data, extra); From 1b996823d9a98078031cf135731629c5fa0bc42a Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 27 May 2022 15:44:07 +0100 Subject: [PATCH 2/5] Session: Use zend_string* consistently for key in mod_files --- ext/session/mod_files.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/ext/session/mod_files.c b/ext/session/mod_files.c index a0401019a725a..e6098e7feb050 100644 --- a/ext/session/mod_files.c +++ b/ext/session/mod_files.c @@ -104,20 +104,18 @@ const ps_module ps_mod_files = { }; -static char *ps_files_path_create(char *buf, size_t buflen, ps_files *data, const char *key) +static char *ps_files_path_create(char *buf, size_t buflen, ps_files *data, const zend_string *key) { - size_t key_len; const char *p; int i; size_t n; - key_len = strlen(key); - if (!data || key_len <= data->dirdepth || - buflen < (strlen(data->basedir) + 2 * data->dirdepth + key_len + 5 + sizeof(FILE_PREFIX))) { + if (!data || ZSTR_LEN(key) <= data->dirdepth || + buflen < (strlen(data->basedir) + 2 * data->dirdepth + ZSTR_LEN(key) + 5 + sizeof(FILE_PREFIX))) { return NULL; } - p = key; + p = ZSTR_VAL(key); memcpy(buf, data->basedir, data->basedir_len); n = data->basedir_len; buf[n++] = PHP_DIR_SEPARATOR; @@ -127,8 +125,8 @@ static char *ps_files_path_create(char *buf, size_t buflen, ps_files *data, cons } memcpy(buf + n, FILE_PREFIX, sizeof(FILE_PREFIX) - 1); n += sizeof(FILE_PREFIX) - 1; - memcpy(buf + n, key, key_len); - n += key_len; + memcpy(buf + n, ZSTR_VAL(key), ZSTR_LEN(key)); + n += ZSTR_LEN(key); buf[n] = '\0'; return buf; @@ -151,7 +149,7 @@ static void ps_files_close(ps_files *data) } } -static void ps_files_open(ps_files *data, const char *key) +static void ps_files_open(ps_files *data, const zend_string *key) { char buf[MAXPATHLEN]; #if !defined(O_NOFOLLOW) || !defined(PHP_WIN32) @@ -159,7 +157,7 @@ static void ps_files_open(ps_files *data, const char *key) #endif int ret; - if (data->fd < 0 || !data->lastkey || strcmp(key, data->lastkey)) { + if (data->fd < 0 || !data->lastkey || strcmp(ZSTR_VAL(key), data->lastkey)) { if (data->lastkey) { efree(data->lastkey); data->lastkey = NULL; @@ -167,7 +165,7 @@ static void ps_files_open(ps_files *data, const char *key) ps_files_close(data); - if (php_session_valid_key(key) == FAILURE) { + if (php_session_valid_key(ZSTR_VAL(key)) == FAILURE) { php_error_docref(NULL, E_WARNING, "Session ID is too long or contains illegal characters. Only the A-Z, a-z, 0-9, \"-\", and \",\" characters are allowed"); return; } @@ -177,7 +175,7 @@ static void ps_files_open(ps_files *data, const char *key) return; } - data->lastkey = estrdup(key); + data->lastkey = estrdup(ZSTR_VAL(key)); /* O_NOFOLLOW to prevent us from following evil symlinks */ #ifdef O_NOFOLLOW @@ -233,7 +231,7 @@ static zend_result ps_files_write(ps_files *data, zend_string *key, zend_string /* PS(id) may be changed by calling session_regenerate_id(). Re-initialization should be tried here. ps_files_open() checks data->lastkey and reopen when it is needed. */ - ps_files_open(data, ZSTR_VAL(key)); + ps_files_open(data, key); if (data->fd < 0) { return FAILURE; } @@ -337,7 +335,7 @@ static int ps_files_cleanup_dir(const char *dirname, zend_long maxlifetime) return (nrdels); } -static zend_result ps_files_key_exists(ps_files *data, const char *key) +static zend_result ps_files_key_exists(ps_files *data, const zend_string *key) { char buf[MAXPATHLEN]; zend_stat_t sbuf = {0}; @@ -476,7 +474,7 @@ PS_READ_FUNC(files) zend_stat_t sbuf = {0}; PS_FILES_DATA; - ps_files_open(data, ZSTR_VAL(key)); + ps_files_open(data, key); if (data->fd < 0) { return FAILURE; } @@ -571,7 +569,7 @@ PS_UPDATE_TIMESTAMP_FUNC(files) int ret; PS_FILES_DATA; - if (!ps_files_path_create(buf, sizeof(buf), data, ZSTR_VAL(key))) { + if (!ps_files_path_create(buf, sizeof(buf), data, key)) { return FAILURE; } @@ -601,7 +599,7 @@ PS_DESTROY_FUNC(files) char buf[MAXPATHLEN]; PS_FILES_DATA; - if (!ps_files_path_create(buf, sizeof(buf), data, ZSTR_VAL(key))) { + if (!ps_files_path_create(buf, sizeof(buf), data, key)) { return FAILURE; } @@ -680,7 +678,7 @@ PS_CREATE_SID_FUNC(files) } /* Check collision */ /* FIXME: mod_data(data) should not be NULL (User handler could be NULL) */ - if (data && ps_files_key_exists(data, ZSTR_VAL(sid)) == SUCCESS) { + if (data && ps_files_key_exists(data, sid) == SUCCESS) { if (sid) { zend_string_release_ex(sid, 0); sid = NULL; @@ -708,5 +706,5 @@ PS_VALIDATE_SID_FUNC(files) { PS_FILES_DATA; - return ps_files_key_exists(data, ZSTR_VAL(key)); + return ps_files_key_exists(data, key); } From 8a5706f313957ca71ca82102797ee514d1252974 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 27 May 2022 16:15:29 +0100 Subject: [PATCH 3/5] Session: Use zend_string* consistently for key in mod_mm --- ext/session/mod_mm.c | 53 ++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/ext/session/mod_mm.c b/ext/session/mod_mm.c index 9610fb75bb4cc..b6ebcafafb3b6 100644 --- a/ext/session/mod_mm.c +++ b/ext/session/mod_mm.c @@ -45,7 +45,7 @@ typedef struct ps_sd { void *data; size_t datalen; /* amount of valid data */ size_t alloclen; /* amount of allocated memory for data */ - char key[1]; /* inline key */ + zend_string *key; } ps_sd; typedef struct { @@ -64,14 +64,15 @@ static ps_mm *ps_mm_instance = NULL; # define ps_mm_debug(a) #endif -static inline uint32_t ps_sd_hash(const char *data, size_t len) +static inline uint32_t ps_sd_hash(const zend_string *data) { uint32_t h; - const char *e = data + len; + const char *data_char = ZSTR_VAL(data); + const char *e = ZSTR_VAL(data) + ZSTR_LEN(data); - for (h = 2166136261U; data < e; ) { + for (h = 2166136261U; data_char < e; ) { h *= 16777619; - h ^= *data++; + h ^= *data_char++; } return h; @@ -106,22 +107,19 @@ static void hash_split(ps_mm *data) data->hash_max = nmax; } -static ps_sd *ps_sd_new(ps_mm *data, const char *key) +static ps_sd *ps_sd_new(ps_mm *data, zend_string *key) { uint32_t hv, slot; ps_sd *sd; - size_t keylen; - keylen = strlen(key); - - sd = mm_malloc(data->mm, sizeof(ps_sd) + keylen); + sd = mm_malloc(data->mm, sizeof(ps_sd) + ZSTR_LEN(key)); if (!sd) { php_error_docref(NULL, E_WARNING, "mm_malloc failed, avail %ld, err %s", mm_available(data->mm), mm_error()); return NULL; } - hv = ps_sd_hash(key, keylen); + hv = ps_sd_hash(key); slot = hv & data->hash_max; sd->ctime = 0; @@ -129,7 +127,7 @@ static ps_sd *ps_sd_new(ps_mm *data, const char *key) sd->data = NULL; sd->alloclen = sd->datalen = 0; - memcpy(sd->key, key, keylen + 1); + sd->key = zend_string_copy(key); sd->next = data->hash[slot]; data->hash[slot] = sd; @@ -142,7 +140,7 @@ static ps_sd *ps_sd_new(ps_mm *data, const char *key) } } - ps_mm_debug(("inserting %s(%p) into slot %d\n", key, sd, slot)); + ps_mm_debug(("inserting %s(%p) into slot %d\n", ZSTR_VAL(key), sd, slot)); return sd; } @@ -151,7 +149,7 @@ static void ps_sd_destroy(ps_mm *data, ps_sd *sd) { uint32_t slot; - slot = ps_sd_hash(sd->key, strlen(sd->key)) & data->hash_max; + slot = ps_sd_hash(sd->key) & data->hash_max; if (data->hash[slot] == sd) { data->hash[slot] = sd->next; @@ -168,20 +166,21 @@ static void ps_sd_destroy(ps_mm *data, ps_sd *sd) if (sd->data) { mm_free(data->mm, sd->data); } + zend_string_release(sd->key); mm_free(data->mm, sd); } -static ps_sd *ps_sd_lookup(ps_mm *data, const char *key, bool rw) +static ps_sd *ps_sd_lookup(ps_mm *data, const zend_string *key, bool rw) { uint32_t hv, slot; ps_sd *ret, *prev; - hv = ps_sd_hash(key, strlen(key)); + hv = ps_sd_hash(key); slot = hv & data->hash_max; for (prev = NULL, ret = data->hash[slot]; ret; prev = ret, ret = ret->next) { - if (ret->hv == hv && !strcmp(ret->key, key)) { + if (ret->hv == hv && zend_string_equals(ret->key, key)) { break; } } @@ -196,12 +195,12 @@ static ps_sd *ps_sd_lookup(ps_mm *data, const char *key, bool rw) data->hash[slot] = ret; } - ps_mm_debug(("lookup(%s): ret=%p,hv=%u,slot=%d\n", key, ret, hv, slot)); + ps_mm_debug(("lookup(%s): ret=%p,hv=%u,slot=%d\n", ZSTR_VAL(key), ret, hv, slot)); return ret; } -static zend_result ps_mm_key_exists(ps_mm *data, const char *key) +static zend_result ps_mm_key_exists(ps_mm *data, const zend_string *key) { ps_sd *sd; @@ -349,7 +348,7 @@ PS_READ_FUNC(mm) /* If there is an ID and strict mode, verify existence */ if (PS(use_strict_mode) - && ps_mm_key_exists(data, key->val) == FAILURE) { + && ps_mm_key_exists(data, key) == FAILURE) { /* key points to PS(id), but cannot change here. */ if (key) { efree(PS(id)); @@ -366,7 +365,7 @@ PS_READ_FUNC(mm) PS(session_status) = php_session_active; } - sd = ps_sd_lookup(data, PS(id)->val, 0); + sd = ps_sd_lookup(data, PS(id), 0); if (sd) { *val = zend_string_init(sd->data, sd->datalen, 0); ret = SUCCESS; @@ -384,10 +383,10 @@ PS_WRITE_FUNC(mm) mm_lock(data->mm, MM_LOCK_RW); - sd = ps_sd_lookup(data, key->val, 1); + sd = ps_sd_lookup(data, key, 1); if (!sd) { - sd = ps_sd_new(data, key->val); - ps_mm_debug(("new entry for %s\n", key->val)); + sd = ps_sd_new(data, key); + ps_mm_debug(("new entry for %s\n", ZSTR_VAL(key))); } if (sd) { @@ -423,7 +422,7 @@ PS_DESTROY_FUNC(mm) mm_lock(data->mm, MM_LOCK_RW); - sd = ps_sd_lookup(data, key->val, 0); + sd = ps_sd_lookup(data, key, 0); if (sd) { ps_sd_destroy(data, sd); } @@ -454,7 +453,7 @@ PS_GC_FUNC(mm) for (sd = *ohash; sd; sd = next) { next = sd->next; if (sd->ctime < limit) { - ps_mm_debug(("purging %s\n", sd->key)); + ps_mm_debug(("purging %s\n", ZSTR_VAL(sd->key))); ps_sd_destroy(data, sd); (*nrdels)++; } @@ -475,7 +474,7 @@ PS_CREATE_SID_FUNC(mm) do { sid = php_session_create_id((void **)&data); /* Check collision */ - if (ps_mm_key_exists(data, sid->val) == SUCCESS) { + if (ps_mm_key_exists(data, sid) == SUCCESS) { if (sid) { zend_string_release_ex(sid, 0); sid = NULL; From 9143cba14404eb5e33c521a292fb5504ce8b5373 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 27 May 2022 16:24:23 +0100 Subject: [PATCH 4/5] Session: Refactor last key to be a zend_string in mod_files --- ext/session/mod_files.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/ext/session/mod_files.c b/ext/session/mod_files.c index e6098e7feb050..7b48c827e8a10 100644 --- a/ext/session/mod_files.c +++ b/ext/session/mod_files.c @@ -89,7 +89,7 @@ #endif typedef struct { - char *lastkey; + zend_string *last_key; char *basedir; size_t basedir_len; size_t dirdepth; @@ -149,7 +149,7 @@ static void ps_files_close(ps_files *data) } } -static void ps_files_open(ps_files *data, const zend_string *key) +static void ps_files_open(ps_files *data, /* const */ zend_string *key) { char buf[MAXPATHLEN]; #if !defined(O_NOFOLLOW) || !defined(PHP_WIN32) @@ -157,10 +157,10 @@ static void ps_files_open(ps_files *data, const zend_string *key) #endif int ret; - if (data->fd < 0 || !data->lastkey || strcmp(ZSTR_VAL(key), data->lastkey)) { - if (data->lastkey) { - efree(data->lastkey); - data->lastkey = NULL; + if (data->fd < 0 || !data->last_key || !zend_string_equals(key, data->last_key)) { + if (data->last_key) { + zend_string_release_ex(data->last_key, /* persistent */ false); + data->last_key = NULL; } ps_files_close(data); @@ -175,7 +175,7 @@ static void ps_files_open(ps_files *data, const zend_string *key) return; } - data->lastkey = estrdup(ZSTR_VAL(key)); + data->last_key = zend_string_copy(key); /* O_NOFOLLOW to prevent us from following evil symlinks */ #ifdef O_NOFOLLOW @@ -230,7 +230,7 @@ static zend_result ps_files_write(ps_files *data, zend_string *key, zend_string /* PS(id) may be changed by calling session_regenerate_id(). Re-initialization should be tried here. ps_files_open() checks - data->lastkey and reopen when it is needed. */ + data->last_key and reopen when it is needed. */ ps_files_open(data, key); if (data->fd < 0) { return FAILURE; @@ -445,9 +445,9 @@ PS_CLOSE_FUNC(files) ps_files_close(data); - if (data->lastkey) { - efree(data->lastkey); - data->lastkey = NULL; + if (data->last_key) { + zend_string_release_ex(data->last_key, /* persistent */ false); + data->last_key = NULL; } efree(data->basedir); From ffe9726f366c756e70fc2eade60b0427ff16996f Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sun, 29 May 2022 11:12:56 +0100 Subject: [PATCH 5/5] Session: Refactor basedir to be a zend_string in mod_files --- ext/session/mod_files.c | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/ext/session/mod_files.c b/ext/session/mod_files.c index 7b48c827e8a10..a3207569616eb 100644 --- a/ext/session/mod_files.c +++ b/ext/session/mod_files.c @@ -90,8 +90,7 @@ typedef struct { zend_string *last_key; - char *basedir; - size_t basedir_len; + zend_string *basedir; size_t dirdepth; size_t st_size; int filemode; @@ -111,13 +110,13 @@ static char *ps_files_path_create(char *buf, size_t buflen, ps_files *data, cons size_t n; if (!data || ZSTR_LEN(key) <= data->dirdepth || - buflen < (strlen(data->basedir) + 2 * data->dirdepth + ZSTR_LEN(key) + 5 + sizeof(FILE_PREFIX))) { + buflen < (ZSTR_LEN(data->basedir) + 2 * data->dirdepth + ZSTR_LEN(key) + 5 + sizeof(FILE_PREFIX))) { return NULL; } p = ZSTR_VAL(key); - memcpy(buf, data->basedir, data->basedir_len); - n = data->basedir_len; + memcpy(buf, ZSTR_VAL(data->basedir), ZSTR_LEN(data->basedir)); + n = ZSTR_LEN(data->basedir); buf[n++] = PHP_DIR_SEPARATOR; for (i = 0; i < (int)data->dirdepth; i++) { buf[n++] = *p++; @@ -277,7 +276,7 @@ static zend_result ps_files_write(ps_files *data, zend_string *key, zend_string return SUCCESS; } -static int ps_files_cleanup_dir(const char *dirname, zend_long maxlifetime) +static int ps_files_cleanup_dir(const zend_string *dirname, zend_long maxlifetime) { DIR *dir; struct dirent *entry; @@ -285,27 +284,24 @@ static int ps_files_cleanup_dir(const char *dirname, zend_long maxlifetime) char buf[MAXPATHLEN]; time_t now; int nrdels = 0; - size_t dirname_len; - dir = opendir(dirname); + dir = opendir(ZSTR_VAL(dirname)); if (!dir) { - php_error_docref(NULL, E_NOTICE, "ps_files_cleanup_dir: opendir(%s) failed: %s (%d)", dirname, strerror(errno), errno); + php_error_docref(NULL, E_NOTICE, "ps_files_cleanup_dir: opendir(%s) failed: %s (%d)", ZSTR_VAL(dirname), strerror(errno), errno); return (0); } time(&now); - dirname_len = strlen(dirname); - - if (dirname_len >= MAXPATHLEN) { - php_error_docref(NULL, E_NOTICE, "ps_files_cleanup_dir: dirname(%s) is too long", dirname); + if (ZSTR_LEN(dirname) >= MAXPATHLEN) { + php_error_docref(NULL, E_NOTICE, "ps_files_cleanup_dir: dirname(%s) is too long", ZSTR_VAL(dirname)); closedir(dir); return (0); } /* Prepare buffer (dirname never changes) */ - memcpy(buf, dirname, dirname_len); - buf[dirname_len] = PHP_DIR_SEPARATOR; + memcpy(buf, ZSTR_VAL(dirname), ZSTR_LEN(dirname)); + buf[ZSTR_LEN(dirname)] = PHP_DIR_SEPARATOR; while ((entry = readdir(dir))) { /* does the file start with our prefix? */ @@ -313,12 +309,12 @@ static int ps_files_cleanup_dir(const char *dirname, zend_long maxlifetime) size_t entry_len = strlen(entry->d_name); /* does it fit into our buffer? */ - if (entry_len + dirname_len + 2 < MAXPATHLEN) { + if (entry_len + ZSTR_LEN(dirname) + 2 < MAXPATHLEN) { /* create the full path.. */ - memcpy(buf + dirname_len + 1, entry->d_name, entry_len); + memcpy(buf + ZSTR_LEN(dirname) + 1, entry->d_name, entry_len); /* NUL terminate it and */ - buf[dirname_len + entry_len + 1] = '\0'; + buf[ZSTR_LEN(dirname) + entry_len + 1] = '\0'; /* check whether its last access was more than maxlifetime ago */ if (VCWD_STAT(buf, &sbuf) == 0 && @@ -417,8 +413,7 @@ PS_OPEN_FUNC(files) data->fd = -1; data->dirdepth = dirdepth; data->filemode = filemode; - data->basedir_len = strlen(save_path); - data->basedir = estrndup(save_path, data->basedir_len); + data->basedir = zend_string_init(save_path, strlen(save_path), /* persistent */ false); if (PS_GET_MOD_DATA()) { ps_close_files(mod_data); @@ -450,7 +445,7 @@ PS_CLOSE_FUNC(files) data->last_key = NULL; } - efree(data->basedir); + zend_string_release_ex(data->basedir, /* persistent */ false); efree(data); PS_SET_MOD_DATA(NULL);