Skip to content

Commit

Permalink
Fixes #68948 related to a BC break introduced by #68532 fix.
Browse files Browse the repository at this point in the history
  • Loading branch information
jails authored and sgolemon committed Nov 6, 2017
1 parent b152633 commit 5060fc2
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 5 deletions.
30 changes: 30 additions & 0 deletions ext/standard/tests/streams/bug68948.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
--TEST--
Bug #68948: feof() on temporary streams broken
--FILE--
<?php

$testString = '0123456789';

$stream = fopen("php://memory", "r+");
fwrite($stream, $testString);
rewind($stream);

var_dump(fread($stream, 10));
var_dump(ftell($stream));
var_dump(feof($stream));

rewind($stream);

var_dump(fread($stream, 11));
var_dump(ftell($stream));
var_dump(feof($stream));

?>
--EXPECT--
string(10) "0123456789"
int(10)
bool(false)
string(10) "0123456789"
int(10)
bool(true)

10 changes: 6 additions & 4 deletions ext/standard/tests/streams/stream_set_chunk_size.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ var_dump(fwrite($f, str_repeat('b', 3)));

echo "should return previous chunk size (1)\n";
var_dump(stream_set_chunk_size($f, 100));
echo "should elicit one read of size 100 (chunk size)\n";
echo "should elicit 3 reads of size 100 (chunk size)\n";
var_dump(strlen(fread($f, 250)));
echo "should elicit one read of size 100 (chunk size)\n";
var_dump(strlen(fread($f, 50)));
Expand Down Expand Up @@ -67,13 +67,15 @@ write with size: 1
int(3)
should return previous chunk size (1)
int(1)
should elicit one read of size 100 (chunk size)
should elicit 3 reads of size 100 (chunk size)
read with size: 100
int(100)
should elicit one read of size 100 (chunk size)
read with size: 100
read with size: 100
int(250)
should elicit one read of size 100 (chunk size)
int(50)
should elicit no read because there is sufficient cached data
read with size: 100
int(50)
should elicit 2 writes of size 100 and one of size 50
write with size: 100
Expand Down
2 changes: 1 addition & 1 deletion main/streams/streams.c
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ PHPAPI size_t _php_stream_read(php_stream *stream, char *buf, size_t size)
}

/* just break anyway, to avoid greedy read */
if (stream->wrapper != &php_plain_files_wrapper) {
if (!stream->wrapper || stream->wrapper->is_url) {
break;
}
}
Expand Down

5 comments on commit 5060fc2

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgolemon
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like while is_url might seem to be the most expedient answer to the question this diff was trying to answer, it's not the best answer. I'll do a partial revert which adds in exceptions for memory/temp streams.

@sgolemon
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0a45e8f should do the trick, and given that it's mostly a revert to RC5 state, I think it's okay to cherry-pick to 7.2.0. Thoughts?

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolas-grekas can you please give a try to this commit for 75515

@sgolemon I'm +1 for this in 7.2.0, changing test should usually raise our attention, and this revert the changed phpt to same as previous version.

Please sign in to comment.