Skip to content

Commit c59d1c4

Browse files
GirgiasGirgias
committed
ext/sqlite3: Use new F ZPP modifier
1 parent 283eee9 commit c59d1c4

5 files changed

+229
-34
lines changed

‎ext/sqlite3/sqlite3.c

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,13 @@ static int php_sqlite3_compare_stmt_zval_free(php_sqlite3_free_list **free_list,
4444
RETURN_THROWS(); \
4545
}
4646

47+
#define SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE(db_obj, member, class_name, trampoline_fcc) \
48+
if (!(db_obj) || !(member)) { \
49+
zend_release_fcall_info_cache((trampoline_fcc)); \
50+
zend_throw_error(NULL, "The " #class_name " object has not been correctly initialised or is already closed"); \
51+
RETURN_THROWS(); \
52+
}
53+
4754
#define SQLITE3_CHECK_INITIALIZED_STMT(member, class_name) \
4855
if (!(member)) { \
4956
zend_throw_error(NULL, "The " #class_name " object has not been correctly initialised or is already closed"); \
@@ -942,13 +949,16 @@ PHP_METHOD(SQLite3, createFunction)
942949
zend_long flags = 0;
943950
db_obj = Z_SQLITE3_DB_P(object);
944951

945-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sf|ll", &sql_func, &sql_func_len, &fci, &fcc, &sql_func_num_args, &flags) == FAILURE) {
952+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sF|ll", &sql_func, &sql_func_len, &fci, &fcc, &sql_func_num_args, &flags) == FAILURE) {
953+
zend_release_fcall_info_cache(&fcc);
946954
RETURN_THROWS();
947955
}
948956

949-
SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3)
957+
SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE(db_obj, db_obj->initialised, SQLite3, &fcc);
950958

951959
if (!sql_func_len) {
960+
/* TODO Add warning/ValueError that name cannot be empty? */
961+
zend_release_fcall_info_cache(&fcc);
952962
RETURN_FALSE;
953963
}
954964

@@ -990,13 +1000,24 @@ PHP_METHOD(SQLite3, createAggregate)
9901000
zend_long sql_func_num_args = -1;
9911001
db_obj = Z_SQLITE3_DB_P(object);
9921002

993-
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) {
1003+
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) {
1004+
zend_release_fcall_info_cache(&step_fcc);
1005+
zend_release_fcall_info_cache(&fini_fcc);
9941006
RETURN_THROWS();
9951007
}
9961008

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

9991017
if (!sql_func_len) {
1018+
/* TODO Add warning/ValueError that name cannot be empty? */
1019+
zend_release_fcall_info_cache(&step_fcc);
1020+
zend_release_fcall_info_cache(&fini_fcc);
10001021
RETURN_FALSE;
10011022
}
10021023

@@ -1005,19 +1026,7 @@ PHP_METHOD(SQLite3, createAggregate)
10051026
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) {
10061027
func->func_name = estrdup(sql_func);
10071028

1008-
if (!ZEND_FCC_INITIALIZED(step_fcc)) {
1009-
/* Call trampoline has been cleared by zpp. Refetch it, because we want to deal
1010-
* with it outselves. It is important that it is not refetched on every call,
1011-
* because calls may occur from different scopes. */
1012-
zend_is_callable_ex(&step_fci.function_name, NULL, IS_CALLABLE_SUPPRESS_DEPRECATIONS, NULL, &step_fcc, NULL);
1013-
}
10141029
zend_fcc_dup(&func->step, &step_fcc);
1015-
if (!ZEND_FCC_INITIALIZED(fini_fcc)) {
1016-
/* Call trampoline has been cleared by zpp. Refetch it, because we want to deal
1017-
* with it outselves. It is important that it is not refetched on every call,
1018-
* because calls may occur from different scopes. */
1019-
zend_is_callable_ex(&fini_fci.function_name, NULL, IS_CALLABLE_SUPPRESS_DEPRECATIONS, NULL, &fini_fcc, NULL);
1020-
}
10211030
zend_fcc_dup(&func->fini, &fini_fcc);
10221031

10231032
func->argc = sql_func_num_args;
@@ -1044,26 +1053,22 @@ PHP_METHOD(SQLite3, createCollation)
10441053
zend_fcall_info_cache fcc;
10451054
db_obj = Z_SQLITE3_DB_P(object);
10461055

1047-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sf", &collation_name, &collation_name_len, &fci, &fcc) == FAILURE) {
1056+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sF", &collation_name, &collation_name_len, &fci, &fcc) == FAILURE) {
10481057
RETURN_THROWS();
10491058
}
10501059

