Skip to content

Commit c019421

Browse files
authored
Fix regression from GH-8587 (#8615)
* Fix regression from GH-8587 Streams hold a reference to the stream wrapper. User stream wrappers must not be released until the streams themselves are closed. * Add test for directories
1 parent 12a3066 commit c019421

File tree

3 files changed

+87
-2
lines changed

3 files changed

+87
-2
lines changed

Zend/tests/gh8548.phpt

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,35 @@ class Wrapper
1111
{
1212
return true;
1313
}
14+
15+
public function stream_eof(): bool
16+
{
17+
return true;
18+
}
1419
}
1520

1621
function test() {
1722
if (!stream_wrapper_register('foo', \Wrapper::class)) {
1823
throw new \Exception('Could not register stream wrapper');
1924
}
25+
26+
$file = fopen('foo://bar', 'r');
27+
2028
if (!stream_wrapper_unregister('foo')) {
2129
throw new \Exception('Could not unregister stream wrapper');
2230
}
31+
32+
$wrapper = stream_get_meta_data($file)['wrapper_data'];
33+
if (!$wrapper instanceof Wrapper) {
34+
throw new \Exception('Wrapper is not of expected type');
35+
}
36+
37+
fclose($file);
38+
unset($file);
2339
}
2440

2541
// The first iterations will allocate space for things like the resource list
26-
for ($i = 0; $i < 5; $i++) {
42+
for ($i = 0; $i < 10; $i++) {
2743
test();
2844
}
2945

Zend/tests/gh8548_2.phpt

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
--TEST--
2+
GH-8548: Test stream_wrapper_unregister() for directories
3+
--FILE--
4+
<?php
5+
6+
class Wrapper
7+
{
8+
public $context;
9+
10+
public function dir_opendir(string $path, int $options): bool
11+
{
12+
return true;
13+
}
14+
15+
public function stream_eof(): bool
16+
{
17+
return true;
18+
}
19+
}
20+
21+
function test() {
22+
if (!stream_wrapper_register('foo', \Wrapper::class)) {
23+
throw new \Exception('Could not register stream wrapper');
24+
}
25+
26+
$dir = opendir('foo://bar');
27+
28+
if (!stream_wrapper_unregister('foo')) {
29+
throw new \Exception('Could not unregister stream wrapper');
30+
}
31+
32+
$wrapper = stream_get_meta_data($dir)['wrapper_data'];
33+
if (!$wrapper instanceof Wrapper) {
34+
throw new \Exception('Wrapper is not of expected type');
35+
}
36+
37+
closedir($dir);
38+
unset($dir);
39+
}
40+
41+
// The first iterations will allocate space for things like the resource list
42+
for ($i = 0; $i < 10; $i++) {
43+
test();
44+
}
45+
46+
$before = memory_get_usage();
47+
for ($i = 0; $i < 1000; $i++) {
48+
test();
49+
}
50+
$after = memory_get_usage();
51+
52+
var_dump($before === $after);
53+
54+
?>
55+
--EXPECT--
56+
bool(true)

main/streams/userspace.c

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ struct php_user_stream_wrapper {
4242
};
4343

4444
static php_stream *user_wrapper_opener(php_stream_wrapper *wrapper, const char *filename, const char *mode, int options, zend_string **opened_path, php_stream_context *context STREAMS_DC);
45+
static int user_wrapper_close(php_stream_wrapper *wrapper, php_stream *stream);
4546
static int user_wrapper_stat_url(php_stream_wrapper *wrapper, const char *url, int flags, php_stream_statbuf *ssb, php_stream_context *context);
4647
static int user_wrapper_unlink(php_stream_wrapper *wrapper, const char *url, int options, php_stream_context *context);
4748
static int user_wrapper_rename(php_stream_wrapper *wrapper, const char *url_from, const char *url_to, int options, php_stream_context *context);
@@ -53,7 +54,7 @@ static php_stream *user_wrapper_opendir(php_stream_wrapper *wrapper, const char
5354

5455
static const php_stream_wrapper_ops user_stream_wops = {
5556
user_wrapper_opener,
56-
NULL, /* close - the streams themselves know how */
57+
user_wrapper_close,
5758
NULL, /* stat - the streams themselves know how */
5859
user_wrapper_stat_url,
5960
user_wrapper_opendir,
@@ -375,6 +376,8 @@ static php_stream *user_wrapper_opener(php_stream_wrapper *wrapper, const char *
375376

376377
/* set wrapper data to be a reference to our object */
377378
ZVAL_COPY(&stream->wrapperdata, &us->object);
379+
380+
GC_ADDREF(us->wrapper->resource);
378381
} else {
379382
php_stream_wrapper_log_error(wrapper, options, "\"%s::" USERSTREAM_OPEN "\" call failed",
380383
ZSTR_VAL(us->wrapper->ce->name));
@@ -399,6 +402,14 @@ static php_stream *user_wrapper_opener(php_stream_wrapper *wrapper, const char *
399402
return stream;
400403
}
401404

405+
static int user_wrapper_close(php_stream_wrapper *wrapper, php_stream *stream)
406+
{
407+
struct php_user_stream_wrapper *uwrap = (struct php_user_stream_wrapper*)wrapper->abstract;
408+
zend_list_delete(uwrap->resource);
409+
// FIXME: Unused?
410+
return 0;
411+
}
412+
402413
static php_stream *user_wrapper_opendir(php_stream_wrapper *wrapper, const char *filename, const char *mode,
403414
int options, zend_string **opened_path, php_stream_context *context STREAMS_DC)
404415
{
@@ -440,6 +451,8 @@ static php_stream *user_wrapper_opendir(php_stream_wrapper *wrapper, const char
440451

441452
/* set wrapper data to be a reference to our object */
442453
ZVAL_COPY(&stream->wrapperdata, &us->object);
454+
455+
GC_ADDREF(us->wrapper->resource);
443456
} else {
444457
php_stream_wrapper_log_error(wrapper, options, "\"%s::" USERSTREAM_DIR_OPEN "\" call failed",
445458
ZSTR_VAL(us->wrapper->ce->name));

0 commit comments

Comments
 (0)