17 years ago by Garrett Serack — view source — reply

unread

I finally got to the bottom of this bug. It was hiding nice and deep.

I thought I'd run this by folks first before committing, as it impacts streams, which are pretty important :D . It passes the current tests, and I wrote test for it too. (below).

Index: main/streams/streams.c

--- main/streams/streams.c (revision 287973)
+++ main/streams/streams.c (working copy)
@@ -901,10 +901,7 @@
}

            if (!e) {

                  if (seek_len < maxlen && !stream->eof) {

```
                          return NULL;
```
```
                  }
```
```
                  toread = maxlen;
```

                  toread = (seek_len < maxlen) ? seek_len : maxlen;
          } else {
                  toread = e - (char *) stream->readbuf - stream->readpos;
                  skip = 1;

=======/ext/standard/tests/streams/bug49148.phpt:=======

--TEST--
Bug #49148 (combination of stream_get_line and fseek does not work correctly)
--FILE--
<?php
// should give back "line 1" and ""
$tmp = tmpfile();
fwrite($tmp, "line1\r\n");
fseek($tmp, 0);

var_dump(stream_get_line($tmp, null, "\r\n"));
var_dump(stream_get_line($tmp, null, "\r\n"));
fclose($tmp);

// should give back "line 1" and ""
$tmp = tmpfile();
fwrite($tmp, "line1\r\n");
fseek($tmp, 0);

var_dump(stream_get_line($tmp, null, "\r\n"));
fseek($tmp, ftell($tmp)); // expected that this does not affect result.
var_dump(stream_get_line($tmp, null, "\r\n"));
fclose($tmp);

// should give back "line 1" and "line 2"
$tmp = tmpfile();
fwrite($tmp, "line1\r\n");
fwrite($tmp, "line2");
fseek($tmp, 0);
var_dump(stream_get_line($tmp, null, "\r\n"));
var_dump(stream_get_line($tmp, null, "\r\n"));
fclose($tmp);

// should give back "line 1" and "line 2"
$tmp = tmpfile();
fwrite($tmp, "line1\r\n");
fwrite($tmp, "line2");
fseek($tmp, 0);
var_dump(stream_get_line($tmp, null, "\r\n"));
fseek($tmp, ftell($tmp)); // expected that this does not affect result.
var_dump(stream_get_line($tmp, null, "\r\n"));
fclose($tmp);

// should give back "line 1" and "line 2"
$tmp = tmpfile();
fwrite($tmp, "line1\r\n");
fwrite($tmp, "line2\r\n");
fseek($tmp, 0);
var_dump(stream_get_line($tmp, null, "\r\n"));
var_dump(stream_get_line($tmp, null, "\r\n"));
fclose($tmp);

// should give back "line 1" and "line 2"
$tmp = tmpfile();
fwrite($tmp, "line1\r\n");
fwrite($tmp, "line2\r\n");
fseek($tmp, 0);
var_dump(stream_get_line($tmp, null, "\r\n"));
fseek($tmp, ftell($tmp)); // expected that this does not affect result.
var_dump(stream_get_line($tmp, null, "\r\n"));
fclose($tmp);

?>
--EXPECT--
string(5) "line1"
string(0) ""
string(5) "line1"
string(0) ""
string(5) "line1"
string(5) "line2"
string(5) "line1"
string(5) "line2"
string(5) "line1"
string(5) "line2"
string(5) "line1"
string(5) "line2"

Garrett

17 years ago by Stanislav Malyshev — view source — reply

unread

Hi!

I thought I'd run this by folks first before committing, as it
impacts streams, which are pretty important :D . It passes the
current tests, and I wrote test for it too. (below).

I notice that the bug says it happens only on Windows, while the fix
doesn't have anything specific for windows. Could you explain why it is so?
Another question is why stream->eof is not set in this case on Windows -
shouldn't it be?

Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

17 years ago by Garrett Serack — view source — reply

unread

it happens on Linux and Windows. I've repro'd it in both places.

The bug submitter hadn't tested elsewhere.

-----Original Message-----
From: Stanislav Malyshev [mailto:stas@zend.com]
Sent: Wednesday, September 02, 2009 2:15 PM
To: Garrett Serack
Cc: 'PHP Internals'
Subject: Re: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

Hi!

I thought I'd run this by folks first before committing, as it
impacts streams, which are pretty important :D . It passes the
current tests, and I wrote test for it too. (below).

I notice that the bug says it happens only on Windows, while the fix
doesn't have anything specific for windows. Could you explain why it is so?
Another question is why stream->eof is not set in this case on Windows -
shouldn't it be?

Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

17 years ago by Stanislav Malyshev — view source — reply

unread

Hi!

it happens on Linux and Windows. I've repro'd it in both places.

The bug submitter hadn't tested elsewhere.

You are right. However, I notice something strange with your patch - if
I call stream_get_line again with the same parameters I get "".
Shouldn't I get false instead, as it's at EOF? `fgets()` returns false.

Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

17 years ago by Garrett Serack — view source — reply

unread

Well, that's a really good question.

Pre-patch, at the end of the stream (when you don't explicitly seek), you get empty strings:

--example.php--
$tmp = tmpfile();
fwrite($tmp, "line1\r\n");
fseek($tmp, 0);

var_dump(stream_get_line($tmp, null, "\r\n"));
var_dump(stream_get_line($tmp, null, "\r\n"));
var_dump(stream_get_line($tmp, null, "\r\n"));
var_dump(stream_get_line($tmp, null, "\r\n"));
var_dump(stream_get_line($tmp, null, "\r\n"));

fclose($tmp);

--results--
string(5) "line1"
string(0) ""
string(0) ""
string(0) ""
string(0) ""

Do you want to change the behavior so that it does return false? It's trivial to do (and I'd agree that it would be correct), but it does change the behavior from the current behavior.

-----Original Message-----
From: Stanislav Malyshev [mailto:stas@zend.com]
Sent: Thursday, September 03, 2009 5:31 PM
To: Garrett Serack
Cc: 'PHP Internals'
Subject: Re: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

Hi!

it happens on Linux and Windows. I've repro'd it in both places.

The bug submitter hadn't tested elsewhere.

You are right. However, I notice something strange with your patch - if
I call stream_get_line again with the same parameters I get "".
Shouldn't I get false instead, as it's at EOF? `fgets()` returns false.

Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

17 years ago by Stanislav Malyshev — view source — reply

unread

Hi!

Well, that's a really good question.

Pre-patch, at the end of the stream (when you don't explicitly seek),
you get empty strings:

I think it's a bug, docs say it's like fgets and fgets returns false on
EOF. Also, "" with this function can mean "delimiter, than another
delimititer" if I understand correctly, because it doesn't return the
delims - which means returning "" on EOF is even worse bug.

Do you want to change the behavior so that it does return false? It's
trivial to do (and I'd agree that it would be correct), but it does
change the behavior from the current behavior.

I think it's a bug and should be changed to return false on EOF, just as
fgets does. Of course, if there's data before EOF it should return it,
and then next read should return false. The case where file ends with
delim should return data before delim on first read, and false on the
next after delim was read.

Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

17 years ago by Garrett Serack — view source — reply

unread

I've updated my patch to return null on EOF.

Index: streams/streams.c

--- streams/streams.c (revision 289073)
+++ streams/streams.c (working copy)
@@ -890,6 +890,9 @@
size_t seek_len;

            seek_len = stream->writepos - stream->readpos;

```
          if(!seek_len)
```
```
                  return FALSE;
```

          if (seek_len > maxlen) {
                  seek_len = maxlen;
          }

@@ -901,10 +904,7 @@
}

            if (!e) {

                  if (seek_len < maxlen && !stream->eof) {

```
                          return NULL;
```
```
                  }
```
```
                  toread = maxlen;
```

                  toread = (seek_len < maxlen) ? seek_len : maxlen;
          } else {
                  toread = e - (char *) stream->readbuf - stream->readpos;
                  skip = 1;

And the test script for it:

--TEST--
Bug #49148 (combination of stream_get_line and fseek does not work correctly)
--FILE--
<?php
// should give back "line 1" , false, false
$tmp = tmpfile();
fwrite($tmp, "line1\r\n");
fseek($tmp, 0);

var_dump(stream_get_line($tmp, null, "\r\n"));
var_dump(stream_get_line($tmp, null, "\r\n"));
var_dump(stream_get_line($tmp, null, "\r\n"));
fclose($tmp);

// should give back "line 1" , false, false
$tmp = tmpfile();
fwrite($tmp, "line1\r\n");
fseek($tmp, 0);

var_dump(stream_get_line($tmp, null, "\r\n"));
fseek($tmp, ftell($tmp)); // expected that this does not affect result.
var_dump(stream_get_line($tmp, null, "\r\n"));
var_dump(stream_get_line($tmp, null, "\r\n"));
fclose($tmp);

// should give back "line 1" , "line 2", false
$tmp = tmpfile();
fwrite($tmp, "line1\r\n");
fwrite($tmp, "line2");
fseek($tmp, 0);
var_dump(stream_get_line($tmp, null, "\r\n"));
var_dump(stream_get_line($tmp, null, "\r\n"));
var_dump(stream_get_line($tmp, null, "\r\n"));
fclose($tmp);

// should give back "line 1" , "line 2", false
$tmp = tmpfile();
fwrite($tmp, "line1\r\n");
fwrite($tmp, "line2");
fseek($tmp, 0);
var_dump(stream_get_line($tmp, null, "\r\n"));
fseek($tmp, ftell($tmp)); // expected that this does not affect result.
var_dump(stream_get_line($tmp, null, "\r\n"));
var_dump(stream_get_line($tmp, null, "\r\n"));
fclose($tmp);

// should give back "line 1" , "line 2", false
$tmp = tmpfile();
fwrite($tmp, "line1\r\n");
fwrite($tmp, "line2\r\n");
fseek($tmp, 0);
var_dump(stream_get_line($tmp, null, "\r\n"));
fseek($tmp, ftell($tmp)); // expected that this does not affect result.
var_dump(stream_get_line($tmp, null, "\r\n"));
var_dump(stream_get_line($tmp, null, "\r\n"));
fclose($tmp);
?>
--EXPECT--
string(5) "line1"
bool(false)
bool(false)
string(5) "line1"
bool(false)
bool(false)
string(5) "line1"
string(5) "line2"
bool(false)
string(5) "line1"
string(5) "line2"
bool(false)
string(5) "line1"
string(5) "line2"
bool(false)
string(5) "line1"
string(5) "line2"
bool(false)

-----Original Message-----
From: Stanislav Malyshev [mailto:stas@zend.com]
Sent: Wednesday, September 09, 2009 2:59 PM
To: Garrett Serack
Cc: 'PHP Internals'
Subject: Re: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

Hi!

Well, that's a really good question.

Pre-patch, at the end of the stream (when you don't explicitly seek),
you get empty strings:

Do you want to change the behavior so that it does return false? It's
trivial to do (and I'd agree that it would be correct), but it does
change the behavior from the current behavior.

I think it's a bug and should be changed to return false on EOF, just as
fgets does. Of course, if there's data before EOF it should return it,
and then next read should return false. The case where file ends with
delim should return data before delim on first read, and false on the
next after delim was read.

Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

16 years ago by Stanislav Malyshev — view source — reply

unread

Hi!

I've updated my patch to return null on EOF.

Unfortunately, on Linux this patch fails test stream_get_line_nb.phpt -
looks like it changes the stream behavior in the non-blocking case.

Index: streams/streams.c

--- streams/streams.c (revision 289073)
+++ streams/streams.c (working copy)
@@ -890,6 +890,9 @@
size_t seek_len;

             seek_len = stream->writepos - stream->readpos;

```
          if(!seek_len)
```
```
                  return FALSE;
```

           if (seek_len>  maxlen) {
                   seek_len = maxlen;
           }

@@ -901,10 +904,7 @@
}

             if (!e) {

                  if (seek_len<  maxlen&&  !stream->eof) {

```
                          return NULL;
```
```
                  }
```
```
                  toread = maxlen;
```

                  toread = (seek_len<  maxlen) ? seek_len : maxlen;
           } else {
                   toread = e - (char *) stream->readbuf - stream->readpos;
                   skip = 1;

--
Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

17 years ago by ahlin.hans@kronan-net.com — view source — reply

unread

I just ran the test on Linux 2.6.29 FC11 Apache 2.2.13 and PHP 5.2.9

Result:

string 'line1' (length=5)
string '' (length=0)
string 'line1' (length=5)
string '' (length=0)
string 'line1' (length=5)
string 'line2' (length=5)
string 'line1' (length=5)
boolean false
string 'line1' (length=5)
string 'line2' (length=5)
string 'line1' (length=5)
string 'line2' (length=5)

--
MvH / Hans Åhlin
Tel: +46761488019
http//www.kronan-net.com/

17 years ago by Garrett Serack — view source — reply

unread

Without the patch, That is what will you see (windows or linux). (Which is wrong...)

The patch corrects the case where the stream has been fseek'd to the last line in the file, when the last line doesn't end with a line terminator. (where you see 'false' in the results).

Without the patch, the code mistakenly checks for EOF after characters have been read in, which is completely unnecessary (and incorrect) since the read operation has taken place already.

There could be some question as to the proper behavior when the stream is at EOF, and stream_get_line is called (return NULL or empty string), but the behavior previously was to return the empty string (keep calling stream_get_line and once you reach the end, you will just get empty strings).

-----Original Message-----
From: ahlin.hans@gmail.com [mailto:ahlin.hans@gmail.com] On Behalf Of Hans Åhlin
Sent: Wednesday, September 02, 2009 10:29 PM
To: internals@lists.php.net
Subject: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

I just ran the test on Linux 2.6.29 FC11 Apache 2.2.13 and PHP 5.2.9

Result:

--
MvH / Hans Åhlin
Tel: +46761488019
http//www.kronan-net.com/

17 years ago by Garrett Serack — view source — reply

unread

Ah, I found another case... when the line length is longer than the read buffer. I'll post again in a few minutes.

-----Original Message-----
From: Garrett Serack [mailto:garretts@microsoft.com]
Sent: Thursday, September 03, 2009 1:22 PM
To: Hans Åhlin; internals@lists.php.net
Subject: RE: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

Without the patch, That is what will you see (windows or linux). (Which is wrong...)

The patch corrects the case where the stream has been fseek'd to the last line in the file, when the last line doesn't end with a line terminator. (where you see 'false' in the results).

Without the patch, the code mistakenly checks for EOF after characters have been read in, which is completely unnecessary (and incorrect) since the read operation has taken place already.

I just ran the test on Linux 2.6.29 FC11 Apache 2.2.13 and PHP 5.2.9

Result:

--
MvH / Hans Åhlin
Tel: +46761488019
http//www.kronan-net.com/

17 years ago by Garrett Serack — view source — reply

unread

Ah, no. That is correctly handled. (the default max line length is 8k), and passing in a larger number works just fine.

G
-----Original Message-----
From: Garrett Serack
Sent: Thursday, September 03, 2009 1:32 PM
To: Garrett Serack; Hans Åhlin; internals@lists.php.net
Subject: RE: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

Ah, I found another case... when the line length is longer than the read buffer. I'll post again in a few minutes.

Without the patch, That is what will you see (windows or linux). (Which is wrong...)

The patch corrects the case where the stream has been fseek'd to the last line in the file, when the last line doesn't end with a line terminator. (where you see 'false' in the results).

Without the patch, the code mistakenly checks for EOF after characters have been read in, which is completely unnecessary (and incorrect) since the read operation has taken place already.

I just ran the test on Linux 2.6.29 FC11 Apache 2.2.13 and PHP 5.2.9

Result:

--
MvH / Hans Åhlin
Tel: +46761488019
http//www.kronan-net.com/

17 years ago by Pierre Joye — view source — reply

unread

hi Garrett,

As the patch looks simple I still like to have a bunch set of other
tests before applying it (especially for 5.2/3). The tests should
ideally use various stream backends (user, bz, etc.).

Cheers,

Ah, no. That is correctly handled. (the default max line length is 8k), and passing in a larger number works just fine.

G
-----Original Message-----
From: Garrett Serack
Sent: Thursday, September 03, 2009 1:32 PM
To: Garrett Serack; Hans Åhlin; internals@lists.php.net
Subject: RE: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

Ah, I found another case... when the line length is longer than the read buffer. I'll post again in a few minutes.

G

-----Original Message-----
From: Garrett Serack [mailto:garretts@microsoft.com]
Sent: Thursday, September 03, 2009 1:22 PM
To: Hans Åhlin; internals@lists.php.net
Subject: RE: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

Without the patch, That is what will you see (windows or linux). (Which is wrong...)

The patch corrects the case where the stream has been fseek'd to the last line in the file, when the last line doesn't end with a line terminator. (where you see 'false' in the results).

Without the patch, the code mistakenly checks for EOF after characters have been read in, which is completely unnecessary (and incorrect) since the read operation has taken place already.

There could be some question as to the proper behavior when the stream is at EOF, and stream_get_line is called (return NULL or empty string), but the behavior previously was to return the empty string (keep calling stream_get_line and once you reach the end, you will just get empty strings).

G

-----Original Message-----
From: ahlin.hans@gmail.com [mailto:ahlin.hans@gmail.com] On Behalf Of Hans Åhlin
Sent: Wednesday, September 02, 2009 10:29 PM
To: internals@lists.php.net
Subject: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

I just ran the test on Linux 2.6.29 FC11 Apache 2.2.13 and PHP 5.2.9

Result:

string 'line1' (length=5)
string '' (length=0)
string 'line1' (length=5)
string '' (length=0)
string 'line1' (length=5)
string 'line2' (length=5)
string 'line1' (length=5)
boolean false
string 'line1' (length=5)
string 'line2' (length=5)
string 'line1' (length=5)
string 'line2' (length=5)

--
MvH / Hans Åhlin
Tel: +46761488019
http//www.kronan-net.com/

--

--
Pierre

http://blog.thepimp.net | http://www.libgd.org

17 years ago by Garrett Serack — view source — reply

unread

I'll have to wait till next week to write those. No Problem.
G

-----Original Message-----
From: Pierre Joye [mailto:pierre.php@gmail.com]
Sent: Thursday, September 03, 2009 2:16 PM
To: Garrett Serack
Cc: Hans Åhlin; internals@lists.php.net
Subject: Re: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

hi Garrett,

As the patch looks simple I still like to have a bunch set of other
tests before applying it (especially for 5.2/3). The tests should
ideally use various stream backends (user, bz, etc.).

Cheers,

Ah, no. That is correctly handled. (the default max line length is 8k), and passing in a larger number works just fine.

G
-----Original Message-----
From: Garrett Serack
Sent: Thursday, September 03, 2009 1:32 PM
To: Garrett Serack; Hans Åhlin; internals@lists.php.net
Subject: RE: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

Ah, I found another case... when the line length is longer than the read buffer. I'll post again in a few minutes.

G

-----Original Message-----
From: Garrett Serack [mailto:garretts@microsoft.com]
Sent: Thursday, September 03, 2009 1:22 PM
To: Hans Åhlin; internals@lists.php.net
Subject: RE: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

Without the patch, That is what will you see (windows or linux). (Which is wrong...)

The patch corrects the case where the stream has been fseek'd to the last line in the file, when the last line doesn't end with a line terminator. (where you see 'false' in the results).

Without the patch, the code mistakenly checks for EOF after characters have been read in, which is completely unnecessary (and incorrect) since the read operation has taken place already.

There could be some question as to the proper behavior when the stream is at EOF, and stream_get_line is called (return NULL or empty string), but the behavior previously was to return the empty string (keep calling stream_get_line and once you reach the end, you will just get empty strings).

G

-----Original Message-----
From: ahlin.hans@gmail.com [mailto:ahlin.hans@gmail.com] On Behalf Of Hans Åhlin
Sent: Wednesday, September 02, 2009 10:29 PM
To: internals@lists.php.net
Subject: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

I just ran the test on Linux 2.6.29 FC11 Apache 2.2.13 and PHP 5.2.9

Result:

string 'line1' (length=5)
string '' (length=0)
string 'line1' (length=5)
string '' (length=0)
string 'line1' (length=5)
string 'line2' (length=5)
string 'line1' (length=5)
boolean false
string 'line1' (length=5)
string 'line2' (length=5)
string 'line1' (length=5)
string 'line2' (length=5)

--
MvH / Hans Åhlin
Tel: +46761488019
http//www.kronan-net.com/

--

--
Pierre

http://blog.thepimp.net | http://www.libgd.org

17 years ago by Garrett Serack — view source — reply

unread

Hmmm.

stream_get_line doesn't appear to work correctly with bz streams:

--example.php--
$tmpname = tempnam("/tmp", "tmp");
$tmp = fopen($tmpname, "w");
stream_filter_append($tmp, 'bzip2.compress', STREAM_FILTER_WRITE);
fwrite($tmp, "line1\n");
fwrite($tmp, "line2\n");
fwrite($tmp, "line3");
fclose($tmp);

// reopen for reading using stream_get_line
$tmp = fopen($tmpname, "r");
stream_filter_prepend($tmp, 'bzip2.decompress', STREAM_FILTER_READ);
var_dump(stream_get_line($tmp, null, "\n"));
var_dump(stream_get_line($tmp, null, "\n"));
var_dump(stream_get_line($tmp, null, "\n"));
var_dump(stream_get_line($tmp, null, "\n"));
fclose($tmp);

// reopen for reading again, this time using `fgets()`
$tmp = fopen($tmpname, "r");
stream_filter_prepend($tmp, 'bzip2.decompress', STREAM_FILTER_READ);
var_dump(fgets($tmp));
var_dump(fgets($tmp));
var_dump(fgets($tmp));
var_dump(fgets($tmp));
fclose($tmp);

--output--
string(5) "line1"
string(0) ""
string(0) ""
string(0) ""
string(6) "line1
"
string(6) "line2
"
string(6) "line2
"
bool(false)

Both pre- and post- patch, the stream_get_line doesn't work right with the bz stream. I get the first read ok, then subsequent ones don't work.

fgets() works perfectly fine however.

Perhaps the whole thing is a tad buggier than it should be.

Any ideas?

G
-----Original Message-----
From: Pierre Joye [mailto:pierre.php@gmail.com]
Sent: Thursday, September 03, 2009 2:16 PM
To: Garrett Serack
Cc: Hans Åhlin; internals@lists.php.net
Subject: Re: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

hi Garrett,

As the patch looks simple I still like to have a bunch set of other
tests before applying it (especially for 5.2/3). The tests should
ideally use various stream backends (user, bz, etc.).

Cheers,

Ah, no. That is correctly handled. (the default max line length is 8k), and passing in a larger number works just fine.

G
-----Original Message-----
From: Garrett Serack
Sent: Thursday, September 03, 2009 1:32 PM
To: Garrett Serack; Hans Åhlin; internals@lists.php.net
Subject: RE: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

Ah, I found another case... when the line length is longer than the read buffer. I'll post again in a few minutes.

G

-----Original Message-----
From: Garrett Serack [mailto:garretts@microsoft.com]
Sent: Thursday, September 03, 2009 1:22 PM
To: Hans Åhlin; internals@lists.php.net
Subject: RE: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

Without the patch, That is what will you see (windows or linux). (Which is wrong...)

The patch corrects the case where the stream has been fseek'd to the last line in the file, when the last line doesn't end with a line terminator. (where you see 'false' in the results).

Without the patch, the code mistakenly checks for EOF after characters have been read in, which is completely unnecessary (and incorrect) since the read operation has taken place already.

There could be some question as to the proper behavior when the stream is at EOF, and stream_get_line is called (return NULL or empty string), but the behavior previously was to return the empty string (keep calling stream_get_line and once you reach the end, you will just get empty strings).

G

-----Original Message-----
From: ahlin.hans@gmail.com [mailto:ahlin.hans@gmail.com] On Behalf Of Hans Åhlin
Sent: Wednesday, September 02, 2009 10:29 PM
To: internals@lists.php.net
Subject: [PHP-DEV] Fix for 49148 (combination of stream_get_line and fseek does not work correctly)

I just ran the test on Linux 2.6.29 FC11 Apache 2.2.13 and PHP 5.2.9

Result:

string 'line1' (length=5)
string '' (length=0)
string 'line1' (length=5)
string '' (length=0)
string 'line1' (length=5)
string 'line2' (length=5)
string 'line1' (length=5)
boolean false
string 'line1' (length=5)
string 'line2' (length=5)
string 'line1' (length=5)
string 'line2' (length=5)

--
MvH / Hans Åhlin
Tel: +46761488019
http//www.kronan-net.com/

--

--
Pierre

http://blog.thepimp.net | http://www.libgd.org

Fix for 49148 (combination of stream_get_line and fseek does not work correctly) View as Markdown

Index: main/streams/streams.c

I notice that the bug says it happens only on Windows, while the fix doesn't have anything specific for windows. Could you explain why it is so? Another question is why stream->eof is not set in this case on Windows - shouldn't it be?

I notice that the bug says it happens only on Windows, while the fix doesn't have anything specific for windows. Could you explain why it is so? Another question is why stream->eof is not set in this case on Windows - shouldn't it be?

You are right. However, I notice something strange with your patch - if I call stream_get_line again with the same parameters I get "". Shouldn't I get false instead, as it's at EOF? fgets() returns false.

fclose($tmp);

--results-- string(5) "line1" string(0) "" string(0) "" string(0) "" string(0) ""

You are right. However, I notice something strange with your patch - if I call stream_get_line again with the same parameters I get "". Shouldn't I get false instead, as it's at EOF? fgets() returns false.

Index: streams/streams.c

Index: streams/streams.c

// reopen for reading again, this time using fgets() $tmp = fopen($tmpname, "r"); stream_filter_prepend($tmp, 'bzip2.decompress', STREAM_FILTER_READ); var_dump(fgets($tmp)); var_dump(fgets($tmp)); var_dump(fgets($tmp)); var_dump(fgets($tmp)); fclose($tmp);

--output-- string(5) "line1" string(0) "" string(0) "" string(0) "" string(6) "line1 " string(6) "line2 " string(6) "line2 " bool(false)

I notice that the bug says it happens only on Windows, while the fix
doesn't have anything specific for windows. Could you explain why it is so?
Another question is why stream->eof is not set in this case on Windows -
shouldn't it be?

I notice that the bug says it happens only on Windows, while the fix
doesn't have anything specific for windows. Could you explain why it is so?
Another question is why stream->eof is not set in this case on Windows -
shouldn't it be?

You are right. However, I notice something strange with your patch - if
I call stream_get_line again with the same parameters I get "".
Shouldn't I get false instead, as it's at EOF? `fgets()` returns false.

--results--
string(5) "line1"
string(0) ""
string(0) ""
string(0) ""
string(0) ""

You are right. However, I notice something strange with your patch - if
I call stream_get_line again with the same parameters I get "".
Shouldn't I get false instead, as it's at EOF? `fgets()` returns false.

// reopen for reading again, this time using `fgets()`
$tmp = fopen($tmpname, "r");
stream_filter_prepend($tmp, 'bzip2.decompress', STREAM_FILTER_READ);
var_dump(fgets($tmp));
var_dump(fgets($tmp));
var_dump(fgets($tmp));
var_dump(fgets($tmp));
fclose($tmp);

--output--
string(5) "line1"
string(0) ""
string(0) ""
string(0) ""
string(6) "line1
"
string(6) "line2
"
string(6) "line2
"
bool(false)