From: Wez Furlong Date: Sun, 20 Apr 2003 15:13:04 +0000 (+0000) Subject: Eliminate leaks from registering plain or aggregate functions. X-Git-Tag: SPL_ALPHA~129 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8ccae81db2bc9f0588fc476dd9c115487f4b32a7;p=php Eliminate leaks from registering plain or aggregate functions. Also, reduce (probably eliminate) the risk of a segfault when registering a callback on a persistent connection and later triggering the callback from a different script. --- diff --git a/ext/sqlite/TODO b/ext/sqlite/TODO index 3942b7181c..b3e5594450 100644 --- a/ext/sqlite/TODO +++ b/ext/sqlite/TODO @@ -1,9 +1,5 @@ - Transparent binary encoding of return values from PHP callback functions. -- Add per-db hashtable to store the aggregate+plain function callback - structures. The hashtable and structs also need to be allocated using - pemalloc() based on the persistent nature of the db connection. - - Add user-space callback for the authorizer function (this is potentially very slow, so it needs to be implemented carefully). @@ -11,6 +7,13 @@ - Test-suite + o Test how robust we are when a user-space function is registered as + a callback for a persistent connection in script A, then script B is + called that doesn't register the callback but does make use of the + function in an SQL query. + --> Our test suite doesn't allow us to test persistent connections + at this time :/ + - If building a ZTS build, -DTHREADSAFE while compiling libsqlite - If building a non-debug build, -DNDEBUG will disable the expensive diff --git a/ext/sqlite/sqlite.c b/ext/sqlite/sqlite.c index fb1e92238b..291130e423 100644 --- a/ext/sqlite/sqlite.c +++ b/ext/sqlite/sqlite.c @@ -40,7 +40,6 @@ extern int sqlite_encode_binary(const unsigned char *in, int n, unsigned char *out); extern int sqlite_decode_binary(const unsigned char *in, unsigned char *out); - static unsigned char arg3_force_ref[] = {3, BYREF_NONE, BYREF_NONE, BYREF_FORCE }; static int le_sqlite_db, le_sqlite_result, le_sqlite_pdb; @@ -63,11 +62,15 @@ struct php_sqlite_db { int last_err_code; int is_persistent; int rsrc_id; + + HashTable callbacks; }; struct php_sqlite_agg_functions { + struct php_sqlite_db *db; + int is_valid; zval *step; - zval *final; + zval *fini; }; @@ -120,12 +123,47 @@ zend_module_entry sqlite_module_entry = { ZEND_GET_MODULE(sqlite) #endif +static int php_sqlite_callback_invalidator(struct php_sqlite_agg_functions *funcs TSRMLS_DC) +{ + if (!funcs->is_valid) { + return 0; + } + + if (funcs->step) { + zval_ptr_dtor(&funcs->step); + funcs->step = NULL; + } + + if (funcs->fini) { + zval_ptr_dtor(&funcs->fini); + funcs->fini = NULL; + } + + funcs->is_valid = 0; + + return 0; +} + + +static void php_sqlite_callback_dtor(void *pDest) +{ + struct php_sqlite_agg_functions *funcs = (struct php_sqlite_agg_functions*)pDest; + + if (funcs->is_valid) { + TSRMLS_FETCH(); + + php_sqlite_callback_invalidator(funcs TSRMLS_CC); + } +} + static ZEND_RSRC_DTOR_FUNC(php_sqlite_db_dtor) { if (rsrc->ptr) { struct php_sqlite_db *db = (struct php_sqlite_db*)rsrc->ptr; sqlite_close(db->db); + zend_hash_destroy(&db->callbacks); + pefree(db, db->is_persistent); rsrc->ptr = NULL; @@ -182,6 +220,9 @@ static int php_sqlite_forget_persistent_id_numbers(zend_rsrc_list_entry *rsrc TS /* don't leave pending commits hanging around */ sqlite_exec(db->db, "ROLLBACK", NULL, NULL, NULL); + /* prevent bad mojo if someone tries to use a previously registered function in the next request */ + zend_hash_apply(&db->callbacks, (apply_func_t)php_sqlite_callback_invalidator TSRMLS_CC); + return 0; } @@ -274,15 +315,20 @@ static void php_sqlite_generic_function_callback(sqlite_func *func, int argc, co } /* }}} */ +/* {{{ callback for sqlite_create_function */ static void php_sqlite_function_callback(sqlite_func *func, int argc, const char **argv) { zval *retval = NULL; zval ***zargs; int i, res; - char *errbuf=NULL; struct php_sqlite_agg_functions *funcs = sqlite_user_data(func); TSRMLS_FETCH(); + if (!funcs->is_valid) { + sqlite_set_result_error(func, "this function has not been correctly defined for this request", -1); + return; + } + if (argc > 0) { zargs = (zval ***)emalloc(argc * sizeof(zval **)); @@ -337,7 +383,9 @@ static void php_sqlite_function_callback(sqlite_func *func, int argc, const char efree(zargs); } } +/* }}} */ +/* {{{ callback for sqlite_create_aggregate: step function */ static void php_sqlite_agg_step_function_callback(sqlite_func *func, int argc, const char **argv) { zval *retval = NULL; @@ -347,6 +395,11 @@ static void php_sqlite_agg_step_function_callback(sqlite_func *func, int argc, c struct php_sqlite_agg_functions *funcs = sqlite_user_data(func); TSRMLS_FETCH(); + if (!funcs->is_valid) { + sqlite_set_result_error(func, "this function has not been correctly defined for this request", -1); + return; + } + /* sanity check the args */ if (argc < 1) { return; @@ -357,8 +410,11 @@ static void php_sqlite_agg_step_function_callback(sqlite_func *func, int argc, c /* first arg is always the context zval */ context_p = (zval **)sqlite_aggregate_context(func, sizeof(*context_p)); + if (*context_p == NULL) { MAKE_STD_ZVAL(*context_p); + (*context_p)->is_ref = 1; + Z_TYPE_PP(context_p) = IS_NULL; } zargs[0] = context_p; @@ -394,23 +450,27 @@ static void php_sqlite_agg_step_function_callback(sqlite_func *func, int argc, c efree(zargs); } } +/* }}} */ +/* {{{ callback for sqlite_create_aggregate: finalize function */ static void php_sqlite_agg_fini_function_callback(sqlite_func *func) { zval *retval = NULL; - zval ***zargs; - zval funcname; - int i, res; - char *callable = NULL, *errbuf=NULL; + int res; struct php_sqlite_agg_functions *funcs = sqlite_user_data(func); zval **context_p; TSRMLS_FETCH(); + if (!funcs->is_valid) { + sqlite_set_result_error(func, "this function has not been correctly defined for this request", -1); + return; + } + context_p = (zval **)sqlite_aggregate_context(func, sizeof(*context_p)); res = call_user_function_ex(EG(function_table), NULL, - funcs->final, + funcs->fini, &retval, 1, &context_p, @@ -447,8 +507,7 @@ static void php_sqlite_agg_fini_function_callback(sqlite_func *func) zval_ptr_dtor(context_p); } - - +/* }}} */ /* {{{ Authorization Callback */ static int php_sqlite_authorizer(void *autharg, int access_type, const char *arg3, const char *arg4) @@ -551,6 +610,8 @@ static struct php_sqlite_db *php_sqlite_open(char *filename, int mode, char *per db->is_persistent = persistent_id ? 1 : 0; db->last_err_code = SQLITE_OK; db->db = sdb; + + zend_hash_init(&db->callbacks, 0, NULL, php_sqlite_callback_dtor, db->is_persistent); /* register the PHP functions */ sqlite_create_function(sdb, "php", -1, php_sqlite_generic_function_callback, 0); @@ -589,7 +650,6 @@ PHP_FUNCTION(sqlite_popen) char *filename, *fullpath, *hashkey; long filename_len, hashkeylen; zval *errmsg = NULL; - char *errtext = NULL; struct php_sqlite_db *db = NULL; list_entry *le; @@ -712,10 +772,9 @@ PHP_FUNCTION(sqlite_unbuffered_query) char *sql; long sql_len; struct php_sqlite_result res, *rres; - int ret, i, base; + int ret; char *errtext = NULL; const char *tail; - const char **rowdata, **colnames; if (FAILURE == zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sr", &sql, &sql_len, &zdb)) { return; @@ -1192,6 +1251,68 @@ PHP_FUNCTION(sqlite_error_string) } /* }}} */ +/* manages duplicate registrations of a particular function, and + * also handles the case where the db is using a persistent connection */ +enum callback_prep_t { DO_REG, SKIP_REG, ERR }; + +static enum callback_prep_t prep_callback_struct(struct php_sqlite_db *db, int is_agg, + char *funcname, + zval *step, zval *fini, struct php_sqlite_agg_functions **funcs) +{ + struct php_sqlite_agg_functions *alloc_funcs, func_tmp; + char *hashkey; + int hashkeylen; + enum callback_prep_t ret; + + hashkeylen = spprintf(&hashkey, 0, "%s-%s", is_agg ? "agg" : "reg", funcname); + + /* is it already registered ? */ + if (SUCCESS == zend_hash_find(&db->callbacks, hashkey, hashkeylen+1, (void*)&alloc_funcs)) { + /* override the previous definition */ + + if (alloc_funcs->is_valid) { + /* release these */ + + if (alloc_funcs->step) { + zval_ptr_dtor(&alloc_funcs->step); + alloc_funcs->step = NULL; + } + + if (alloc_funcs->fini) { + zval_ptr_dtor(&alloc_funcs->fini); + alloc_funcs->fini = NULL; + } + } + + ret = SKIP_REG; + } else { + /* add a new one */ + func_tmp.db = db; + + ret = SUCCESS == zend_hash_update(&db->callbacks, hashkey, hashkeylen+1, + (void*)&func_tmp, sizeof(func_tmp), (void**)&alloc_funcs) ? DO_REG : ERR; + } + + efree(hashkey); + + MAKE_STD_ZVAL(alloc_funcs->step); + *(alloc_funcs->step) = *step; + zval_copy_ctor(alloc_funcs->step); + + if (is_agg) { + MAKE_STD_ZVAL(alloc_funcs->fini); + *(alloc_funcs->fini) = *fini; + zval_copy_ctor(alloc_funcs->fini); + } else { + alloc_funcs->fini = NULL; + } + alloc_funcs->is_valid = 1; + *funcs = alloc_funcs; + + return ret; +} + + /* {{{ proto bool sqlite_create_aggregate(resource db, string funcname, mixed step_func, mixed finalize_func[, long num_args]) Registers an aggregated function for queries*/ PHP_FUNCTION(sqlite_create_aggregate) @@ -1225,17 +1346,13 @@ PHP_FUNCTION(sqlite_create_aggregate) DB_FROM_ZVAL(db, &zdb); - /* TODO: this needs to be cleaned up */ - funcs = (struct php_sqlite_agg_functions *)emalloc(sizeof(*funcs)); + if (prep_callback_struct(db, 1, funcname, zstep, zfinal, &funcs) == DO_REG) { + sqlite_create_aggregate(db->db, funcname, num_args, + php_sqlite_agg_step_function_callback, + php_sqlite_agg_fini_function_callback, funcs); + } - MAKE_STD_ZVAL(funcs->step); - MAKE_STD_ZVAL(funcs->final); - *(funcs->step) = *zstep; - *(funcs->final) = *zfinal; - zval_copy_ctor(funcs->step); - zval_copy_ctor(funcs->final); - - sqlite_create_aggregate(db->db, funcname, num_args, php_sqlite_agg_step_function_callback, php_sqlite_agg_fini_function_callback, funcs); + } /* }}} */ @@ -1256,7 +1373,7 @@ PHP_FUNCTION(sqlite_create_function) } if (!zend_is_callable(zcall, 0, &callable)) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "step function `%s' is not callable", callable); + php_error_docref(NULL TSRMLS_CC, E_WARNING, "function `%s' is not callable", callable); efree(callable); return; } @@ -1264,13 +1381,8 @@ PHP_FUNCTION(sqlite_create_function) DB_FROM_ZVAL(db, &zdb); - /* TODO: this needs to be cleaned up */ - funcs = (struct php_sqlite_agg_functions *)emalloc(sizeof(*funcs)); - - MAKE_STD_ZVAL(funcs->step); - *(funcs->step) = *zcall; - zval_copy_ctor(funcs->step); - - sqlite_create_function(db->db, funcname, num_args, php_sqlite_function_callback, funcs); + if (prep_callback_struct(db, 0, funcname, zcall, NULL, &funcs) == DO_REG) { + sqlite_create_function(db->db, funcname, num_args, php_sqlite_function_callback, funcs); + } } /* }}} */