Skip to content

Commit 6303d1f

Browse files
GirgiasGirgias
authored
ext/sqlite3: Use new F ZPP modifier (#14040)
1 parent d2d4596 commit 6303d1f

6 files changed

+302
-54
lines changed

‎ext/sqlite3/sqlite3.c

Lines changed: 46 additions & 54 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"); \
@@ -936,33 +943,29 @@ PHP_METHOD(SQLite3, createFunction)
936943
php_sqlite3_func *func;
937944
char *sql_func;
938945
size_t sql_func_len;
939-
zend_fcall_info fci;
940-
zend_fcall_info_cache fcc;
946+
zend_fcall_info fci = empty_fcall_info;
947+
zend_fcall_info_cache fcc = empty_fcall_info_cache;
941948
zend_long sql_func_num_args = -1;
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

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

957967
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) {
958968
func->func_name = estrdup(sql_func);
959-
960-
if (!ZEND_FCC_INITIALIZED(fcc)) {
961-
zend_is_callable_ex(&fci.function_name, NULL, IS_CALLABLE_SUPPRESS_DEPRECATIONS, NULL, &fcc, NULL);
962-
/* Call trampoline has been cleared by zpp. Refetch it, because we want to deal
963-
* with it outselves. It is important that it is not refetched on every call,
964-
* because calls may occur from different scopes. */
965-
}
966969
zend_fcc_dup(&func->func, &fcc);
967970

968971
func->argc = sql_func_num_args;
@@ -972,6 +975,7 @@ PHP_METHOD(SQLite3, createFunction)
972975
RETURN_TRUE;
973976
}
974977
efree(func);
978+
zend_release_fcall_info_cache(&fcc);
975979

976980
RETURN_FALSE;
977981
}
@@ -985,39 +989,34 @@ PHP_METHOD(SQLite3, createAggregate)
985989
php_sqlite3_func *func;
986990
char *sql_func;
987991
size_t sql_func_len;
988-
zend_fcall_info step_fci, fini_fci;
989-
zend_fcall_info_cache step_fcc, fini_fcc;
992+
zend_fcall_info step_fci = empty_fcall_info;
993+
zend_fcall_info_cache step_fcc = empty_fcall_info_cache;
994+
zend_fcall_info fini_fci = empty_fcall_info;
995+
zend_fcall_info_cache fini_fcc = empty_fcall_info_cache;
990996
zend_long sql_func_num_args = -1;
991997
db_obj = Z_SQLITE3_DB_P(object);
992998

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) {
994-
RETURN_THROWS();
999+
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) {
1000+
goto error;
9951001
}
9961002

997-
SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3)
1003+
/* Cannot use SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE() as we have 2 FCCs */
1004+
if (!db_obj || !db_obj->initialised) {
1005+
zend_throw_error(NULL, "The SQLite3 object has not been correctly initialised or is already closed");
1006+
goto error;
1007+
}
9981008

9991009
if (!sql_func_len) {
1000-
RETURN_FALSE;
1010+
/* TODO Add warning/ValueError that name cannot be empty? */
1011+
goto error;
10011012
}
10021013

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

10051016
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) {
10061017
func->func_name = estrdup(sql_func);
10071018

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-
}
10141019
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-
}
10211020
zend_fcc_dup(&func->fini, &fini_fcc);
10221021

10231022
func->argc = sql_func_num_args;
@@ -1028,6 +1027,10 @@ PHP_METHOD(SQLite3, createAggregate)
10281027
}
10291028
efree(func);
10301029

1030+
error:
1031+
zend_release_fcall_info_cache(&step_fcc);
1032+
zend_release_fcall_info_cache(&fini_fcc);
1033+
10311034
RETURN_FALSE;
10321035
}
10331036
/* }}} */
@@ -1040,30 +1043,26 @@ PHP_METHOD(SQLite3, createCollation)
10401043
php_sqlite3_collation *collation;
10411044
char *collation_name;
10421045
size_t collation_name_len;
1043-
zend_fcall_info fci;
1044-
zend_fcall_info_cache fcc;
1046+
zend_fcall_info fci = empty_fcall_info;
1047+
zend_fcall_info_cache fcc = empty_fcall_info_cache;
10451048
db_obj = Z_SQLITE3_DB_P(object);
10461049

1047-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sf", &collation_name, &collation_name_len, &fci, &fcc) == FAILURE) {
1050+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sF", &collation_name, &collation_name_len, &fci, &fcc) == FAILURE) {
10481051
RETURN_THROWS();
10491052
}
10501053

1051-
SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3)
1054+
SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE(db_obj, db_obj->initialised, SQLite3, &fcc);
10521055

10531056
if (!collation_name_len) {
1057+
/* TODO Add warning/ValueError that name cannot be empty? */
1058+
zend_release_fcall_info_cache(&fcc);
10541059
RETURN_FALSE;
10551060
}
10561061

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

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-
}
10671066
zend_fcc_dup(&collation->cmp_func, &fcc);
10681067

10691068
collation->next = db_obj->collations;
@@ -1072,6 +1071,7 @@ PHP_METHOD(SQLite3, createCollation)
10721071
RETURN_TRUE;
10731072
}
10741073
efree(collation);
1074+
zend_release_fcall_info_cache(&fcc);
10751075

