Skip to content

Commit c9923a6

Browse files
committed
Remove include "sanity check" to get better error
Instead of rejecting directories / non-regular files early with a generic error, we should just accept them and error later when a read is attempted. This is more general and will generate a better error message on Linux. On Windows, the error remains the same as before.
1 parent 17d4002 commit c9923a6

File tree

4 files changed

+48
-26
lines changed

4 files changed

+48
-26
lines changed

Zend/tests/require_directory.phpt

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
Including a directory generates an error
3+
--SKIPIF--
4+
<?php
5+
if (PHP_OS_FAMILY === 'Windows') die("skip Not for Windows");
6+
?>
7+
--FILE--
8+
<?php
9+
10+
/* Just a random test directory */
11+
$dir = __DIR__ . '/variadic';
12+
require $dir;
13+
14+
?>
15+
--EXPECTF--
16+
Notice: require(): Read of %d bytes failed with errno=21 Is a directory in %s on line %d
17+
18+
Fatal error: Uncaught Error: Failed opening required '%s' (include_path='.:') in %s:%d
19+
Stack trace:
20+
#0 {main}
21+
thrown in %s on line %d
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
Including a directory generates an error (Windows variant)
3+
--SKIPIF--
4+
<?php
5+
if (PHP_OS_FAMILY !== 'Windows') die("skip Only for Windows");
6+
?>
7+
--FILE--
8+
<?php
9+
10+
/* Just a random test directory */
11+
$dir = __DIR__ . '/variadic';
12+
require $dir;
13+
14+
?>
15+
--EXPECTF--
16+
Warning: require(%s): Failed to open stream: Permission denied in %s on line %d
17+
18+
Fatal error: Uncaught Error: Failed opening required '%s' (include_path='.:') in %s:%d
19+
Stack trace:
20+
#0 {main}
21+
thrown in %s on line %d

ext/standard/tests/file/bug35740.phpt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
--TEST--
22
Bug #35740 (memory leak when including a directory)
3+
--SKIPIF--
4+
<?php
5+
if (PHP_OS_FAMILY === 'Windows') die("skip Not for Windows");
6+
?>
37
--FILE--
48
<?php
59

@@ -8,7 +12,7 @@ include (__DIR__);
812
echo "Done\n";
913
?>
1014
--EXPECTF--
11-
Warning: include(%s): Failed to open stream: %s in %s on line %d
15+
Notice: include(): Read of %s bytes failed with errno=21 Is a directory in %s on line %d
1216

1317
Warning: include(): Failed opening '%s' for inclusion (include_path='%s') in %s on line %d
1418
Done

main/streams/plain_wrapper.c

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,6 @@ typedef struct {
135135
unsigned is_pipe:1; /* stream is an actual pipe, currently Windows only*/
136136
unsigned cached_fstat:1; /* sb is valid */
137137
unsigned is_pipe_blocking:1; /* allow blocking read() on pipes, currently Windows only */
138-
unsigned no_forced_fstat:1; /* Use fstat cache even if forced */
139138
unsigned is_seekable:1; /* don't try and seek, if not set */
140139
unsigned _reserved:26;
141140

@@ -161,7 +160,7 @@ typedef struct {
161160

162161
static int do_fstat(php_stdio_stream_data *d, int force)
163162
{
164-
if (!d->cached_fstat || (force && !d->no_forced_fstat)) {
163+
if (!d->cached_fstat || force) {
165164
int fd;
166165
int r;
167166

@@ -1188,30 +1187,7 @@ PHPAPI php_stream *_php_stream_fopen(const char *filename, const char *mode, zen
11881187
efree(persistent_id);
11891188
}
11901189

1191-
/* WIN32 always set ISREG flag */
11921190
#ifndef PHP_WIN32
1193-
/* sanity checks for include/require.
1194-
* We check these after opening the stream, so that we save
1195-
* on fstat() syscalls */
1196-
if (options & STREAM_OPEN_FOR_INCLUDE) {
1197-
php_stdio_stream_data *self = (php_stdio_stream_data*)ret->abstract;
1198-
int r;
1199-
1200-
r = do_fstat(self, 0);
1201-
if ((r == 0 && !S_ISREG(self->sb.st_mode))) {
1202-
if (opened_path) {
1203-
zend_string_release_ex(*opened_path, 0);
1204-
*opened_path = NULL;
1205-
}
1206-
php_stream_close(ret);
1207-
return NULL;
1208-
}
1209-
1210-
/* Make sure the fstat result is reused when we later try to get the
1211-
* file size. */
1212-
self->no_forced_fstat = 1;
1213-
}
1214-
12151191
if (options & STREAM_USE_BLOCKING_PIPE) {
12161192
php_stdio_stream_data *self = (php_stdio_stream_data*)ret->abstract;
12171193
self->is_pipe_blocking = 1;

0 commit comments

Comments
 (0)