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 3942b7181c587be786701fdb76aed3882796980a..b3e559445031a9336ef3ce033933be39daa95827 100644 (file)
--- 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 fb1e92238b1f81c4ce499528dd71d3b002659c49..291130e42384776b394793584d851ad47d9099dc 100644 (file)
--- 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);
+       }
 }
 /* }}} */