Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
2 changes: 2 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ PHP NEWS
- Streams:
. Fixed bug GH-19798: XP_SOCKET XP_SSL (Socket stream modules): Incorrect
condition for Win32/Win64. (Jakub Zelenka)
. Fixed bug GH-20370 (User stream filters could violate typed property
constraints). (alexandre-daubois)

- Tidy:
. Fixed GH-19021 (improved tidyOptGetCategory detection).
Expand Down
44 changes: 44 additions & 0 deletions ext/standard/tests/filters/gh20370.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
--TEST--
GH-20370 (User filters should respect typed properties)
--FILE--
<?php

class pass_filter
{
public $filtername;
public $params;
public int $stream = 1;

function filter($in, $out, &$consumed, $closing): int
{
while ($bucket = stream_bucket_make_writeable($in)) {
$consumed += $bucket->datalen;
stream_bucket_append($out, $bucket);
}
return PSFS_PASS_ON;
}
}

stream_filter_register("pass", "pass_filter");
$fp = fopen("php://memory", "w");
stream_filter_append($fp, "pass");

try {
fwrite($fp, "data");
} catch (TypeError $e) {
echo $e::class, ": ", $e->getMessage(), "\n";
}

try {
fclose($fp);
} catch (TypeError $e) {
echo $e::class, ": ", $e->getMessage(), "\n";
}

unset($fp); // prevent cleanup at shutdown

?>
--EXPECTF--
Warning: fwrite(): Unprocessed filter buckets remaining on input brigade in %s on line %d
TypeError: Cannot assign resource to property pass_filter::$stream of type int
TypeError: Cannot assign resource to property pass_filter::$stream of type int
51 changes: 51 additions & 0 deletions ext/standard/tests/filters/gh20370_dynamic_stream_property.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
--TEST--
GH-20370 (User filters should update dynamic stream property if it exists)
--FILE--
<?php

#[\AllowDynamicProperties]
class pass_filter
{
public $filtername;
public $params;

function onCreate(): bool
{
$this->stream = null;
return true;
}

function filter($in, $out, &$consumed, $closing): int
{
while ($bucket = stream_bucket_make_writeable($in)) {
$consumed += $bucket->datalen;
stream_bucket_append($out, $bucket);
}
var_dump(property_exists($this, 'stream'));
if (is_resource($this->stream)) {
var_dump(get_resource_type($this->stream));
}
return PSFS_PASS_ON;
}
}

stream_filter_register("pass", "pass_filter");
$fp = fopen("php://memory", "w");
stream_filter_append($fp, "pass");

fwrite($fp, "data");
rewind($fp);
echo fread($fp, 1024) . "\n";

?>
--EXPECTF--
bool(true)
string(6) "stream"
bool(true)
string(6) "stream"
bool(true)
string(6) "stream"
bool(true)
string(6) "stream"
data
bool(true)
37 changes: 37 additions & 0 deletions ext/standard/tests/filters/gh20370_no_stream_property.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
--TEST--
GH-20370 (User filters should not create stream property if not declared)
--FILE--
<?php

class pass_filter
{
public $filtername;
public $params;

function filter($in, $out, &$consumed, $closing): int
{
while ($bucket = stream_bucket_make_writeable($in)) {
$consumed += $bucket->datalen;
stream_bucket_append($out, $bucket);
}

var_dump(property_exists($this, 'stream'));
return PSFS_PASS_ON;
}
}

stream_filter_register("pass", "pass_filter");
$fp = fopen("php://memory", "w");
stream_filter_append($fp, "pass");
fwrite($fp, "data");
rewind($fp);
echo fread($fp, 1024) . "\n";

?>
--EXPECT--
bool(false)
bool(false)
bool(false)
bool(false)
data
bool(false)
43 changes: 36 additions & 7 deletions ext/standard/user_filters.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "ext/standard/basic_functions.h"
#include "ext/standard/file.h"
#include "ext/standard/user_filters_arginfo.h"
#include "zend_exceptions.h"
Copy link
Member

