Fixed error conditions handling in stream_filter_append()

ext/standard/streamsfuncs.c

@@ -1261,6 +1261,7 @@ static void apply_filter_to_stream(int append, INTERNAL_FUNCTION_PARAMETERS)
 	long read_write = 0;
 	zval *filterparams = NULL;
 	php_stream_filter *filter = NULL;
+	int ret;
 
 	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rs|lz", &zstream,
 				&filtername, &filternamelen, &read_write, &filterparams) == FAILURE) {
@@ -1290,9 +1291,13 @@ static void apply_filter_to_stream(int append, INTERNAL_FUNCTION_PARAMETERS)
 		}
 
 		if (append) {
-			php_stream_filter_append(&stream->readfilters, filter);
+			ret = php_stream_filter_append_ex(&stream->readfilters, filter TSRMLS_CC);
 		} else {
-			php_stream_filter_prepend(&stream->readfilters, filter);
+			ret = php_stream_filter_prepend_ex(&stream->readfilters, filter TSRMLS_CC);
+		}
+		if (ret != SUCCESS) {
+			php_stream_filter_remove(filter, 1 TSRMLS_CC);
+			RETURN_FALSE;
 		}
 		if (FAILURE == php_stream_filter_check_chain(&stream->readfilters)) {
 			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Readfilter chain unstable -- unresolvable unicode/string conversion conflict");
@@ -1306,9 +1311,13 @@ static void apply_filter_to_stream(int append, INTERNAL_FUNCTION_PARAMETERS)
 		}
 
 		if (append) { 
-			php_stream_filter_append(&stream->writefilters, filter);
+			ret = php_stream_filter_append_ex(&stream->writefilters, filter TSRMLS_CC);
 		} else {
-			php_stream_filter_prepend(&stream->writefilters, filter);
+			ret = php_stream_filter_prepend_ex(&stream->writefilters, filter TSRMLS_CC);
+		}
+		if (ret != SUCCESS) {
+			php_stream_filter_remove(filter, 1 TSRMLS_CC);
+			RETURN_FALSE;
 		}
 		if (FAILURE == php_stream_filter_check_chain(&stream->writefilters)) {
 			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Writefilter chain unstable -- unresolvable unicode/string conversion conflict");

ext/standard/tests/filters/filter_errors.inc

@@ -0,0 +1,34 @@
+<?php
+
+function filter_errors_skipif($needle) {
+	$filters = stream_get_filters();
+	foreach($filters as $filter) {
+		if (fnmatch($filter, $needle)) return;
+	}
+	die("skip $needle not available");
+}
+
+function filter_errors_test($filter, $data) {
+
+	echo "test filtering of buffered data\n";
+
+	$stream = fopen('php://memory', 'wb+');
+
+	fwrite($stream, b".\r\n$data");
+	fseek($stream, 0, SEEK_SET);
+	stream_get_line($stream, 8192, "\r\n");
+
+	$f = stream_filter_append($stream, $filter);
+
+	echo "test filtering of non buffered data\n";
+
+	$stream = fopen('php://memory', 'wb+');
+
+	fwrite($stream, b"$data");
+	fseek($stream, 0, SEEK_SET);
+
+	stream_get_line($stream, 8192, "\r\n");
+	stream_get_contents($stream);
+
+}
+

ext/standard/tests/filters/filter_errors_convert_base64_decode.phpt

@@ -0,0 +1,16 @@
+--TEST--
+Filter errors: convert.base64-decode
+--SKIPIF--
+<?php require 'filter_errors.inc'; filter_errors_skipif('convert.base64-decode'); ?>
+--FILE--
+<?php
+require 'filter_errors.inc';
+filter_errors_test('convert.base64-decode', '===');
+?>
+--EXPECTF--
+test filtering of buffered data
+
+Warning: stream_filter_append(): stream filter (convert.base64-decode): invalid byte sequence in %s
+
+Warning: stream_filter_append(): Filter failed to process pre-buffered data in %s
+test filtering of non buffered data

ext/standard/tests/filters/filter_errors_user.phpt

@@ -0,0 +1,84 @@
+--TEST--
+Filter errors: user filter
+--FILE--
+<?php
+require 'filter_errors.inc';
+
+class test_filter0 extends php_user_filter {
+	function filter($in, $out, &$consumed, $closing) {
+		return PSFS_ERR_FATAL;
+	}
+}
+class test_filter1 extends php_user_filter {
+	function filter($in, $out, &$consumed, $closing) {
+		$bucket = stream_bucket_make_writeable($in);
+		return PSFS_ERR_FATAL;
+	}
+}
+class test_filter2 extends php_user_filter {
+	function filter($in, $out, &$consumed, $closing) {
+		while ($bucket = stream_bucket_make_writeable($in)) {
+			$consumed += $bucket->datalen;
+			stream_bucket_append($out, $bucket);
+		}
+		return PSFS_ERR_FATAL;
+	}
+}
+class test_filter3 extends php_user_filter {
+	function filter($in, $out, &$consumed, $closing) {
+		$bucket = stream_bucket_new($this->stream, "42");
+		stream_bucket_append($out, $bucket);
+		return PSFS_ERR_FATAL;
+	}
+}
+class test_filter4 extends php_user_filter {
+	function filter($in, $out, &$consumed, $closing) {
+		$bucket = stream_bucket_new($this->stream, "42");
+		return PSFS_ERR_FATAL;
+	}
+}
+
+for($i = 0; $i < 5; ++$i) {
+	echo "test_filter$i\n";
+	var_dump(stream_filter_register("test_filter$i", "test_filter$i"));
+	filter_errors_test("test_filter$i", "42");
+}
+
+?>
+--EXPECTF--
+test_filter0
+bool(true)
+test filtering of buffered data
+
+Warning: stream_filter_append(): Unprocessed filter buckets remaining on input brigade in %s
+
+Warning: stream_filter_append(): Filter failed to process pre-buffered data in %s
+test filtering of non buffered data
+test_filter1
+bool(true)
+test filtering of buffered data
+
+Warning: stream_filter_append(): Filter failed to process pre-buffered data in %s
+test filtering of non buffered data
+test_filter2
+bool(true)
+test filtering of buffered data
+
+Warning: stream_filter_append(): Filter failed to process pre-buffered data in %s
+test filtering of non buffered data
+test_filter3
+bool(true)
+test filtering of buffered data
+
+Warning: stream_filter_append(): Unprocessed filter buckets remaining on input brigade in %s
+
+Warning: stream_filter_append(): Filter failed to process pre-buffered data in %s
+test filtering of non buffered data
+test_filter4
+bool(true)
+test filtering of buffered data
+
+Warning: stream_filter_append(): Unprocessed filter buckets remaining on input brigade in %s
+
+Warning: stream_filter_append(): Filter failed to process pre-buffered data in %s
+test filtering of non buffered data

ext/standard/tests/filters/filter_errors_zlib_inflate.phpt

@@ -0,0 +1,14 @@
+--TEST--
+Filter errors: zlib.inflate
+--SKIPIF--
+<?php require 'filter_errors.inc'; filter_errors_skipif('zlib.inflate'); ?>
+--FILE--
+<?php
+require 'filter_errors.inc';
+filter_errors_test('zlib.inflate', gzencode(b'42'));
+?>
+--EXPECTF--
+test filtering of buffered data
+
+Warning: stream_filter_append(): Filter failed to process pre-buffered data in %s
+test filtering of non buffered data

main/streams/filter.c

@@ -376,7 +376,7 @@ PHPAPI void php_stream_filter_free(php_stream_filter *filter TSRMLS_DC)
 	pefree(filter, filter->is_persistent);
 }
 