10761076
RETURN_FALSE;
10771077
}
@@ -1310,17 +1310,16 @@ PHP_METHOD(SQLite3, enableExceptions)
13101310
/* {{{ Register a callback function to be used as an authorizer by SQLite. The callback should return SQLite3::OK, SQLite3::IGNORE or SQLite3::DENY. */
13111311
PHP_METHOD(SQLite3, setAuthorizer)
13121312
{
1313-
php_sqlite3_db_object *db_obj;
1314-
zval *object = ZEND_THIS;
1315-
db_obj = Z_SQLITE3_DB_P(object);
1316-
zend_fcall_info fci;
1317-
zend_fcall_info_cache fcc;
1313+
zend_fcall_info fci = empty_fcall_info;
1314+
zend_fcall_info_cache fcc = empty_fcall_info_cache;
13181315

13191316
ZEND_PARSE_PARAMETERS_START(1, 1)
1320-
Z_PARAM_FUNC_OR_NULL(fci, fcc)
1317+
Z_PARAM_FUNC_NO_TRAMPOLINE_FREE_OR_NULL(fci, fcc)
13211318
ZEND_PARSE_PARAMETERS_END();
13221319

1323-
SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3)
1320+
php_sqlite3_db_object *db_obj = Z_SQLITE3_DB_P(ZEND_THIS);
1321+
1322+
SQLITE3_CHECK_INITIALIZED_FREE_TRAMPOLINE(db_obj, db_obj->initialised, SQLite3, &fcc);
13241323

13251324
/* Clear previously set callback */
13261325
if (ZEND_FCC_INITIALIZED(db_obj->authorizer_fcc)) {
@@ -1329,14 +1328,7 @@ PHP_METHOD(SQLite3, setAuthorizer)
13291328

13301329
/* Only enable userland authorizer if argument is not NULL */
13311330
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);
1331+
zend_fcc_dup(&db_obj->authorizer_fcc, &fcc);
13401332
}
13411333

13421334
RETURN_TRUE;

‎ext/sqlite3/tests/sqlite3_trampoline_create_aggregate_no_leak.phpt

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
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(new stdClass(), $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, $finalize, new stdClass()));
38+
} catch (\Throwable $e) {
39+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
40+
}
41+
try {
42+
var_dump($db->createAggregate('S', $step, 'no_func', 1));
43+
} catch (\Throwable $e) {
44+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
45+
}
46+
try {
47+
var_dump($db->createAggregate('S', 'no_func', $finalize, 1));
48+
} catch (\Throwable $e) {
49+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
50+
}
51+
52+
try {
53+
var_dump($db->createAggregate('S', $step, $finalize, 'not a number'));
54+
} catch (\Throwable $e) {
55+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
56+
}
57+
58+
echo "Invalid SQLite3 object:\n";
59+
$rc = new ReflectionClass(SQLite3::class);
60+
$obj = $rc->newInstanceWithoutConstructor();
61+
62+
try {
63+
var_dump($obj->createAggregate('S', $step, $finalize, 1));
64+
} catch (\Throwable $e) {
65+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
66+
}
67+
68+
var_dump($db->createAggregate('S', $step, $finalize, 1));
69+
70+
echo "Closing database\n";
71+
var_dump($db->close());
72+
echo "Done\n";
73+
?>
74+
--EXPECT--
75+
bool(false)
76+
TypeError: SQLite3::createAggregate(): Argument #1 ($name) must be of type string, stdClass given
77+
TypeError: SQLite3::createAggregate(): Argument #4 ($argCount) must be of type int, stdClass given
78+
TypeError: SQLite3::createAggregate(): Argument #3 ($finalCallback) must be a valid callback, function "no_func" not found or invalid function name
79+
TypeError: SQLite3::createAggregate(): Argument #2 ($stepCallback) must be a valid callback, function "no_func" not found or invalid function name
80+
TypeError: SQLite3::createAggregate(): Argument #4 ($argCount) must be of type int, string given
81+
Invalid SQLite3 object:
82+
Error: The SQLite3 object has not been correctly initialised or is already closed
83+
bool(true)
84+
Closing database
85+
bool(true)
86+
Done

‎ext/sqlite3/tests/sqlite3_trampoline_createcollation_no_leak.phpt

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
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+
21+
try {
22+
var_dump($db->createCollation(new stdClass(), $callback));
23+
} catch (\Throwable $e) {
24+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
25+
}
26+
try {
27+
var_dump($db->createCollation('NAT', $callback, new stdClass()));
28+
} catch (\Throwable $e) {
29+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
30+
}
31+
32+
echo "Invalid SQLite3 object:\n";
33+
$rc = new ReflectionClass(SQLite3::class);
34+
$obj = $rc->newInstanceWithoutConstructor();
35+
36+
try {
37+
var_dump($obj->createCollation('NAT', $callback));
38+
} catch (\Throwable $e) {
39+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
40+
}
41+
42+
var_dump($db->createCollation('NAT', $callback));
43+
44+
?>
45+
--EXPECT--
46+
bool(false)
47+
TypeError: SQLite3::createCollation(): Argument #1 ($name) must be of type string, stdClass given
48+
ArgumentCountError: SQLite3::createCollation() expects exactly 2 arguments, 3 given
49+
Invalid SQLite3 object:
50+
Error: The SQLite3 object has not been correctly initialised or is already closed
51+
bool(true)

0 commit comments

Comments
 (0)