Skip to content

Commit

Permalink
ext/sqlite3: Use new F ZPP modifier
Browse files Browse the repository at this point in the history
  • Loading branch information
Girgias committed Oct 9, 2023
1 parent c8a2bb2 commit 928faec
Show file tree
Hide file tree
Showing 5 changed files with 239 additions and 41 deletions.
77 changes: 36 additions & 41 deletions ext/sqlite3/sqlite3.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ static int php_sqlite3_compare_stmt_zval_free(php_sqlite3_free_list **free_list,
RETURN_THROWS(); \
}

#define SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE(db_obj, member, class_name, trampoline_fcc) \
if (!(db_obj) || !(member)) { \
zend_release_fcall_info_cache((trampoline_fcc)); \
zend_throw_error(NULL, "The " #class_name " object has not been correctly initialised or is already closed"); \
RETURN_THROWS(); \
}

#define SQLITE3_CHECK_INITIALIZED_STMT(member, class_name) \
if (!(member)) { \
zend_throw_error(NULL, "The " #class_name " object has not been correctly initialised or is already closed"); \
Expand Down Expand Up @@ -942,27 +949,23 @@ PHP_METHOD(SQLite3, createFunction)
zend_long flags = 0;
db_obj = Z_SQLITE3_DB_P(object);

if (zend_parse_parameters(ZEND_NUM_ARGS(), "sf|ll", &sql_func, &sql_func_len, &fci, &fcc, &sql_func_num_args, &flags) == FAILURE) {
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sF|ll", &sql_func, &sql_func_len, &fci, &fcc, &sql_func_num_args, &flags) == FAILURE) {
zend_release_fcall_info_cache(&fcc);
RETURN_THROWS();
}

SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3)
SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE(db_obj, db_obj->initialised, SQLite3, &fcc);

if (!sql_func_len) {
/* TODO Add warning/ValueError that name cannot be empty? */
zend_release_fcall_info_cache(&fcc);
RETURN_FALSE;
}

func = (php_sqlite3_func *)ecalloc(1, sizeof(*func));

if (sqlite3_create_function(db_obj->db, sql_func, sql_func_num_args, flags | SQLITE_UTF8, func, php_sqlite3_callback_func, NULL, NULL) == SQLITE_OK) {
func->func_name = estrdup(sql_func);

if (!ZEND_FCC_INITIALIZED(fcc)) {
zend_is_callable_ex(&fci.function_name, NULL, IS_CALLABLE_SUPPRESS_DEPRECATIONS, NULL, &fcc, NULL);
/* Call trampoline has been cleared by zpp. Refetch it, because we want to deal
* with it outselves. It is important that it is not refetched on every call,
* because calls may occur from different scopes. */
}
zend_fcc_dup(&func->func, &fcc);

func->argc = sql_func_num_args;
Expand All @@ -972,6 +975,7 @@ PHP_METHOD(SQLite3, createFunction)
RETURN_TRUE;
}
efree(func);
zend_release_fcall_info_cache(&fcc);

RETURN_FALSE;
}
Expand All @@ -990,13 +994,24 @@ PHP_METHOD(SQLite3, createAggregate)
zend_long sql_func_num_args = -1;
db_obj = Z_SQLITE3_DB_P(object);

if (zend_parse_parameters(ZEND_NUM_ARGS(), "sff|l", &sql_func, &sql_func_len, &step_fci, &step_fcc, &fini_fci, &fini_fcc, &sql_func_num_args) == FAILURE) {
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sFF|l", &sql_func, &sql_func_len, &step_fci, &step_fcc, &fini_fci, &fini_fcc, &sql_func_num_args) == FAILURE) {
zend_release_fcall_info_cache(&step_fcc);
zend_release_fcall_info_cache(&fini_fcc);
RETURN_THROWS();
}

SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3)
/* Cannot use SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE() as we have 2 FCCs */
if (!db_obj || !db_obj->initialised) {
zend_release_fcall_info_cache(&step_fcc);
zend_release_fcall_info_cache(&fini_fcc);
zend_throw_error(NULL, "The SQLite3 object has not been correctly initialised or is already closed");
RETURN_THROWS();
}