Choose a reason for hiding this comment

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

Why did you need this include?


#define PHP_STREAM_BRIGADE_RES_NAME "userfilter.bucket brigade"
#define PHP_STREAM_BUCKET_RES_NAME "userfilter.bucket"
Expand Down Expand Up @@ -147,13 +148,30 @@ php_stream_filter_status_t userfilter_filter(
uint32_t orig_no_fclose = stream->flags & PHP_STREAM_FLAG_NO_FCLOSE;
stream->flags |= PHP_STREAM_FLAG_NO_FCLOSE;

zval *stream_prop = zend_hash_str_find_ind(Z_OBJPROP_P(obj), "stream", sizeof("stream")-1);
if (stream_prop) {
/* Give the userfilter class a hook back to the stream */
zval_ptr_dtor(stream_prop);
php_stream_to_zval(stream, stream_prop);
Z_ADDREF_P(stream_prop);
/* Give the userfilter class a hook back to the stream */
zend_class_entry *old_scope = EG(fake_scope);
EG(fake_scope) = Z_OBJCE_P(obj);

zend_string *stream_name = ZSTR_INIT_LITERAL("stream", 0);
if (Z_OBJ_HT_P(obj)->has_property(Z_OBJ_P(obj), stream_name, ZEND_PROPERTY_EXISTS, NULL)) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd be tempted to use a cache slot anyway as you repeat the check below, not critical though.
Alternatively: The previous code remembered whether the property existed, perhaps you can do this too, but it's not critical. This alternative idea seems clean as it would simplify the code a bit further.

zval stream_zval;
Copy link
Member

@ndossche ndossche Nov 12, 2025

Choose a reason for hiding this comment

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

A good follow-up for master would be to clean up the duplication of the error branches.
(You don't need to do anything with this comment in this PR, this is just a note for future work)

php_stream_to_zval(stream, &stream_zval);
zend_update_property_ex(Z_OBJCE_P(obj), Z_OBJ_P(obj), stream_name, &stream_zval);
/* If property update threw an exception, skip filter execution */
if (EG(exception)) {
if (buckets_in->head) {
php_error_docref(NULL, E_WARNING, "Unprocessed filter buckets remaining on input brigade");
}
zend_string_release(stream_name);
EG(fake_scope) = old_scope;
Copy link
Member

Choose a reason for hiding this comment

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

I think you need to restore the fake scope prior to emitting the warning such that potential error handlers can't utilise the fake scope (not sure).

stream->flags &= ~PHP_STREAM_FLAG_NO_FCLOSE;
stream->flags |= orig_no_fclose;
return PSFS_ERR_FATAL;
}
}
zend_string_release(stream_name);

EG(fake_scope) = old_scope;

ZVAL_STRINGL(&func_name, "filter", sizeof("filter")-1);

Expand Down Expand Up @@ -196,8 +214,19 @@ php_stream_filter_status_t userfilter_filter(
/* filter resources are cleaned up by the stream destructor,
* keeping a reference to the stream resource here would prevent it
* from being destroyed properly */
zend_property_info *prop_info = zend_hash_str_find_ptr(&Z_OBJCE_P(obj)->properties_info, "stream", sizeof("stream")-1);
zval *stream_prop = zend_hash_str_find(Z_OBJPROP_P(obj), "stream", sizeof("stream")-1);

if (stream_prop) {
convert_to_null(stream_prop);
if (prop_info) {
/* Declared property: set to UNDEF to make it uninitialized */
zval_ptr_dtor(stream_prop);
ZVAL_UNDEF(stream_prop);
} else {
/* Dynamic property: set to null */
zval_ptr_dtor(stream_prop);
ZVAL_NULL(stream_prop);
}
}

zval_ptr_dtor(&args[3]);
Expand Down
Loading