cataphract                               Sun, 22 Jan 2012 20:30:37 +0000

Revision: http://svn.php.net/viewvc?view=revision&revision=322582

Log:
- Further fix for bug #60455 (stream_get_line misbehaves if EOF is not detected
  together with the last read).
- Fixed bug #60817 (stream_get_line() reads from stream even when there is
  already sufficient data buffered). stream_get_line() now behaves more like
  fgets(), as is documented.
#withheld commit to 5.4

Bugs: https://bugs.php.net/60455 (Closed) stream_get_line reports two lines 
instead of one
      https://bugs.php.net/60817 (Assigned) stream_get_line() incorrectly blocks
      
Changed paths:
    U   php/php-src/branches/PHP_5_3/NEWS
    U   php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60455_02.phpt
    U   php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60455_03.phpt
    A   php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60455_04.phpt
    A   php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60817.phpt
    U   php/php-src/branches/PHP_5_3/main/streams/streams.c
    U   php/php-src/trunk/ext/standard/tests/streams/bug60455_02.phpt
    U   php/php-src/trunk/ext/standard/tests/streams/bug60455_03.phpt
    A   php/php-src/trunk/ext/standard/tests/streams/bug60455_04.phpt
    A   php/php-src/trunk/ext/standard/tests/streams/bug60817.phpt
    U   php/php-src/trunk/main/streams/streams.c

Modified: php/php-src/branches/PHP_5_3/NEWS
===================================================================
--- php/php-src/branches/PHP_5_3/NEWS	2012-01-22 20:09:22 UTC (rev 322581)
+++ php/php-src/branches/PHP_5_3/NEWS	2012-01-22 20:30:37 UTC (rev 322582)
@@ -3,12 +3,21 @@
 ?? ?? 2012, PHP 5.3.10

 - Core:
- . fixed bug #60227: header() cannot detect the multi-line header with CR (rui).
+ . Fixed bug #60227 (header() cannot detect the multi-line header with CR).
+   (rui)
+
 - Firebird Database extension (ibase):
- . Fixed bug #60802: ibase_trans() gives segfault when passing params
+ . Fixed bug #60802 (ibase_trans() gives segfault when passing params).

+- Streams:
+ . Further fix for bug #60455 (stream_get_line misbehaves if EOF is not detected
+   together with the last read). (Gustavo)
+ . Fixed bug #60817 (stream_get_line() reads from stream even when there is
+   already sufficient data buffered). stream_get_line() now behaves more like
+   fgets(), as is documented. (Gustavo)
+
 - PHP-FPM SAPI:
- . fixed bug #60811: php-fpm compilation problem (rasmus)
+ . Fixed bug #60811 (php-fpm compilation problem). (rasmus)

 10 Jan 2012, PHP 5.3.9


Modified: php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60455_02.phpt
===================================================================
--- php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60455_02.phpt	2012-01-22 20:09:22 UTC (rev 322581)
+++ php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60455_02.phpt	2012-01-22 20:30:37 UTC (rev 322582)
@@ -28,3 +28,4 @@
 }
 --EXPECT--
 string(1) "a"
+bool(false)

Modified: php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60455_03.phpt
===================================================================
--- php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60455_03.phpt	2012-01-22 20:09:22 UTC (rev 322581)
+++ php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60455_03.phpt	2012-01-22 20:30:37 UTC (rev 322582)
@@ -47,7 +47,9 @@
 --EXPECT--
 string(1) "a"
 string(1) "b"
+bool(false)
 string(1) "a"
 string(0) ""
+bool(false)
 string(1) "a"
 string(0) ""