if (!sql_func_len) {
/* TODO Add warning/ValueError that name cannot be empty? */
zend_release_fcall_info_cache(&step_fcc);
zend_release_fcall_info_cache(&fini_fcc);
RETURN_FALSE;
}

Expand All @@ -1005,19 +1020,7 @@ PHP_METHOD(SQLite3, createAggregate)
if (sqlite3_create_function(db_obj->db, sql_func, sql_func_num_args, SQLITE_UTF8, func, NULL, php_sqlite3_callback_step, php_sqlite3_callback_final) == SQLITE_OK) {
func->func_name = estrdup(sql_func);

if (!ZEND_FCC_INITIALIZED(step_fcc)) {
/* Call trampoline has been cleared by zpp. Refetch it, because we want to deal
* with it outselves. It is important that it is not refetched on every call,
* because calls may occur from different scopes. */
zend_is_callable_ex(&step_fci.function_name, NULL, IS_CALLABLE_SUPPRESS_DEPRECATIONS, NULL, &step_fcc, NULL);
}
zend_fcc_dup(&func->step, &step_fcc);
if (!ZEND_FCC_INITIALIZED(fini_fcc)) {
/* Call trampoline has been cleared by zpp. Refetch it, because we want to deal
* with it outselves. It is important that it is not refetched on every call,
* because calls may occur from different scopes. */
zend_is_callable_ex(&fini_fci.function_name, NULL, IS_CALLABLE_SUPPRESS_DEPRECATIONS, NULL, &fini_fcc, NULL);
}
zend_fcc_dup(&func->fini, &fini_fcc);

func->argc = sql_func_num_args;
Expand All @@ -1027,6 +1030,8 @@ PHP_METHOD(SQLite3, createAggregate)
RETURN_TRUE;
}
efree(func);
zend_release_fcall_info_cache(&step_fcc);
zend_release_fcall_info_cache(&fini_fcc);

RETURN_FALSE;
}
Expand All @@ -1044,26 +1049,22 @@ PHP_METHOD(SQLite3, createCollation)
zend_fcall_info_cache fcc;
db_obj = Z_SQLITE3_DB_P(object);

if (zend_parse_parameters(ZEND_NUM_ARGS(), "sf", &collation_name, &collation_name_len, &fci, &fcc) == FAILURE) {
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sF", &collation_name, &collation_name_len, &fci, &fcc) == FAILURE) {
RETURN_THROWS();
}

SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3)
SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE(db_obj, db_obj->initialised, SQLite3, &fcc);

if (!collation_name_len) {
/* TODO Add warning/ValueError that name cannot be empty? */
zend_release_fcall_info_cache(&fcc);
RETURN_FALSE;
}

collation = (php_sqlite3_collation *)ecalloc(1, sizeof(*collation));
if (sqlite3_create_collation(db_obj->db, collation_name, SQLITE_UTF8, collation, php_sqlite3_callback_compare) == SQLITE_OK) {
collation->collation_name = estrdup(collation_name);

if (!ZEND_FCC_INITIALIZED(fcc)) {
/* Call trampoline has been cleared by zpp. Refetch it, because we want to deal
* with it outselves. It is important that it is not refetched on every call,
* because calls may occur from different scopes. */
zend_is_callable_ex(&fci.function_name, NULL, IS_CALLABLE_SUPPRESS_DEPRECATIONS, NULL, &fcc, NULL);
}
zend_fcc_dup(&collation->cmp_func, &fcc);

collation->next = db_obj->collations;
Expand All @@ -1072,6 +1073,7 @@ PHP_METHOD(SQLite3, createCollation)
RETURN_TRUE;
}
efree(collation);
zend_release_fcall_info_cache(&fcc);

