Fix GH-13264: Part 1 - Memory leak on filter failure

Closes GH-13790

Author: bukka

NEWS

@@ -6,6 +6,10 @@ PHP                                                                        NEWS
   . Fixed bug GH-13563 (Setting bool values via env in FPM config fails).
     (Jakub Zelenka)
 
+- Streams:
+  . Fixed bug GH-13264 (Part 1 - Memory leak on stream filter failure).
+    (Jakub Zelenka)
+
 11 Apr 2024, PHP 8.2.18
 
 - Core:

ext/standard/tests/filters/gh13264.phpt

@@ -0,0 +1,49 @@
+--TEST--
+GH-81475: Memory leak during stream filter failure
+--SKIPIF--
+<?php require 'filter_errors.inc'; filter_errors_skipif('zlib.inflate'); ?>
+--FILE--
+<?php
+// Prepare a big enough input so that it is not entirely buffered
+$stream = fopen('php://memory', 'r+');
+$content = '';
+for ($i = 0; $i < 10000; $i++) {
+	$content .= "Hello $i\n";
+}
+fwrite($stream, gzcompress($content));
+
+// Mess up the checksum
+fseek($stream, -1, SEEK_CUR);
+fwrite($stream, '1');
+
+// Rewind and add the zlib filter
+rewind($stream);
+stream_filter_append($stream, 'zlib.inflate', STREAM_FILTER_READ, ['window' => 15]);
+
+// Read the filtered stream line by line.
+while (($line = fgets($stream)) !== false) {
+	$error = error_get_last();
+	if ($error !== null) {
+		// An error is thrown but fgets didn't return false
+		var_dump(error_get_last());
+		var_dump($line);
+	}
+}
+
+fclose($stream);
+?>
+--EXPECTF--
+
+Notice: fgets(): zlib: data error in %s on line %d
+array(4) {
+  ["type"]=>
+  int(8)
+  ["message"]=>
+  string(25) "fgets(): zlib: data error"
+  ["file"]=>
+  string(%d) "%s"
+  ["line"]=>
+  int(%d)
+}
+string(7) "Hello 6"
+

ext/standard/user_filters.c

@@ -196,22 +196,7 @@ php_stream_filter_status_t userfilter_filter(
 	}
 
 	if (buckets_in->head) {
-		php_stream_bucket *bucket;
-
 		php_error_docref(NULL, E_WARNING, "Unprocessed filter buckets remaining on input brigade");
-		while ((bucket = buckets_in->head)) {
-			/* Remove unconsumed buckets from the brigade */
-			php_stream_bucket_unlink(bucket);
-			php_stream_bucket_delref(bucket);
-		}
-	}
-	if (ret != PSFS_PASS_ON) {
-		php_stream_bucket *bucket = buckets_out->head;
-		while (bucket != NULL) {
-			php_stream_bucket_unlink(bucket);
-			php_stream_bucket_delref(bucket);
-			bucket = buckets_out->head;
-		}
 	}
 
 	/* filter resources are cleaned up by the stream destructor,

main/streams/streams.c

@@ -632,6 +632,17 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size)
 					/* some fatal error. Theoretically, the stream is borked, so all
 					 * further reads should fail. */
 					stream->eof = 1;
+					/* free all data left in brigades */
+					while ((bucket = brig_inp->head)) {
+						/* Remove unconsumed buckets from the input brigade */
+						php_stream_bucket_unlink(bucket);
+						php_stream_bucket_delref(bucket);
+					}
+					while ((bucket = brig_outp->head)) {
+						/* Remove unconsumed buckets from the output brigade */
+						php_stream_bucket_unlink(bucket);
+						php_stream_bucket_delref(bucket);
+					}
 					efree(chunk_buf);
 					return FAILURE;
 			}