Added: php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60455_04.phpt
===================================================================
--- php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60455_04.phpt	                        (rev 0)
+++ php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60455_04.phpt	2012-01-22 20:30:37 UTC (rev 322582)
@@ -0,0 +1,32 @@
+--TEST--
+Bug #60455: stream_get_line and 1-line with maxlen size followed by 0-length
+read with EOL indication
+--FILE--
+<?php
+class TestStream {
+	private $s = 0;
+	function stream_open($path, $mode, $options, &$opened_path) {
+	        return true;
+	}
+	function stream_read($count) {
+		if ($this->s++ == 0)
+			return "a\n";
+
+		return "";
+	}
+	function stream_eof() {
+		return $this->s >= 2;
+	}
+
+}
+
+stream_wrapper_register("test", "TestStream");
+
+$f = fopen("test://", "r");
+while (!feof($f)) {
+    $line = stream_get_line($f, 2, "\n");
+    var_dump($line);
+}
+--EXPECT--
+string(1) "a"
+bool(false)


Property changes on: php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60455_04.phpt
___________________________________________________________________
Added: svn:keywords
   + Id Rev Revision
Added: svn:eol-style
   + native

Added: php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60817.phpt
===================================================================
--- php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60817.phpt	                        (rev 0)
+++ php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60817.phpt	2012-01-22 20:30:37 UTC (rev 322582)
@@ -0,0 +1,36 @@
+--TEST--
+Bug #60817: stream_get_line() reads from stream even when there is already sufficient data buffered
+--FILE--
+<?php
+class TestStream { //data, empty data, empty data + eof
+    private $s = 0;
+    function stream_open($path, $mode, $options, &$opened_path) {
+            return true;
+    }
+    function stream_read($count) {
+        echo "Read done\n";
+        if ($this->s++ == 0)
+            return "a\nbb\ncc";
+
+        return "";
+    }
+    function stream_eof() {
+        return $this->s >= 2;
+    }
+
+}
+
+stream_wrapper_register("test", "TestStream");
+
+$f = fopen("test://", "r");
+while (!feof($f)) {
+    $line = stream_get_line($f, 99, "\n");
+    var_dump($line);
+}
+
+--EXPECT--
+Read done
+string(1) "a"
+string(2) "bb"
+Read done
+string(2) "cc"


Property changes on: php/php-src/branches/PHP_5_3/ext/standard/tests/streams/bug60817.phpt
___________________________________________________________________
Added: svn:keywords
   + Id Rev Revision
Added: svn:eol-style
   + native

Modified: php/php-src/branches/PHP_5_3/main/streams/streams.c
===================================================================
--- php/php-src/branches/PHP_5_3/main/streams/streams.c	2012-01-22 20:09:22 UTC (rev 322581)
+++ php/php-src/branches/PHP_5_3/main/streams/streams.c	2012-01-22 20:30:37 UTC (rev 322582)
@@ -899,77 +899,111 @@
 	return bufstart;
 }

