diff --git a/NEWS b/NEWS index d2bc197f0dfba..dccc80a9ec8b4 100644 --- a/NEWS +++ b/NEWS @@ -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). diff --git a/ext/standard/tests/filters/gh20370.phpt b/ext/standard/tests/filters/gh20370.phpt new file mode 100644 index 0000000000000..abcf49bfc9f4c --- /dev/null +++ b/ext/standard/tests/filters/gh20370.phpt @@ -0,0 +1,44 @@ +--TEST-- +GH-20370 (User filters should respect typed properties) +--FILE-- +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 diff --git a/ext/standard/tests/filters/gh20370_dynamic_stream_property.phpt b/ext/standard/tests/filters/gh20370_dynamic_stream_property.phpt new file mode 100644 index 0000000000000..97f24b854c5ea --- /dev/null +++ b/ext/standard/tests/filters/gh20370_dynamic_stream_property.phpt @@ -0,0 +1,51 @@ +--TEST-- +GH-20370 (User filters should update dynamic stream property if it exists) +--FILE-- +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) diff --git a/ext/standard/tests/filters/gh20370_no_stream_property.phpt b/ext/standard/tests/filters/gh20370_no_stream_property.phpt new file mode 100644 index 0000000000000..1b52c4c4155b8 --- /dev/null +++ b/ext/standard/tests/filters/gh20370_no_stream_property.phpt @@ -0,0 +1,37 @@ +--TEST-- +GH-20370 (User filters should not create stream property if not declared) +--FILE-- +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) diff --git a/ext/standard/user_filters.c b/ext/standard/user_filters.c index 962eaaba7d6b0..2518644299b4e 100644 --- a/ext/standard/user_filters.c +++ b/ext/standard/user_filters.c @@ -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" #define PHP_STREAM_BRIGADE_RES_NAME "userfilter.bucket brigade" #define PHP_STREAM_BUCKET_RES_NAME "userfilter.bucket" @@ -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)) { + zval stream_zval; + 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; + 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); @@ -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]);