Skip to content

Commit 677a1f8

Browse files
GirgiasGirgias
authored
ext/standard/stream: Use FCC instead of zval for notification callback (php#19024)
Also check that the callable exists while setting the option
1 parent c338057 commit 677a1f8

5 files changed

Lines changed: 76 additions & 26 deletions

‎ext/standard/streamsfuncs.c‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -856,10 +856,7 @@ PHP_FUNCTION(stream_select)
856856
static void user_space_stream_notifier(php_stream_context *context, int notifycode, int severity,
857857
char *xmsg, int xcode, size_t bytes_sofar, size_t bytes_max, void * ptr)
858858
{
859-
zval *callback = &context->notifier->ptr;
860-
zval retval;
861859
zval zvs[6];
862-
int i;
863860

864861
ZVAL_LONG(&zvs[0], notifycode);
865862
ZVAL_LONG(&zvs[1], severity);
@@ -872,21 +869,19 @@ static void user_space_stream_notifier(php_stream_context *context, int notifyco
872869
ZVAL_LONG(&zvs[4], bytes_sofar);
873870
ZVAL_LONG(&zvs[5], bytes_max);
874871

875-
if (FAILURE == call_user_function(NULL, NULL, callback, &retval, 6, zvs)) {
876-
php_error_docref(NULL, E_WARNING, "Failed to call user notifier");
877-
}
878-
for (i = 0; i < 6; i++) {
879-
zval_ptr_dtor(&zvs[i]);
880-
}
881-
zval_ptr_dtor(&retval);
872+
zend_call_known_fcc(context->notifier->fcc, NULL, 6, zvs, NULL);
873+
/* Free refcounted string parameter */
874+
zval_ptr_dtor_str(&zvs[2]);
882875
}
883876

884877
static void user_space_stream_notifier_dtor(php_stream_notifier *notifier)
885878
{
886-
if (notifier && Z_TYPE(notifier->ptr) != IS_UNDEF) {
887-
zval_ptr_dtor(&notifier->ptr);
888-
ZVAL_UNDEF(&notifier->ptr);
889-
}
879+
ZEND_ASSERT(notifier);
880+
ZEND_ASSERT(notifier->fcc);
881+
ZEND_ASSERT(notifier->fcc->function_handler);
882+
zend_fcc_dtor(notifier->fcc);
883+
efree(notifier->fcc);
884+
notifier->fcc = NULL;
890885
}
891886

892887
static zend_result parse_context_options(php_stream_context *context, HashTable *options)
@@ -924,9 +919,19 @@ static zend_result parse_context_params(php_stream_context *context, HashTable *
924919
context->notifier = NULL;
925920
}
926921

922+
zend_fcall_info_cache *fcc = emalloc(sizeof(*fcc));
923+
char *error;
924+
if (!zend_is_callable_ex(tmp, NULL, 0, NULL, fcc, &error)) {
925+
zend_argument_type_error(1, "must be an array with valid callbacks as values, %s", error);
926+
efree(fcc);
927+
efree(error);
928+
return FAILURE;
929+
}
930+
zend_fcc_addref(fcc);
931+
927932
context->notifier = php_stream_notification_alloc();
928933
context->notifier->func = user_space_stream_notifier;
929-
ZVAL_COPY(&context->notifier->ptr, tmp);
934+
context->notifier->fcc = fcc;
930935
context->notifier->dtor = user_space_stream_notifier_dtor;
931936
}
932937
if (NULL != (tmp = zend_hash_str_find(params, "options", sizeof("options")-1))) {
@@ -1123,9 +1128,11 @@ PHP_FUNCTION(stream_context_get_params)
11231128
}
11241129

11251130
array_init(return_value);
1126-
if (context->notifier && Z_TYPE(context->notifier->ptr) != IS_UNDEF && context->notifier->func == user_space_stream_notifier) {
1127-
Z_TRY_ADDREF(context->notifier->ptr);
1128-
add_assoc_zval_ex(return_value, "notification", sizeof("notification")-1, &context->notifier->ptr);
1131+
if (context->notifier && context->notifier->fcc) {
1132+
ZEND_ASSERT(context->notifier->func == user_space_stream_notifier);
1133+
zval fn;
1134+
zend_get_callable_zval_from_fcc(context->notifier->fcc, &fn);
1135+
add_assoc_zval_ex(return_value, ZEND_STRL("notification"), &fn);
11291136
}
11301137
Z_TRY_ADDREF(context->options);
11311138
add_assoc_zval_ex(return_value, "options", sizeof("options")-1, &context->options);

‎ext/standard/tests/streams/stream_context_get_params_001.phpt‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,19 @@ stream_context_get_params()
44
<?php
55

66
$ctx = stream_context_create();
7-
var_dump($ctx);
87
var_dump(stream_context_get_params($ctx));
98

109
var_dump(stream_context_set_option($ctx, "foo","bar","baz"));
1110
var_dump(stream_context_get_params($ctx));
1211

12+
function stream_notification_callback() {}
1313
var_dump(stream_context_set_params($ctx, array("notification" => "stream_notification_callback")));
1414
var_dump(stream_context_get_params($ctx));
1515

16-
var_dump(stream_context_set_params($ctx, array("notification" => array("stream","notification_callback"))));
16+
class MyStream {
17+
public static function notification_callback() {}
18+
}
19+
var_dump(stream_context_set_params($ctx, array("notification" => ["MyStream", "notification_callback"])));
1720
var_dump(stream_context_get_params($ctx));
1821

1922
var_dump(stream_context_get_params($ctx));
@@ -22,8 +25,7 @@ var_dump(stream_context_get_params($ctx));
2225
var_dump(stream_context_get_options($ctx));
2326

2427
?>
25-
--EXPECTF--
26-
resource(%d) of type (stream-context)
28+
--EXPECT--
2729
array(1) {
2830
["options"]=>
2931
array(0) {
@@ -58,7 +60,7 @@ array(2) {
5860
["notification"]=>
5961
array(2) {
6062
[0]=>
61-
string(6) "stream"
63+
string(8) "MyStream"
6264
[1]=>
6365
string(21) "notification_callback"
6466
}
@@ -75,7 +77,7 @@ array(2) {
7577
["notification"]=>
7678
array(2) {
7779
[0]=>
78-
string(6) "stream"
80+
string(8) "MyStream"
7981
[1]=>
8082
string(21) "notification_callback"
8183
}
@@ -99,7 +101,7 @@ array(2) {
99101
["notification"]=>
100102
array(2) {
101103
[0]=>
102-
string(6) "stream"
104+
string(8) "MyStream"
103105
[1]=>
104106
string(21) "notification_callback"
105107
}

‎ext/standard/tests/streams/stream_context_set_params_notification_error.phpt‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
stream_context_set_params() with invalid notification option
3+
--FILE--
4+
<?php
5+
6+
$ctx = stream_context_create();
7+
try {
8+
var_dump(stream_context_set_params($ctx, ["notification" => "fn_not_exist"]));
9+
} catch (\Throwable $e) {
10+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
11+
}
12+
try {
13+
var_dump(stream_context_set_params($ctx, ["notification" => ["myclass", "fn_not_exist"]]));
14+
} catch (\Throwable $e) {
15+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
16+
}
17+
18+
?>
19+
--EXPECT--
20+
TypeError: stream_context_set_params(): Argument #1 ($context) must be an array with valid callbacks as values, function "fn_not_exist" not found or invalid function name
21+
TypeError: stream_context_set_params(): Argument #1 ($context) must be an array with valid callbacks as values, class "myclass" not found

‎ext/standard/tests/streams/stream_context_set_params_notification_valid_then_invalid.phpt‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
stream_context_set_params() with valid, then invalid notification option
3+
--FILE--
4+
<?php
5+
6+
function foo() {}
7+
8+
$ctx = stream_context_create();
9+
var_dump(stream_context_set_params($ctx, ["notification" => "foo"]));
10+
11+
try {
12+
var_dump(stream_context_set_params($ctx, ["notification" => "fn_not_exist"]));
13+
} catch (\Throwable $e) {
14+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
15+
}
16+
17+
?>
18+
--EXPECT--
19+
bool(true)
20+
TypeError: stream_context_set_params(): Argument #1 ($context) must be an array with valid callbacks as values, function "fn_not_exist" not found or invalid function name

‎main/streams/php_stream_context.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ typedef struct _php_stream_notifier php_stream_notifier;
4444
struct _php_stream_notifier {
4545
php_stream_notification_func func;
4646
void (*dtor)(php_stream_notifier *notifier);
47-
zval ptr;
47+
zend_fcall_info_cache *fcc;
4848
int mask;
4949
size_t progress, progress_max; /* position for progress notification */
5050
};

0 commit comments

Comments
 (0)