Skip to content

Commit bee31be

Browse files
GirgiasGirgias
committed
ext/sqlite3: Use new F ZPP modifier
1 parent 87715c5 commit bee31be

5 files changed

+164
-30
lines changed

‎ext/sqlite3/sqlite3.c

Lines changed: 15 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -942,13 +942,16 @@ PHP_METHOD(SQLite3, createFunction)
942942
zend_long flags = 0;
943943
db_obj = Z_SQLITE3_DB_P(object);
944944

945-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sf|ll", &sql_func, &sql_func_len, &fci, &fcc, &sql_func_num_args, &flags) == FAILURE) {
945+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sF|ll", &sql_func, &sql_func_len, &fci, &fcc, &sql_func_num_args, &flags) == FAILURE) {
946+
zend_release_fcall_info_cache(&fcc);
946947
RETURN_THROWS();
947948
}
948949

949950
SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3)
950951

951952
if (!sql_func_len) {
953+
/* TODO Add warning/ValueError that name cannot be empty? */
954+
zend_release_fcall_info_cache(&fcc);
952955
RETURN_FALSE;
953956
}
954957

@@ -990,13 +993,18 @@ PHP_METHOD(SQLite3, createAggregate)
990993
zend_long sql_func_num_args = -1;
991994
db_obj = Z_SQLITE3_DB_P(object);
992995

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) {
996+
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) {
997+
zend_release_fcall_info_cache(&step_fcc);
998+
zend_release_fcall_info_cache(&fini_fcc);
994999
RETURN_THROWS();
9951000
}
9961001

9971002
SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3)
9981003

9991004
if (!sql_func_len) {
1005+
/* TODO Add warning/ValueError that name cannot be empty? */
1006+
zend_release_fcall_info_cache(&step_fcc);
1007+
zend_release_fcall_info_cache(&fini_fcc);
10001008
RETURN_FALSE;
10011009
}
10021010

@@ -1005,19 +1013,7 @@ PHP_METHOD(SQLite3, createAggregate)
10051013
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) {
10061014
func->func_name = estrdup(sql_func);
10071015

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-
}
10141016
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-
}
10211017
zend_fcc_dup(&func->fini, &fini_fcc);
10221018

10231019
func->argc = sql_func_num_args;
@@ -1044,26 +1040,22 @@ PHP_METHOD(SQLite3, createCollation)
10441040
zend_fcall_info_cache fcc;
10451041
db_obj = Z_SQLITE3_DB_P(object);
10461042

1047-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sf", &collation_name, &collation_name_len, &fci, &fcc) == FAILURE) {
1043+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "sF", &collation_name, &collation_name_len, &fci, &fcc) == FAILURE) {
10481044
RETURN_THROWS();
10491045
}
10501046

10511047
SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3)
10521048

10531049
if (!collation_name_len) {
1050+
/* TODO Add warning/ValueError that name cannot be empty? */
1051+
zend_release_fcall_info_cache(&fcc);
10541052
RETURN_FALSE;
10551053
}
10561054

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

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

10691061
collation->next = db_obj->collations;
@@ -1317,7 +1309,7 @@ PHP_METHOD(SQLite3, setAuthorizer)
13171309
zend_fcall_info_cache fcc;
13181310

13191311
ZEND_PARSE_PARAMETERS_START(1, 1)
1320-
Z_PARAM_FUNC_OR_NULL(fci, fcc)
1312+
Z_PARAM_FUNC_NO_TRAMPOLINE_FREE_OR_NULL(fci, fcc)
13211313
ZEND_PARSE_PARAMETERS_END();
13221314

13231315
SQLITE3_CHECK_INITIALIZED(db_obj, db_obj->initialised, SQLite3)
@@ -1329,14 +1321,7 @@ PHP_METHOD(SQLite3, setAuthorizer)
13291321

13301322
/* Only enable userland authorizer if argument is not NULL */
13311323
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);
1324+
zend_fcc_dup(&db_obj->authorizer_fcc, &fcc);
13401325
}
13411326

13421327
RETURN_TRUE;

‎ext/sqlite3/tests/sqlite3_trampoline_create_aggregate_no_leak.phpt

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
--TEST--
2+
SQLite3::createAggregate() trampoline callback
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+
var_dump($db->createAggregate('S', $step, $finalize, 1));
43+
44+
echo "Closing database\n";
45+
var_dump($db->close());
46+
echo "Done\n";
47+
?>
48+
--EXPECT--
49+
bool(false)
50+
TypeError: SQLite3::createAggregate(): Argument #4 ($argCount) must be of type int, stdClass given
51+
TypeError: SQLite3::createAggregate(): Argument #3 ($finalCallback) must be a valid callback, function "no_func" not found or invalid function name
52+
bool(true)
53+
Closing database
54+
bool(true)
55+
Done

‎ext/sqlite3/tests/sqlite3_trampoline_createcollation_no_leak.phpt

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
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+
var_dump($db->createCollation('NAT', $callback));
27+
28+
?>
29+
--EXPECT--
30+
bool(false)
31+
ArgumentCountError: SQLite3::createCollation() expects exactly 2 arguments, 3 given
32+
bool(true)

‎ext/sqlite3/tests/sqlite3_trampoline_createfunction_no_leak.phpt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
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+
var_dump($db->createfunction('strtoupper', $callback));
28+
29+
?>
30+
--EXPECT--
31+
bool(false)
32+
TypeError: SQLite3::createFunction(): Argument #3 ($argCount) must be of type int, stdClass given
33+
bool(true)

‎ext/sqlite3/tests/sqlite3_trampoline_setauthorizer_no_leak.phpt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
SQLite3 user authorizer trampoline callback
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+
$db->setAuthorizer($callback);
25+
26+
?>
27+
DONE
28+
--EXPECT--
29+
DONE

0 commit comments

Comments
 (0)