Skip to content

Commit a5a89cc

Browse files
committed
Fix stream_wrapper_unregister() resource leak
Closes GH-8548 Closes GH-8587
1 parent 82ab848 commit a5a89cc

File tree

3 files changed

+53
-1
lines changed

3 files changed

+53
-1
lines changed

NEWS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ PHP NEWS
7373
- Streams:
7474
. Set IP_BIND_ADDRESS_NO_PORT if available when connecting to remote host.
7575
(Cristian Rodríguez)
76+
. Fixed bug GH-8548 (stream_wrapper_unregister() leaks memory). (ilutov)
7677

7778
- Zip:
7879
. add ZipArchive::clearError() method

Zend/tests/gh8548.phpt

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
--TEST--
2+
GH-8548: stream_wrapper_unregister() leaks memory
3+
--FILE--
4+
<?php
5+
6+
class Wrapper
7+
{
8+
public $context;
9+
10+
public function stream_open(string $path, string $mode, int $options): bool
11+
{
12+
return true;
13+
}
14+
}
15+
16+
function test() {
17+
if (!stream_wrapper_register('foo', \Wrapper::class)) {
18+
throw new \Exception('Could not register stream wrapper');
19+
}
20+
if (!stream_wrapper_unregister('foo')) {
21+
throw new \Exception('Could not unregister stream wrapper');
22+
}
23+
}
24+
25+
// The first iterations will allocate space for things like the resource list
26+
for ($i = 0; $i < 5; $i++) {
27+
test();
28+
}
29+
30+
$before = memory_get_usage();
31+
for ($i = 0; $i < 1000; $i++) {
32+
test();
33+
}
34+
$after = memory_get_usage();
35+
36+
var_dump($before === $after);
37+
38+
?>
39+
--EXPECT--
40+
bool(true)

main/streams/userspace.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,10 @@
3535
static int le_protocols;
3636

3737
struct php_user_stream_wrapper {
38+
php_stream_wrapper wrapper;
3839
char * protoname;
3940
zend_class_entry *ce;
40-
php_stream_wrapper wrapper;
41+
zend_resource *resource;
4142
};
4243

4344
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);
@@ -481,10 +482,12 @@ PHP_FUNCTION(stream_wrapper_register)
481482
uwrap->wrapper.wops = &user_stream_wops;
482483
uwrap->wrapper.abstract = uwrap;
483484
uwrap->wrapper.is_url = ((flags & PHP_STREAM_IS_URL) != 0);
485+
uwrap->resource = NULL;
484486

485487
rsrc = zend_register_resource(uwrap, le_protocols);
486488

487489
if (php_register_url_stream_wrapper_volatile(protocol, &uwrap->wrapper) == SUCCESS) {
490+
uwrap->resource = rsrc;
488491
RETURN_TRUE;
489492
}
490493

@@ -510,12 +513,20 @@ PHP_FUNCTION(stream_wrapper_unregister)
510513
RETURN_THROWS();
511514
}
512515

516+
php_stream_wrapper *wrapper = zend_hash_find_ptr(php_stream_get_url_stream_wrappers_hash(), protocol);
513517
if (php_unregister_url_stream_wrapper_volatile(protocol) == FAILURE) {
514518
/* We failed */
515519
php_error_docref(NULL, E_WARNING, "Unable to unregister protocol %s://", ZSTR_VAL(protocol));
516520
RETURN_FALSE;
517521
}
518522

523+
ZEND_ASSERT(wrapper != NULL);
524+
if (wrapper->wops == &user_stream_wops) {
525+
struct php_user_stream_wrapper *uwrap = (struct php_user_stream_wrapper *)wrapper;
526+
// uwrap will be released by resource destructor
527+
zend_list_delete(uwrap->resource);
528+
}
529+
519530
RETURN_TRUE;
520531
}
521532
/* }}} */

0 commit comments

Comments
 (0)