From b512bab367e20327594f31eb7dd6bb08d2520579 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Thu, 8 Jan 2009 17:03:42 +0000 Subject: [PATCH] MFH: Fixed error conditions handling in stream_filter_append() --- NEWS | 1 + ext/standard/streamsfuncs.c | 17 +++- ext/standard/tests/filters/filter_errors.inc | 34 ++++++++ .../filter_errors_convert_base64_decode.phpt | 16 ++++ .../tests/filters/filter_errors_user.phpt | 84 +++++++++++++++++++ .../filters/filter_errors_zlib_inflate.phpt | 14 ++++ main/streams/filter.c | 50 +++++++---- main/streams/php_stream_filter_api.h | 2 + 8 files changed, 199 insertions(+), 19 deletions(-) create mode 100644 ext/standard/tests/filters/filter_errors.inc create mode 100644 ext/standard/tests/filters/filter_errors_convert_base64_decode.phpt create mode 100644 ext/standard/tests/filters/filter_errors_user.phpt create mode 100644 ext/standard/tests/filters/filter_errors_zlib_inflate.phpt diff --git a/NEWS b/NEWS index 1f12f516f3..8dbb524218 100644 --- 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 f877aff65e..cd751618cc 100644 --- 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 index 0000000000..d39eea36da --- /dev/null +++ b/ext/standard/tests/filters/filter_errors.inc @@ -0,0 +1,34 @@ + +--FILE-- + +--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 index 0000000000..edf2e7ca85 --- /dev/null +++ b/ext/standard/tests/filters/filter_errors_user.phpt @@ -0,0 +1,84 @@ +--TEST-- +Filter errors: user filter +--FILE-- +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 index 0000000000..ebb3b21df2 --- /dev/null +++ b/ext/standard/tests/filters/filter_errors_zlib_inflate.phpt @@ -0,0 +1,14 @@ +--TEST-- +Filter errors: zlib.inflate +--SKIPIF-- + +--FILE-- + +--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 cd7748b0c4..5f710f6ad8 100644 --- 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 c20dcda409..e2cffaf1a7 100644 --- 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); -- 2.50.1