1051-
SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3)
1060+
SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE(db_obj, db_obj->initialised, SQLite3, &fcc);
10521061

10531062
if (!collation_name_len) {
1063+
/* TODO Add warning/ValueError that name cannot be empty? */
1064+
zend_release_fcall_info_cache(&fcc);
10541065
RETURN_FALSE;
10551066
}
10561067

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

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

10691074
collation->next = db_obj->collations;
@@ -1317,10 +1322,10 @@ PHP_METHOD(SQLite3, setAuthorizer)
13171322
zend_fcall_info_cache fcc;
13181323

13191324
ZEND_PARSE_PARAMETERS_START(1, 1)
1320-
Z_PARAM_FUNC_OR_NULL(fci, fcc)
1325+
Z_PARAM_FUNC_NO_TRAMPOLINE_FREE_OR_NULL(fci, fcc)
13211326
ZEND_PARSE_PARAMETERS_END();
13221327

1323-
SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3)
1328+
SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE(db_obj, db_obj->initialised, SQLite3, &fcc);
13241329

13251330
/* Clear previously set callback */
13261331
if (ZEND_FCC_INITIALIZED(db_obj->authorizer_fcc)) {
@@ -1329,14 +1334,7 @@ PHP_METHOD(SQLite3, setAuthorizer)
13291334

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

13421340
RETURN_TRUE;

‎ext/sqlite3/tests/sqlite3_trampoline_create_aggregate_no_leak.phpt

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
--TEST--
2+
SQLite3::createAggregate() use F ZPP for trampoline callback and does not leak
3+
--EXTENSIONS--
4+
sqlite3
5+
--FILE--
6+
<?php
7+
8+
require_once(__DIR__ . '/new_db.inc');
9+
10+
class TrampolineTest {
11+
public function __call(string $name, array $arguments) {
12+
echo 'Trampoline for ', $name, PHP_EOL;
13+
$context = $arguments[0];
14+
if ($name === 'finalize') {
15+
return implode(',', $context['values']);
16+
}
17+
if (empty($context)) {
18+
$context = ['total' => 0, 'values' => []];
19+
}
20+
$context['total'] += (int) $arguments[2];
21+
$context['values'][] = $context['total'];
22+
return $context;
23+
}
24+
}
25+
$o = new TrampolineTest();
26+
$step = [$o, 'step'];
27+
$finalize = [$o, 'finalize'];
28+
29+
var_dump($db->createAggregate('', $step, $finalize, 1));
30+
31+
try {
32+
var_dump($db->createAggregate('S', $step, $finalize, new stdClass()));
33+
} catch (\Throwable $e) {
34+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
35+
}
36+
try {
37+
var_dump($db->createAggregate('S', $step, 'no_func', 1));
38+
} catch (\Throwable $e) {
39+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
40+
}
41+
42+
echo "Invalid SQLite3 object:\n";
43+
$rc = new ReflectionClass(SQLite3::class);
44+
$obj = $rc->newInstanceWithoutConstructor();
45+
46+
try {
47+
var_dump($obj->createAggregate('S', $step, $finalize, 1));
48+
} catch (\Throwable $e) {
49+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
50+
}
51+
52+
var_dump($db->createAggregate('S', $step, $finalize, 1));
53+
54+
echo "Closing database\n";
55+
var_dump($db->close());
56+
echo "Done\n";
57+
?>
58+
--EXPECT--
59+
bool(false)
60+
TypeError: SQLite3::createAggregate(): Argument #4 ($argCount) must be of type int, stdClass given
61+
TypeError: SQLite3::createAggregate(): Argument #3 ($finalCallback) must be a valid callback, function "no_func" not found or invalid function name
62+
Invalid SQLite3 object:
63+
Error: The SQLite3 object has not been correctly initialised or is already closed
64+
bool(true)
65+
Closing database
66+
bool(true)
67+
Done

‎ext/sqlite3/tests/sqlite3_trampoline_createcollation_no_leak.phpt

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
SQLite3::createFunction use F ZPP for trampoline callback and does not leak
3+
--EXTENSIONS--
4+
sqlite3
5+
--FILE--
6+
<?php
7+
8+
require_once(__DIR__ . '/new_db.inc');
9+
10+
class TrampolineTest {
11+
public function __call(string $name, array $arguments) {
12+
echo 'Trampoline for ', $name, PHP_EOL;
13+
return strnatcmp(...$arguments);
14+
}
15+
}
16+
$o = new TrampolineTest();
17+
$callback = [$o, 'NAT'];
18+
19+
var_dump($db->createCollation('', $callback));
20+
try {
21+
var_dump($db->createCollation('NAT', $callback, new stdClass()));
22+
} catch (\Throwable $e) {
23+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
24+
}
25+
26+
echo "Invalid SQLite3 object:\n";
27+
$rc = new ReflectionClass(SQLite3::class);
28+
$obj = $rc->newInstanceWithoutConstructor();
29+
30+
try {
31+
var_dump($obj->createCollation('NAT', $callback));
32+
} catch (\Throwable $e) {
33+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
34+
}
35+
36+
var_dump($db->createCollation('NAT', $callback));
37+
38+
?>
39+
--EXPECT--
40+
bool(false)
41+
ArgumentCountError: SQLite3::createCollation() expects exactly 2 arguments, 3 given
42+
Invalid SQLite3 object:
43+
Error: The SQLite3 object has not been correctly initialised or is already closed
44+
bool(true)

‎ext/sqlite3/tests/sqlite3_trampoline_createfunction_no_leak.phpt

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
--TEST--
2+
SQLite3::createFunction use F ZPP for trampoline callback and does not leak
3+
--EXTENSIONS--
4+
sqlite3
5+
--FILE--
6+
<?php
7+
8+
require_once(__DIR__ . '/new_db.inc');
9+
10+
class TrampolineTest {
11+
public function __call(string $name, array $arguments) {
12+
echo 'Trampoline for ', $name, PHP_EOL;
13+
return strtoupper($arguments[0]);
14+
}
15+
}
16+
$o = new TrampolineTest();
17+
$callback = [$o, 'strtoupper'];
18+
19+
var_dump($db->createfunction('', $callback));
20+
21+
try {
22+
var_dump($db->createfunction('strtoupper', $callback, new stdClass()));
23+
} catch (\Throwable $e) {
24+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
25+
}
26+
27+
echo "Invalid SQLite3 object:\n";
28+
$rc = new ReflectionClass(SQLite3::class);
29+
$obj = $rc->newInstanceWithoutConstructor();
30+
31+
try {
32+
var_dump($obj->createfunction('strtoupper', $callback));
33+
} catch (\Throwable $e) {
34+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
35+
}
36+
37+
var_dump($db->createfunction('strtoupper', $callback));
38+
39+
?>
40+
--EXPECT--
41+
bool(false)
42+
TypeError: SQLite3::createFunction(): Argument #3 ($argCount) must be of type int, stdClass given
43+
Invalid SQLite3 object:
44+
Error: The SQLite3 object has not been correctly initialised or is already closed
45+
bool(true)

‎ext/sqlite3/tests/sqlite3_trampoline_setauthorizer_no_leak.phpt

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
--TEST--
2+
SQLite3::setAuthorizer use F ZPP for trampoline callback and does not leak
3+
--EXTENSIONS--
4+
sqlite3
5+
--FILE--
6+
<?php
7+
8+
class TrampolineTest {
9+
public function __call(string $name, array $arguments) {
10+
echo 'Trampoline for ', $name, PHP_EOL;
11+
if ($arguments[0] == SQLite3::SELECT) {
12+
return SQLite3::OK;
13+
}
14+
15+
return SQLite3::DENY;
16+
}
17+
}
18+
$o = new TrampolineTest();
19+
$callback = [$o, 'authorizer'];
20+
21+
$db = new SQLite3(':memory:');
22+
$db->enableExceptions(true);
23+
24+
echo "Invalid SQLite3 object:\n";
25+
$rc = new ReflectionClass(SQLite3::class);
26+
$obj = $rc->newInstanceWithoutConstructor();
27+
28+
try {
29+
var_dump($obj->setAuthorizer($callback));
30+
} catch (\Throwable $e) {
31+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
32+
}
33+
34+
$db->setAuthorizer($callback);
35+
36+
?>
37+
DONE
38+
--EXPECT--
39+
Invalid SQLite3 object:
40+
Error: The SQLite3 object has not been correctly initialised or is already closed
41+
DONE

0 commit comments

Comments
 (0)