MFH: Fixed error conditions handling in stream_filter_append()

diff --git a/NEWS b/NEWS
index 1f12f516f34d9f42e7658dbfa13b4ec553c53343..8dbb52421844556b87cc597a2b6d9d549c0f00d3 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,7 @@ PHP                                                                        NEWS
 - Added optional sorting type flag parameter to array_unique(). Default is
   SORT_REGULAR. (Andrei)
 
+- Fixed error conditions handling in stream_filter_append(). (Arnaud)
 - Fixed zip filename property read. (Pierre)
 - Fixed explode() behavior with empty string to respect negative limit. (Shire)
 - Fixed security issue in imagerotate(), background colour isn't validated

diff --git a/ext/standard/streamsfuncs.c b/ext/standard/streamsfuncs.c
index f877aff65ef0e354336b4dbf63b6a8529ad3ec58..cd751618ccf46443f2044b8da13fb5bbb559c2b1 100644 (file)
--- a/ext/standard/streamsfuncs.c
+++ b/ext/standard/streamsfuncs.c
@@ -1083,6 +1083,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) {
@@ -1112,9 +1113,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;
                }
        }
 
@@ -1125,9 +1130,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;
                }
        }
 

diff --git a/ext/standard/tests/filters/filter_errors.inc b/ext/standard/tests/filters/filter_errors.inc
new file mode 100644 (file)
index 0000000..d39eea3
--- /dev/null
+++ b/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);
+
+}
+

diff --git a/ext/standard/tests/filters/filter_errors_convert_base64_decode.phpt b/ext/standard/tests/filters/filter_errors_convert_base64_decode.phpt
new file mode 100644 (file)
index 0000000..39fb28b
--- /dev/null
+++ b/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

diff --git a/ext/standard/tests/filters/filter_errors_user.phpt b/ext/standard/tests/filters/filter_errors_user.phpt
new file mode 100644 (file)
index 0000000..edf2e7c
--- /dev/null
+++ b/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

diff --git a/ext/standard/tests/filters/filter_errors_zlib_inflate.phpt b/ext/standard/tests/filters/filter_errors_zlib_inflate.phpt
new file mode 100644 (file)
index 0000000..ebb3b21
--- /dev/null
+++ b/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

diff --git a/main/streams/filter.c b/main/streams/filter.c
index cd7748b0c470a68afb20f39bc56f1d1ced948144..5f710f6ad8a7ff8861318cf8816538e60055d947 100644 (file)
--- a/main/streams/filter.c
+++ b/main/streams/filter.c
@@ -313,7 +313,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;
@@ -325,9 +325,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;
 
@@ -349,7 +356,7 @@ PHPAPI void _php_stream_filter_append(php_stream_filter_chain *chain, php_stream
                php_stream_bucket *bucket;
                size_t consumed = 0;
 
-               bucket = php_stream_bucket_new(stream, stream->readbuf + stream->readpos, stream->writepos - stream->readpos, 0, 0 TSRMLS_CC);
+               bucket = php_stream_bucket_new(stream, (char*) stream->readbuf + stream->readpos, stream->writepos - stream->readpos, 0, 0 TSRMLS_CC);
                php_stream_bucket_append(brig_inp, bucket TSRMLS_CC);
                status = filter->fops->filter(stream, filter, brig_inp, brig_outp, &consumed, PSFS_FLAG_NORMAL TSRMLS_CC);
 
@@ -360,19 +367,18 @@ PHPAPI void _php_stream_filter_append(php_stream_filter_chain *chain, php_stream
 
                switch (status) {
                        case 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.");
-                               break;
+                               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;
                        case PSFS_FEED_ME:
                                /* We don't actually need data yet,
                                   leave this filter in a feed me state until data is needed. 
@@ -406,6 +412,20 @@ PHPAPI void _php_stream_filter_append(php_stream_filter_chain *chain, php_stream
                }
        }
 
+       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_flush(php_stream_filter *filter, int finish TSRMLS_DC)

diff --git a/main/streams/php_stream_filter_api.h b/main/streams/php_stream_filter_api.h
index c20dcda409d30add113e6d2a43b1a13cf2b9847d..e2cffaf1a7f41f9df171d983612f66789c95a50c 100644 (file)
--- a/main/streams/php_stream_filter_api.h
+++ b/main/streams/php_stream_filter_api.h
@@ -125,7 +125,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_flush(php_stream_filter *filter, int finish TSRMLS_DC);
 PHPAPI php_stream_filter *php_stream_filter_remove(php_stream_filter *filter, int call_dtor TSRMLS_DC);
 PHPAPI void php_stream_filter_free(php_stream_filter *filter TSRMLS_DC);