-PHPAPI void _php_stream_filter_prepend(php_stream_filter_chain *chain, php_stream_filter *filter TSRMLS_DC)
+PHPAPI int php_stream_filter_prepend_ex(php_stream_filter_chain *chain, php_stream_filter *filter TSRMLS_DC)
 {
 	filter->next = chain->head;
 	filter->prev = NULL;
@@ -388,9 +388,16 @@ PHPAPI void _php_stream_filter_prepend(php_stream_filter_chain *chain, php_strea
 	}
 	chain->head = filter;
 	filter->chain = chain;
+
+	return SUCCESS;
 }
 
-PHPAPI void _php_stream_filter_append(php_stream_filter_chain *chain, php_stream_filter *filter TSRMLS_DC)
+PHPAPI void _php_stream_filter_prepend(php_stream_filter_chain *chain, php_stream_filter *filter TSRMLS_DC)
+{
+	php_stream_filter_prepend_ex(chain, filter TSRMLS_CC);
+}
+
+PHPAPI int php_stream_filter_append_ex(php_stream_filter_chain *chain, php_stream_filter *filter TSRMLS_DC)
 {
 	php_stream *stream = chain->stream;
 
@@ -428,18 +435,18 @@ PHPAPI void _php_stream_filter_append(php_stream_filter_chain *chain, php_stream
 		}
 
 		if (status == PSFS_ERR_FATAL) {
-			/* If this first cycle simply fails then there's something wrong with the filter.
-			   Pull the filter off the chain and leave the read buffer alone. */
-			if (chain->head == filter) {
-				chain->head = NULL;
-				chain->tail = NULL;
-			} else {
-				filter->prev->next = NULL;
-				chain->tail = filter->prev;
+			while (brig_in.head) {
+				bucket = brig_in.head;
+				php_stream_bucket_unlink(bucket TSRMLS_CC);
+				php_stream_bucket_delref(bucket TSRMLS_CC);
 			}
-			php_stream_bucket_unlink(bucket TSRMLS_CC);
-			php_stream_bucket_delref(bucket TSRMLS_CC);
-			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Filter failed to process pre-buffered data.  Not adding to filterchain");
+			while (brig_out.head) {
+				bucket = brig_out.head;
+				php_stream_bucket_unlink(bucket TSRMLS_CC);
+				php_stream_bucket_delref(bucket TSRMLS_CC);
+			}
+			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Filter failed to process pre-buffered data");
+			return FAILURE;
 		} else {
 			/* This filter addition may change the readbuffer type.
 			   Since all the previously held data is in the bucket brigade,
@@ -498,6 +505,21 @@ PHPAPI void _php_stream_filter_append(php_stream_filter_chain *chain, php_stream
 			}
 		}
 	} /* end of readfilters specific code */
+
+	return SUCCESS;
+}
+
+PHPAPI void _php_stream_filter_append(php_stream_filter_chain *chain, php_stream_filter *filter TSRMLS_DC)
+{
+	if (php_stream_filter_append_ex(chain, filter TSRMLS_CC) != SUCCESS) {
+		if (chain->head == filter) {
+			chain->head = NULL;
+			chain->tail = NULL;
+		} else {
+			filter->prev->next = NULL;
+			chain->tail = filter->prev;
+		}
+	}
 }
 
 PHPAPI int _php_stream_filter_check_chain(php_stream_filter_chain *chain TSRMLS_DC)

main/streams/php_stream_filter_api.h

@@ -152,7 +152,9 @@ struct _php_stream_filter {
 /* stack filter onto a stream */
 BEGIN_EXTERN_C()
 PHPAPI void _php_stream_filter_prepend(php_stream_filter_chain *chain, php_stream_filter *filter TSRMLS_DC);
+PHPAPI int php_stream_filter_prepend_ex(php_stream_filter_chain *chain, php_stream_filter *filter TSRMLS_DC);
 PHPAPI void _php_stream_filter_append(php_stream_filter_chain *chain, php_stream_filter *filter TSRMLS_DC);
+PHPAPI int php_stream_filter_append_ex(php_stream_filter_chain *chain, php_stream_filter *filter TSRMLS_DC);
 PHPAPI int _php_stream_filter_check_chain(php_stream_filter_chain *chain TSRMLS_DC);
 PHPAPI int _php_stream_filter_output_prefer_unicode(php_stream_filter *filter TSRMLS_DC);
 PHPAPI int _php_stream_filter_product(php_stream_filter_chain *chain, int type TSRMLS_DC);