+#define STREAM_BUFFERED_AMOUNT(stream) \
+	((size_t)(((stream)->writepos) - (stream)->readpos))
+
+static char *_php_stream_search_delim(php_stream *stream,
+									  size_t maxlen,
+									  size_t skiplen,
+									  char *delim, /* non-empty! */
+									  size_t delim_len TSRMLS_DC)
+{
+	size_t	seek_len;
+
+	/* set the maximum number of bytes we're allowed to read from buffer */
+	seek_len = MIN(STREAM_BUFFERED_AMOUNT(stream), maxlen);
+	if (seek_len <= skiplen) {
+		return NULL;
+	}
+
+	if (delim_len == 1) {
+		return memchr(&stream->readbuf[stream->readpos + skiplen],
+			delim[0], seek_len - skiplen);
+	} else {
+		return php_memnstr((char*)&stream->readbuf[stream->readpos + skiplen],
+				delim, delim_len,
+				(char*)&stream->readbuf[stream->readpos + seek_len]);
+	}
+}
+
 PHPAPI char *php_stream_get_record(php_stream *stream, size_t maxlen, size_t *returned_len, char *delim, size_t delim_len TSRMLS_DC)
 {
-	char *e, *buf;
-	size_t toread, len;
-	int skip = 0;
+	char	*ret_buf,				/* returned buffer */
+			*found_delim = NULL;
+	size_t	buffered_len,
+			tent_ret_len;			/* tentative returned length*/
+	int		has_delim	 = delim_len > 0 && delim[0] != '\0';

-	len = stream->writepos - stream->readpos;
+	if (maxlen == 0) {
+		return NULL;
+	}

-	/* make sure the stream read buffer has maxlen bytes */
-	while (len < maxlen) {
+	if (has_delim) {
+		found_delim = _php_stream_search_delim(
+			stream, maxlen, 0, delim, delim_len TSRMLS_CC);
+	}

-		size_t just_read;
-		toread = MIN(maxlen - len, stream->chunk_size);
+	buffered_len = STREAM_BUFFERED_AMOUNT(stream);
+	/* try to read up to maxlen length bytes while we don't find the delim */
+	while (!found_delim && buffered_len < maxlen) {
+		size_t	just_read,
+				to_read_now;

-		php_stream_fill_read_buffer(stream, len + toread TSRMLS_CC);
+		to_read_now = MIN(maxlen - buffered_len, stream->chunk_size);

-		just_read = (stream->writepos - stream->readpos) - len;
-		len += just_read;
+		php_stream_fill_read_buffer(stream, buffered_len + to_read_now TSRMLS_CC);

+		just_read = STREAM_BUFFERED_AMOUNT(stream) - buffered_len;
+
 		/* Assume the stream is temporarily or permanently out of data */
 		if (just_read == 0) {
 			break;
 		}
+
+		if (has_delim) {
+			/* search for delimiter, but skip buffered_len (the number of bytes
+			 * buffered before this loop iteration), as they have already been
+			 * searched for the delimiter */
+			found_delim = _php_stream_search_delim(
+				stream, maxlen, buffered_len, delim, delim_len TSRMLS_CC);
+			if (found_delim) {
+				break;
+			}
+		}
+		buffered_len += just_read;
 	}

-	if (delim_len == 0 || !delim) {
-		toread = maxlen;
+	if (has_delim && found_delim) {
+		tent_ret_len = found_delim - (char*)&stream->readbuf[stream->readpos];
+	} else if (!has_delim && STREAM_BUFFERED_AMOUNT(stream) >= maxlen) {
+		tent_ret_len = maxlen;
 	} else {
-		size_t seek_len;
-
-		/* set the maximum number of bytes we're allowed to read from buffer */
-		seek_len = stream->writepos - stream->readpos;
-		if (seek_len > maxlen) {
-			seek_len = maxlen;
-		}
-
-		if (delim_len == 1) {
-			e = memchr(stream->readbuf + stream->readpos, *delim, seek_len);
+		/* return with error if the delimiter string (if any) was not found, we
+		 * could not completely fill the read buffer with maxlen bytes and we
+		 * don't know we've reached end of file. Added with non-blocking streams
+		 * in mind, where this situation is frequent */
+		if (STREAM_BUFFERED_AMOUNT(stream) < maxlen && !stream->eof) {
+			return NULL;
+		} else if (STREAM_BUFFERED_AMOUNT(stream) == 0 && stream->eof) {
+			/* refuse to return an empty string just because by accident
+			 * we knew of EOF in a read that returned no data */
+			return NULL;
 		} else {
-			e = php_memnstr(stream->readbuf + stream->readpos, delim, delim_len, (stream->readbuf + stream->readpos + seek_len));
+			tent_ret_len = MIN(STREAM_BUFFERED_AMOUNT(stream), maxlen);
 		}
-
-		if (!e) {
-			/* return with error if the delimiter string was not found, we
-			 * could not completely fill the read buffer with maxlen bytes
-			 * and we don't know we've reached end of file. Added with
-			 * non-blocking streams in mind, where this situation is frequent */
-			if (seek_len < maxlen && !stream->eof) {
-				return NULL;
-			}
-			toread = maxlen;
-		} else {
-			toread = e - (char *) stream->readbuf - stream->readpos;
-			/* we found the delimiter, so advance the read pointer past it */
-			skip = 1;
-		}
 	}

-	if (toread > maxlen && maxlen > 0) {
-		toread = maxlen;
-	}
+	ret_buf = emalloc(tent_ret_len + 1);
+	/* php_stream_read will not call ops->read here because the necessary
+	 * data is guaranteedly buffered */
+	*returned_len = php_stream_read(stream, ret_buf, tent_ret_len);

-	buf = emalloc(toread + 1);
-	*returned_len = php_stream_read(stream, buf, toread);
-
-	if (skip) {
+	if (found_delim) {
 		stream->readpos += delim_len;
 		stream->position += delim_len;
 	}
-	buf[*returned_len] = '\0';
-	return buf;
+	ret_buf[*returned_len] = '\0';
+	return ret_buf;
 }

 /* Writes a buffer directly to a stream, using multiple of the chunk size */

Modified: php/php-src/trunk/ext/standard/tests/streams/bug60455_02.phpt
===================================================================
--- php/php-src/trunk/ext/standard/tests/streams/bug60455_02.phpt	2012-01-22 20:09:22 UTC (rev 322581)
+++ php/php-src/trunk/ext/standard/tests/streams/bug60455_02.phpt	2012-01-22 20:30:37 UTC (rev 322582)
@@ -28,3 +28,4 @@
 }
 --EXPECT--
 string(1) "a"
+bool(false)

Modified: php/php-src/trunk/ext/standard/tests/streams/bug60455_03.phpt
===================================================================
--- php/php-src/trunk/ext/standard/tests/streams/bug60455_03.phpt	2012-01-22 20:09:22 UTC (rev 322581)
+++ php/php-src/trunk/ext/standard/tests/streams/bug60455_03.phpt	2012-01-22 20:30:37 UTC (rev 322582)
@@ -47,7 +47,9 @@
 --EXPECT--
 string(1) "a"
 string(1) "b"
+bool(false)
 string(1) "a"
 string(0) ""
+bool(false)
 string(1) "a"
 string(0) ""

Added: php/php-src/trunk/ext/standard/tests/streams/bug60455_04.phpt
===================================================================
--- php/php-src/trunk/ext/standard/tests/streams/bug60455_04.phpt	                        (rev 0)
+++ php/php-src/trunk/ext/standard/tests/streams/bug60455_04.phpt	2012-01-22 20:30:37 UTC (rev 322582)
@@ -0,0 +1,32 @@
+--TEST--
+Bug #60455: stream_get_line and 1-line with maxlen size followed by 0-length
+read with EOL indication
+--FILE--
+<?php
+class TestStream {
+	private $s = 0;
+	function stream_open($path, $mode, $options, &$opened_path) {
+	        return true;
+	}
+	function stream_read($count) {
+		if ($this->s++ == 0)
+			return "a\n";
+
+		return "";
+	}
+	function stream_eof() {
+		return $this->s >= 2;
+	}
+
+}
+
+stream_wrapper_register("test", "TestStream");
+
+$f = fopen("test://", "r");
+while (!feof($f)) {
+    $line = stream_get_line($f, 2, "\n");
+    var_dump($line);
+}
+--EXPECT--
+string(1) "a"
+bool(false)


Property changes on: php/php-src/trunk/ext/standard/tests/streams/bug60455_04.phpt
___________________________________________________________________
Added: svn:keywords
   + Id Rev Revision
Added: svn:eol-style
   + native

Added: php/php-src/trunk/ext/standard/tests/streams/bug60817.phpt
===================================================================
--- php/php-src/trunk/ext/standard/tests/streams/bug60817.phpt	                        (rev 0)
+++ php/php-src/trunk/ext/standard/tests/streams/bug60817.phpt	2012-01-22 20:30:37 UTC (rev 322582)
@@ -0,0 +1,36 @@
+--TEST--
+Bug #60817: stream_get_line() reads from stream even when there is already sufficient data buffered
+--FILE--
+<?php
+class TestStream { //data, empty data, empty data + eof
+    private $s = 0;
+    function stream_open($path, $mode, $options, &$opened_path) {
+            return true;
+    }
+    function stream_read($count) {
+        echo "Read done\n";
+        if ($this->s++ == 0)
+            return "a\nbb\ncc";
+
+        return "";
+    }
+    function stream_eof() {
+        return $this->s >= 2;
+    }
+
+}
+
+stream_wrapper_register("test", "TestStream");
+
+$f = fopen("test://", "r");
+while (!feof($f)) {
+    $line = stream_get_line($f, 99, "\n");
+    var_dump($line);
+}
+
+--EXPECT--
+Read done
+string(1) "a"
+string(2) "bb"
+Read done
+string(2) "cc"


Property changes on: php/php-src/trunk/ext/standard/tests/streams/bug60817.phpt
___________________________________________________________________
Added: svn:keywords
   + Id Rev Revision
Added: svn:eol-style
   + native

Modified: php/php-src/trunk/main/streams/streams.c
===================================================================
--- php/php-src/trunk/main/streams/streams.c	2012-01-22 20:09:22 UTC (rev 322581)
+++ php/php-src/trunk/main/streams/streams.c	2012-01-22 20:30:37 UTC (rev 322582)
@@ -970,77 +970,111 @@
 	return bufstart;
 }

+#define STREAM_BUFFERED_AMOUNT(stream) \
+	((size_t)(((stream)->writepos) - (stream)->readpos))
+
+static char *_php_stream_search_delim(php_stream *stream,
+									  size_t maxlen,
+									  size_t skiplen,
+									  char *delim, /* non-empty! */
+									  size_t delim_len TSRMLS_DC)
+{
+	size_t	seek_len;
+
+	/* set the maximum number of bytes we're allowed to read from buffer */
+	seek_len = MIN(STREAM_BUFFERED_AMOUNT(stream), maxlen);
+	if (seek_len <= skiplen) {
+		return NULL;
+	}
+
+	if (delim_len == 1) {
+		return memchr(&stream->readbuf[stream->readpos + skiplen],
+			delim[0], seek_len - skiplen);
+	} else {
+		return php_memnstr((char*)&stream->readbuf[stream->readpos + skiplen],
+				delim, delim_len,
+				(char*)&stream->readbuf[stream->readpos + seek_len]);
+	}
+}
+
 PHPAPI char *php_stream_get_record(php_stream *stream, size_t maxlen, size_t *returned_len, char *delim, size_t delim_len TSRMLS_DC)
 {
-	char *e, *buf;
-	size_t toread, len;
-	int skip = 0;
+	char	*ret_buf,				/* returned buffer */
+			*found_delim = NULL;
+	size_t	buffered_len,
+			tent_ret_len;			/* tentative returned length*/
+	int		has_delim	 = delim_len > 0 && delim[0] != '\0';

-	len = stream->writepos - stream->readpos;
+	if (maxlen == 0) {
+		return NULL;
+	}

-	/* make sure the stream read buffer has maxlen bytes */
-	while (len < maxlen) {
+	if (has_delim) {
+		found_delim = _php_stream_search_delim(
+			stream, maxlen, 0, delim, delim_len TSRMLS_CC);
+	}

-		size_t just_read;
-		toread = MIN(maxlen - len, stream->chunk_size);
+	buffered_len = STREAM_BUFFERED_AMOUNT(stream);
+	/* try to read up to maxlen length bytes while we don't find the delim */
+	while (!found_delim && buffered_len < maxlen) {
+		size_t	just_read,
+				to_read_now;

-		php_stream_fill_read_buffer(stream, len + toread TSRMLS_CC);
+		to_read_now = MIN(maxlen - buffered_len, stream->chunk_size);

-		just_read = (stream->writepos - stream->readpos) - len;
-		len += just_read;
+		php_stream_fill_read_buffer(stream, buffered_len + to_read_now TSRMLS_CC);

+		just_read = STREAM_BUFFERED_AMOUNT(stream) - buffered_len;
+
 		/* Assume the stream is temporarily or permanently out of data */
 		if (just_read == 0) {
 			break;
 		}
+
+		if (has_delim) {
+			/* search for delimiter, but skip buffered_len (the number of bytes
+			 * buffered before this loop iteration), as they have already been
+			 * searched for the delimiter */
+			found_delim = _php_stream_search_delim(
+				stream, maxlen, buffered_len, delim, delim_len TSRMLS_CC);
+			if (found_delim) {
+				break;
+			}
+		}
+		buffered_len += just_read;
 	}

-	if (delim_len == 0 || !delim) {
-		toread = maxlen;
+	if (has_delim && found_delim) {
+		tent_ret_len = found_delim - (char*)&stream->readbuf[stream->readpos];
+	} else if (!has_delim && STREAM_BUFFERED_AMOUNT(stream) >= maxlen) {
+		tent_ret_len = maxlen;
 	} else {
-		size_t seek_len;
-
-		/* set the maximum number of bytes we're allowed to read from buffer */
-		seek_len = stream->writepos - stream->readpos;
-		if (seek_len > maxlen) {
-			seek_len = maxlen;
-		}
-
-		if (delim_len == 1) {
-			e = memchr(stream->readbuf + stream->readpos, *delim, seek_len);
+		/* return with error if the delimiter string (if any) was not found, we
+		 * could not completely fill the read buffer with maxlen bytes and we
+		 * don't know we've reached end of file. Added with non-blocking streams
+		 * in mind, where this situation is frequent */
+		if (STREAM_BUFFERED_AMOUNT(stream) < maxlen && !stream->eof) {
+			return NULL;
+		} else if (STREAM_BUFFERED_AMOUNT(stream) == 0 && stream->eof) {
+			/* refuse to return an empty string just because by accident
+			 * we knew of EOF in a read that returned no data */
+			return NULL;
 		} else {
-			e = php_memnstr(stream->readbuf + stream->readpos, delim, delim_len, (stream->readbuf + stream->readpos + seek_len));
+			tent_ret_len = MIN(STREAM_BUFFERED_AMOUNT(stream), maxlen);
 		}
-
-		if (!e) {
-			/* return with error if the delimiter string was not found, we
-			 * could not completely fill the read buffer with maxlen bytes
-			 * and we don't know we've reached end of file. Added with
-			 * non-blocking streams in mind, where this situation is frequent */
-			if (seek_len < maxlen && !stream->eof) {
-				return NULL;
-			}
-			toread = maxlen;
-		} else {
-			toread = e - (char *) stream->readbuf - stream->readpos;
-			/* we found the delimiter, so advance the read pointer past it */
-			skip = 1;
-		}
 	}

-	if (toread > maxlen && maxlen > 0) {
-		toread = maxlen;
-	}
+	ret_buf = emalloc(tent_ret_len + 1);
+	/* php_stream_read will not call ops->read here because the necessary
+	 * data is guaranteedly buffered */
+	*returned_len = php_stream_read(stream, ret_buf, tent_ret_len);

-	buf = emalloc(toread + 1);
-	*returned_len = php_stream_read(stream, buf, toread);
-
-	if (skip) {
+	if (found_delim) {
 		stream->readpos += delim_len;
 		stream->position += delim_len;
 	}
-	buf[*returned_len] = '\0';
-	return buf;
+	ret_buf[*returned_len] = '\0';
+	return ret_buf;
 }

 /* Writes a buffer directly to a stream, using multiple of the chunk size */
-- 
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to