Skip to content

Fix regression from GH-8587 #8615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion Zend/tests/gh8548.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,35 @@ class Wrapper
{
return true;
}

public function stream_eof(): bool
{
return true;
}
}

function test() {
if (!stream_wrapper_register('foo', \Wrapper::class)) {
throw new \Exception('Could not register stream wrapper');
}

$file = fopen('foo://bar', 'r');

if (!stream_wrapper_unregister('foo')) {
throw new \Exception('Could not unregister stream wrapper');
}

$wrapper = stream_get_meta_data($file)['wrapper_data'];
if (!$wrapper instanceof Wrapper) {
throw new \Exception('Wrapper is not of expected type');
}

fclose($file);
unset($file);
}

// The first iterations will allocate space for things like the resource list
for ($i = 0; $i < 5; $i++) {
for ($i = 0; $i < 10; $i++) {
test();
}

Expand Down
56 changes: 56 additions & 0 deletions Zend/tests/gh8548_2.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
--TEST--
GH-8548: Test stream_wrapper_unregister() for directories
--FILE--
<?php

class Wrapper
{
public $context;

public function dir_opendir(string $path, int $options): bool
{
return true;
}

public function stream_eof(): bool
{
return true;
}
}

function test() {
if (!stream_wrapper_register('foo', \Wrapper::class)) {
throw new \Exception('Could not register stream wrapper');
}

$dir = opendir('foo://bar');

if (!stream_wrapper_unregister('foo')) {
throw new \Exception('Could not unregister stream wrapper');
}

$wrapper = stream_get_meta_data($dir)['wrapper_data'];
if (!$wrapper instanceof Wrapper) {
throw new \Exception('Wrapper is not of expected type');
}

closedir($dir);
unset($dir);
}

// The first iterations will allocate space for things like the resource list
for ($i = 0; $i < 10; $i++) {
test();
}

$before = memory_get_usage();
for ($i = 0; $i < 1000; $i++) {
test();
}
$after = memory_get_usage();

var_dump($before === $after);

?>
--EXPECT--
bool(true)
15 changes: 14 additions & 1 deletion main/streams/userspace.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ struct php_user_stream_wrapper {
};

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);
static int user_wrapper_close(php_stream_wrapper *wrapper, php_stream *stream);
static int user_wrapper_stat_url(php_stream_wrapper *wrapper, const char *url, int flags, php_stream_statbuf *ssb, php_stream_context *context);
static int user_wrapper_unlink(php_stream_wrapper *wrapper, const char *url, int options, php_stream_context *context);
static int user_wrapper_rename(php_stream_wrapper *wrapper, const char *url_from, const char *url_to, int options, php_stream_context *context);
Expand All @@ -53,7 +54,7 @@ static php_stream *user_wrapper_opendir(php_stream_wrapper *wrapper, const char

static const php_stream_wrapper_ops user_stream_wops = {
user_wrapper_opener,
NULL, /* close - the streams themselves know how */
user_wrapper_close,
NULL, /* stat - the streams themselves know how */
user_wrapper_stat_url,
user_wrapper_opendir,
Expand Down Expand Up @@ -375,6 +376,8 @@ static php_stream *user_wrapper_opener(php_stream_wrapper *wrapper, const char *

/* set wrapper data to be a reference to our object */
ZVAL_COPY(&stream->wrapperdata, &us->object);

GC_ADDREF(us->wrapper->resource);
} else {
php_stream_wrapper_log_error(wrapper, options, "\"%s::" USERSTREAM_OPEN "\" call failed",
ZSTR_VAL(us->wrapper->ce->name));
Expand All @@ -399,6 +402,14 @@ static php_stream *user_wrapper_opener(php_stream_wrapper *wrapper, const char *
return stream;
}

static int user_wrapper_close(php_stream_wrapper *wrapper, php_stream *stream)
{
struct php_user_stream_wrapper *uwrap = (struct php_user_stream_wrapper*)wrapper->abstract;
zend_list_delete(uwrap->resource);
// FIXME: Unused?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be kept?

return 0;
}

static php_stream *user_wrapper_opendir(php_stream_wrapper *wrapper, const char *filename, const char *mode,
int options, zend_string **opened_path, php_stream_context *context STREAMS_DC)
{
Expand Down Expand Up @@ -440,6 +451,8 @@ static php_stream *user_wrapper_opendir(php_stream_wrapper *wrapper, const char

/* set wrapper data to be a reference to our object */
ZVAL_COPY(&stream->wrapperdata, &us->object);

GC_ADDREF(us->wrapper->resource);
} else {
php_stream_wrapper_log_error(wrapper, options, "\"%s::" USERSTREAM_DIR_OPEN "\" call failed",
ZSTR_VAL(us->wrapper->ce->name));
Expand Down