RETURN_FALSE;
}
Expand Down Expand Up @@ -1317,10 +1319,10 @@ PHP_METHOD(SQLite3, setAuthorizer)
zend_fcall_info_cache fcc;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_FUNC_OR_NULL(fci, fcc)
Z_PARAM_FUNC_NO_TRAMPOLINE_FREE_OR_NULL(fci, fcc)
ZEND_PARSE_PARAMETERS_END();

SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3)
SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE(db_obj, db_obj->initialised, SQLite3, &fcc);

/* Clear previously set callback */
if (ZEND_FCC_INITIALIZED(db_obj->authorizer_fcc)) {
Expand All @@ -1329,14 +1331,7 @@ PHP_METHOD(SQLite3, setAuthorizer)

/* Only enable userland authorizer if argument is not NULL */
if (ZEND_FCI_INITIALIZED(fci)) {
if (!ZEND_FCC_INITIALIZED(fcc)) {
zend_is_callable_ex(&fci.function_name, NULL, IS_CALLABLE_SUPPRESS_DEPRECATIONS, NULL, &fcc, NULL);
/* Call trampoline has been cleared by zpp. Refetch it, because we want to deal
* with it outselves. It is important that it is not refetched on every call,
* because calls may occur from different scopes. */
}
db_obj->authorizer_fcc = fcc;
zend_fcc_addref(&db_obj->authorizer_fcc);
zend_fcc_dup(&db_obj->authorizer_fcc, &fcc);
}

RETURN_TRUE;
Expand Down
73 changes: 73 additions & 0 deletions ext/sqlite3/tests/sqlite3_trampoline_create_aggregate_no_leak.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
--TEST--
SQLite3::createAggregate() use F ZPP for trampoline callback and does not leak
--EXTENSIONS--
sqlite3
--FILE--
<?php

require_once(__DIR__ . '/new_db.inc');

class TrampolineTest {
public function __call(string $name, array $arguments) {
echo 'Trampoline for ', $name, PHP_EOL;
$context = $arguments[0];
if ($name === 'finalize') {
return implode(',', $context['values']);
}
if (empty($context)) {
$context = ['total' => 0, 'values' => []];
}
$context['total'] += (int) $arguments[2];
$context['values'][] = $context['total'];
return $context;
}
}
$o = new TrampolineTest();
$step = [$o, 'step'];
$finalize = [$o, 'finalize'];

var_dump($db->createAggregate('', $step, $finalize, 1));

try {
var_dump($db->createAggregate('S', $step, $finalize, new stdClass()));
} catch (\Throwable $e) {
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
}
try {
var_dump($db->createAggregate('S', $step, 'no_func', 1));
} catch (\Throwable $e) {
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
}
try {
var_dump($db->createAggregate('S', 'no_func', $finalize, 1));
} catch (\Throwable $e) {
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
}

echo "Invalid SQLite3 object:\n";
$rc = new ReflectionClass(SQLite3::class);
$obj = $rc->newInstanceWithoutConstructor();

try {
var_dump($obj->createAggregate('S', $step, $finalize, 1));
} catch (\Throwable $e) {
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
}

var_dump($db->createAggregate('S', $step, $finalize, 1));

echo "Closing database\n";
var_dump($db->close());
echo "Done\n";
?>
--EXPECT--
bool(false)
TypeError: SQLite3::createAggregate(): Argument #4 ($argCount) must be of type int, stdClass given
TypeError: SQLite3::createAggregate(): Argument #3 ($finalCallback) must be a valid callback, function "no_func" not found or invalid function name
TypeError: SQLite3::createAggregate(): Argument #2 ($stepCallback) must be a valid callback, function "no_func" not found or invalid function name
Invalid SQLite3 object:
Error: The SQLite3 object has not been correctly initialised or is already closed
bool(true)
Closing database
bool(true)
Done
44 changes: 44 additions & 0 deletions ext/sqlite3/tests/sqlite3_trampoline_createcollation_no_leak.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
--TEST--
SQLite3::createFunction use F ZPP for trampoline callback and does not leak
--EXTENSIONS--
sqlite3
--FILE--
<?php

require_once(__DIR__ . '/new_db.inc');

class TrampolineTest {
public function __call(string $name, array $arguments) {
echo 'Trampoline for ', $name, PHP_EOL;
return strnatcmp(...$arguments);
}
}
$o = new TrampolineTest();
$callback = [$o, 'NAT'];

var_dump($db->createCollation('', $callback));
try {
var_dump($db->createCollation('NAT', $callback, new stdClass()));
} catch (\Throwable $e) {
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
}

echo "Invalid SQLite3 object:\n";
$rc = new ReflectionClass(SQLite3::class);
$obj = $rc->newInstanceWithoutConstructor();

try {
var_dump($obj->createCollation('NAT', $callback));
} catch (\Throwable $e) {
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
}

var_dump($db->createCollation('NAT', $callback));

?>
--EXPECT--
bool(false)
ArgumentCountError: SQLite3::createCollation() expects exactly 2 arguments, 3 given
Invalid SQLite3 object:
Error: The SQLite3 object has not been correctly initialised or is already closed
bool(true)
45 changes: 45 additions & 0 deletions ext/sqlite3/tests/sqlite3_trampoline_createfunction_no_leak.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
--TEST--
SQLite3::createFunction use F ZPP for trampoline callback and does not leak
--EXTENSIONS--
sqlite3
--FILE--
<?php

require_once(__DIR__ . '/new_db.inc');

class TrampolineTest {
public function __call(string $name, array $arguments) {
echo 'Trampoline for ', $name, PHP_EOL;
return strtoupper($arguments[0]);
}
}
$o = new TrampolineTest();
$callback = [$o, 'strtoupper'];

var_dump($db->createfunction('', $callback));

try {
var_dump($db->createfunction('strtoupper', $callback, new stdClass()));
} catch (\Throwable $e) {
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
}

echo "Invalid SQLite3 object:\n";
$rc = new ReflectionClass(SQLite3::class);
$obj = $rc->newInstanceWithoutConstructor();

try {
var_dump($obj->createfunction('strtoupper', $callback));
} catch (\Throwable $e) {
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
}

var_dump($db->createfunction('strtoupper', $callback));

?>
--EXPECT--
bool(false)
TypeError: SQLite3::createFunction(): Argument #3 ($argCount) must be of type int, stdClass given
Invalid SQLite3 object:
Error: The SQLite3 object has not been correctly initialised or is already closed
bool(true)
41 changes: 41 additions & 0 deletions ext/sqlite3/tests/sqlite3_trampoline_setauthorizer_no_leak.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
--TEST--
SQLite3::setAuthorizer use F ZPP for trampoline callback and does not leak
--EXTENSIONS--
sqlite3
--FILE--
<?php

class TrampolineTest {
public function __call(string $name, array $arguments) {
echo 'Trampoline for ', $name, PHP_EOL;
if ($arguments[0] == SQLite3::SELECT) {
return SQLite3::OK;
}

return SQLite3::DENY;
}
}
$o = new TrampolineTest();
$callback = [$o, 'authorizer'];

$db = new SQLite3(':memory:');
$db->enableExceptions(true);

echo "Invalid SQLite3 object:\n";
$rc = new ReflectionClass(SQLite3::class);
$obj = $rc->newInstanceWithoutConstructor();

try {
var_dump($obj->setAuthorizer($callback));
} catch (\Throwable $e) {
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
}

$db->setAuthorizer($callback);

?>
DONE
--EXPECT--
Invalid SQLite3 object:
Error: The SQLite3 object has not been correctly initialised or is already closed
DONE

0 comments on commit 928faec

Please sign in to comment.