Report errors from stream read and write operations

The php_stream_read() and php_stream_write() functions now return
an ssize_t value, with negative results indicating failure. Functions
like fread() and fwrite() will return false in that case.

As a special case, EWOULDBLOCK and EAGAIN on non-blocking streams
should not be regarded as error conditions, and be reported as
successful zero-length reads/writes instead. The handling of EINTR
remains unclear and is internally inconsistent (e.g. some code-paths
will automatically retry on EINTR, while some won't).

I'm landing this now to make sure the stream wrapper ops API changes
make it into 7.4 -- however, if the user-facing changes turn out to
be problematic we have the option of clamping negative returns to
zero in php_stream_read() and php_stream_write() to restore the
old behavior in a relatively non-intrusive manner.

4 files changed, 90 insertions, 11 deletions

diff --git a/ext/zlib/tests/bug71417.phpt b/ext/zlib/tests/bug71417.phpt
new file mode 100644
index 0000000000..717b2effe4
--- /dev/null
+++ b/ext/zlib/tests/bug71417.phpt
@@ -0,0 +1,82 @@
+--TEST--
+Bug #71417: fread() does not detect decoding errors from filter zlib.inflate
+--FILE--
+<?php
+
+function test($case) {
+	$plain = "The quick brown fox jumps over the lazy dog.";
+	$fn = "bug71417.gz";
+	$compressed = (string) gzencode($plain);
+	
+    if ($case == 1) {
+        // 1. Set a random byte in the middle of the compressed data.
+        // $ php test-zlib-inflate.php
+        // --> read: string(0) ""
+        // --> read: string(44) "The quick brown fox jumps over the lazx8dog."
+        // $ gzip test-zlib-inflate.gz
+        // gzip: test-zlib-inflate.gz: invalid compressed data--crc error
+        $compressed[strlen($compressed) - 15] = 'X';
+    } else if ($case == 2) {
+        // 2. Truncate the compressed data.
+        // $ php test-zlib-inflate.php
+        // --> read: string(32) "The quick brown fox jumps over t"
+        // $ gzip test-zlib-inflate.gz
+        // gzip: test-zlib-inflate.gz: unexpected end of file
+        $compressed = substr($compressed, 0, strlen($compressed) - 20);
+    } else if ($case == 3) {
+        // 3. Corrupted final CRC.
+        // $ php test-zlib-inflate.php
+        // --> read: string(0) ""
+        // --> read: string(44) "The quick brown fox jumps over the lazy dog."
+        // $ gzip test-zlib-inflate.gz
+        // gzip: test-zlib-inflate.gz: invalid compressed data--crc error
+        $compressed[strlen($compressed)-5] = 'X';
+    } else if ($case == 4) {
+        // 4. Corrupted final length.
+        // $ php test-zlib-inflate.phpread: string(0) ""
+        // read: string(44) "The quick brown fox jumps over the lazy dog."
+        // $ gunzip test-zlib-inflate.gz 
+        // gzip: test-zlib-inflate.gz: invalid compressed data--length error
+        $compressed[strlen($compressed)-2] = 'X';
+    }
+	
+	// The gzdecode() function applied to the corrupted compressed data always
+	// detects the error:
+	// --> gzdecode(): PHP Fatal error:  Uncaught ErrorException: gzdecode(): data error in ...
+	echo "gzdecode(): ", rawurldecode(gzdecode($compressed)), "\n";
+
+	file_put_contents($fn, $compressed);
+	
+	$r = fopen($fn, "r");
+	stream_filter_append($r, 'zlib.inflate', STREAM_FILTER_READ, array('window' => 15+16));
+	while (!feof($r)) {
+		$s = fread($r, 100);
+		echo "read: "; var_dump($s);
+	}
+	fclose($r);
+	unlink($fn);
+}
+
+test(1);
+test(2);
+test(3);
+test(4);
+
+?>
+--EXPECTF--
+gzdecode(): 
+Warning: gzdecode(): data error in %s on line %d
+
+read: bool(false)
+gzdecode(): 
+Warning: gzdecode(): data error in %s on line %d
+
+read: string(32) "The quick brown fox jumps over t"
+gzdecode(): 
+Warning: gzdecode(): data error in %s on line %d
+
+read: bool(false)
+gzdecode(): 
+Warning: gzdecode(): data error in %s on line %d
+
+read: bool(false)
diff --git a/ext/zlib/tests/bug_52944.phpt b/ext/zlib/tests/bug_52944.phpt
index e5c6694158..782e2396a7 100644
--- a/ext/zlib/tests/bug_52944.phpt
+++ b/ext/zlib/tests/bug_52944.phpt
@@ -19,6 +19,6 @@ var_dump(fread($fp,1));
 fclose($fp);
 echo "Done.\n";
 --EXPECT--
-string(0) ""
+bool(false)
 string(0) ""
 Done.
diff --git a/ext/zlib/tests/gzread_variation1.phpt b/ext/zlib/tests/gzread_variation1.phpt
index d544d6d4c2..9cf4b9c739 100644
--- a/ext/zlib/tests/gzread_variation1.phpt
+++ b/ext/zlib/tests/gzread_variation1.phpt
@@ -27,8 +27,8 @@ unlink($filename);
 ?>
 ===DONE===
 --EXPECT--
-string(0) ""
-string(0) ""
-string(0) ""
+bool(false)
+bool(false)
+bool(false)
 Here is the string to be written. 
 ===DONE===
diff --git a/ext/zlib/zlib_fopen_wrapper.c b/ext/zlib/zlib_fopen_wrapper.c
index d600f09a65..514773f2b0 100644
--- a/ext/zlib/zlib_fopen_wrapper.c
+++ b/ext/zlib/zlib_fopen_wrapper.c
@@ -30,7 +30,7 @@ struct php_gz_stream_data_t	{
 	php_stream *stream;
 };
 
-static size_t php_gziop_read(php_stream *stream, char *buf, size_t count)
+static ssize_t php_gziop_read(php_stream *stream, char *buf, size_t count)
 {
 	struct php_gz_stream_data_t *self = (struct php_gz_stream_data_t *) stream->abstract;
 	int read;
@@ -42,18 +42,15 @@ static size_t php_gziop_read(php_stream *stream, char *buf, size_t count)
 		stream->eof = 1;
 	}
 
-	return (size_t)((read < 0) ? 0 : read);
+	return read;
 }
 
-static size_t php_gziop_write(php_stream *stream, const char *buf, size_t count)
+static ssize_t php_gziop_write(php_stream *stream, const char *buf, size_t count)
 {
 	struct php_gz_stream_data_t *self = (struct php_gz_stream_data_t *) stream->abstract;
-	int wrote;
 
 	/* XXX this needs to be looped for the case count > UINT_MAX */
-	wrote = gzwrite(self->gz_file, (char *) buf, count);
-
-	return (size_t)((wrote < 0) ? 0 : wrote);
+	return gzwrite(self->gz_file, (char *) buf, count);
 }
 
 static int php_gziop_seek(php_stream *stream, zend_off_t offset, int whence, zend_off_t *newoffs)