Skip to content

Commit cbb459c

Browse files
committed
Fix phpGH-10834: exif_read_data() cannot read smaller stream wrapper chunk sizes
Exif uses php_stream_read() to read data from the stream and process it. In this case, it's a userspace stream. Userspace streams can either by local or url streams. In 5060fc2 and subsequently 0a45e8f, the behaviour of the check was changed twice in total. In the latter commit the description talks about exceptions for streams that are local to the process. It looks like the exception for local userspace streams was forgotten. This patch updates the check such that local userspace streams also read greedily, while keeping url userspace streams unchanged. This also updates the existing test to test both local and url userspace streams.
1 parent e2ec59f commit cbb459c

File tree

2 files changed

+75
-32
lines changed

2 files changed

+75
-32
lines changed

ext/standard/tests/streams/stream_set_chunk_size.phpt

Lines changed: 73 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -24,45 +24,87 @@ class test_wrapper {
2424
}
2525
}
2626

27-
var_dump(stream_wrapper_register('test', 'test_wrapper'));
27+
var_dump(stream_wrapper_register('testlocal', 'test_wrapper'));
28+
var_dump(stream_wrapper_register('testurl', 'test_wrapper', STREAM_IS_URL));
2829

29-
$f = fopen("test://foo","r");
30+
foreach (['testlocal', 'testurl'] as $stream_type) {
31+
$f = fopen("$stream_type://foo","r");
3032

31-
/* when the chunk size is 1, the read buffer is skipped, but the
32-
* the writes are made in chunks of size 1 (business as usual)
33-
* This should probably be revisited */
34-
echo "should return previous chunk size (8192)\n";
35-
var_dump(stream_set_chunk_size($f, 1));
36-
echo "should be read without buffer (\$count == 10000)\n";
37-
var_dump(strlen(fread($f, 10000)));
38-
echo "should have no effect on writes\n";
39-
var_dump(fwrite($f, str_repeat('b', 3)));
33+
/* when the chunk size is 1, the read buffer is skipped, but the
34+
* writes are made in chunks of size 1 (business as usual)
35+
* This should probably be revisited */
36+
echo "should return previous chunk size (8192)\n";
37+
var_dump(stream_set_chunk_size($f, 1));
38+
echo "should be read without buffer (\$count == 10000)\n";
39+
var_dump(strlen(fread($f, 10000)));
40+
echo "should have no effect on writes\n";
41+
var_dump(fwrite($f, str_repeat('b', 3)));
4042

41-
echo "should return previous chunk size (1)\n";
42-
var_dump(stream_set_chunk_size($f, 100));
43-
echo "should elicit one read of size 100 (chunk size)\n";
44-
var_dump(strlen(fread($f, 250)));
45-
echo "should elicit one read of size 100 (chunk size)\n";
46-
var_dump(strlen(fread($f, 50)));
47-
echo "should elicit no read because there is sufficient cached data\n";
48-
var_dump(strlen(fread($f, 50)));
49-
echo "should have no effect on writes\n";
50-
var_dump(strlen(fwrite($f, str_repeat('b', 250))));
43+
echo "should return previous chunk size (1)\n";
44+
var_dump(stream_set_chunk_size($f, 100));
45+
if ($stream_type === 'testurl') {
46+
echo "should elicit one read of size 100 (chunk size)\n";
47+
var_dump(strlen(fread($f, 250)));
48+
echo "should elicit one read of size 100 (chunk size)\n";
49+
var_dump(strlen(fread($f, 50)));
50+
echo "should elicit no read because there is sufficient cached data\n";
51+
var_dump(strlen(fread($f, 50)));
52+
} else { // testlocal
53+
echo "should elicit 3 reads of size 100 (chunk size)\n";
54+
var_dump(strlen(fread($f, 250)));
55+
echo "should not elicit a read\n";
56+
var_dump(strlen(fread($f, 50)));
57+
echo "should elicit a read because there is insufficient cached data\n";
58+
var_dump(strlen(fread($f, 50)));
59+
}
60+
echo "should have no effect on writes\n";
61+
var_dump(strlen(fwrite($f, str_repeat('b', 250))));
5162

52-
echo "\nerror conditions\n";
53-
try {
54-
stream_set_chunk_size($f, 0);
55-
} catch (ValueError $exception) {
56-
echo $exception->getMessage() . "\n";
57-
}
58-
try {
59-
stream_set_chunk_size($f, -1);
60-
} catch (ValueError $exception) {
61-
echo $exception->getMessage() . "\n";
63+
echo "\nerror conditions\n";
64+
try {
65+
stream_set_chunk_size($f, 0);
66+
} catch (ValueError $exception) {
67+
echo $exception->getMessage() . "\n";
68+
}
69+
try {
70+
stream_set_chunk_size($f, -1);
71+
} catch (ValueError $exception) {
72+
echo $exception->getMessage() . "\n";
73+
}
74+
75+
fclose($f);
6276
}
6377
?>
6478
--EXPECT--
6579
bool(true)
80+
bool(true)
81+
should return previous chunk size (8192)
82+
int(8192)
83+
should be read without buffer ($count == 10000)
84+
read with size: 10000
85+
int(10000)
86+
should have no effect on writes
87+
write with size: 3
88+
int(3)
89+
should return previous chunk size (1)
90+
int(1)
91+
should elicit 3 reads of size 100 (chunk size)
92+
read with size: 100
93+
read with size: 100
94+
read with size: 100
95+
int(250)
96+
should not elicit a read
97+
int(50)
98+
should elicit a read because there is insufficient cached data
99+
read with size: 100
100+
int(50)
101+
should have no effect on writes
102+
write with size: 250
103+
int(3)
104+
105+
error conditions
106+
stream_set_chunk_size(): Argument #2 ($size) must be greater than 0
107+
stream_set_chunk_size(): Argument #2 ($size) must be greater than 0
66108
should return previous chunk size (8192)
67109
int(8192)
68110
should be read without buffer ($count == 10000)

main/streams/streams.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -748,7 +748,8 @@ PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size)
748748
/* just break anyway, to avoid greedy read for file://, php://memory, and php://temp */
749749
if ((stream->wrapper != &php_plain_files_wrapper) &&
750750
(stream->ops != &php_stream_memory_ops) &&
751-
(stream->ops != &php_stream_temp_ops)) {
751+
(stream->ops != &php_stream_temp_ops) &&
752+
!(stream->ops == &php_stream_userspace_ops && !stream->wrapper->is_url)) {
752753
break;
753754
}
754755
}

0 commit comments

Comments
 (0)