Skip to content

Commit 0dcf2c0

Browse files
committed
stream: check fopen() mode ahead of time
And change the signature of php_stream_parse_fopen_modes() to not require an out param as the failure can simply be communicated via -1 which is not a valid open flag mode.
1 parent 0fdd7ab commit 0dcf2c0

8 files changed

Lines changed: 44 additions & 29 deletions

File tree

UPGRADING.INTERNALS

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,12 @@ PHP 8.6 INTERNALS UPGRADE NOTES
136136
. Added zend_compile_ast().
137137
. Added zend_check_type_ex().
138138
. Added zend_create_partial_closure().
139+
. The php_stream_parse_fopen_modes() function was changed from the following
140+
PHPAPI int php_stream_parse_fopen_modes(const char *mode, int *open_flags);
141+
to
142+
PHPAPI int php_stream_parse_fopen_modes(const char *mode);
143+
Instead of returning SUCCESS or FAILURE it now returns the open flags or -1
144+
on failure.
139145

140146
========================
141147
2. Build system changes

ext/bz2/tests/002.phpt

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,18 @@ var_dump(bzopen($fp, "r"));
2727
$fp = fopen("bz_open_002.txt", "wb");
2828
var_dump(bzopen($fp, "w"));
2929

30-
$fp = fopen("bz_open_002.txt", "br");
3130
try {
31+
$fp = fopen("bz_open_002.txt", "br");
3232
var_dump(bzopen($fp, "r"));
33-
} catch (\TypeError $e) {
34-
echo $e->getMessage() . \PHP_EOL;
33+
} catch (\Throwable $e) {
34+
echo $e::class, ': ', $e->getMessage(), \PHP_EOL;
3535
}
3636

37-
$fp = fopen("bz_open_002.txt", "br");
3837
try {
38+
$fp = fopen("bz_open_002.txt", "br");
3939
var_dump(bzopen($fp, "w"));
40-
} catch (\TypeError $e) {
41-
echo $e->getMessage() . \PHP_EOL;
40+
} catch (\Throwable $e) {
41+
echo $e::class, ': ', $e->getMessage(), \PHP_EOL;
4242
}
4343

4444
$fp = fopen("bz_open_002.txt", "r");
@@ -89,12 +89,8 @@ Warning: bzopen(): Cannot read from a stream opened in write only mode in %s on
8989
bool(false)
9090
resource(%d) of type (stream)
9191
resource(%d) of type (stream)
92-
93-
Warning: fopen(bz_open_002.txt): Failed to open stream: `br' is not a valid mode for fopen in %s on line %d
94-
bzopen(): Argument #1 ($file) must be of type string or file-resource, false given
95-
96-
Warning: fopen(bz_open_002.txt): Failed to open stream: `br' is not a valid mode for fopen in %s on line %d
97-
bzopen(): Argument #1 ($file) must be of type string or file-resource, false given
92+
ValueError: fopen(): Argument #2 ($mode) must be a valid mode
93+
ValueError: fopen(): Argument #2 ($mode) must be a valid mode
9894

9995
Warning: bzopen(): cannot write to a stream opened in read only mode in %s on line %d
10096
bool(false)

ext/standard/file.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -747,6 +747,12 @@ PHP_FUNCTION(fopen)
747747
Z_PARAM_RESOURCE_OR_NULL(zcontext)
748748
ZEND_PARSE_PARAMETERS_END();
749749

750+
int open_flags = php_stream_parse_fopen_modes(mode);
751+
if (UNEXPECTED(open_flags == -1)) {
752+
zend_argument_value_error(2, "must be a valid mode");
753+
RETURN_THROWS();
754+
}
755+
750756
php_stream_error_operation_begin();
751757
context = php_stream_context_from_zval(zcontext, 0);
752758

ext/standard/tests/file/bug76735.phpt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22
Bug #76735 (Incorrect message in fopen on invalid mode)
33
--FILE--
44
<?php
5-
fopen(__FILE__, 'Q');
5+
try {
6+
fopen(__FILE__, 'Q');
7+
} catch (\Throwable $e) {
8+
echo $e::class, ': ', $e->getMessage(), \PHP_EOL;
9+
}
610
?>
7-
--EXPECTF--
8-
Warning: fopen(%s): Failed to open stream: `Q' is not a valid mode for fopen in %s on line %d
11+
--EXPECT--
12+
ValueError: fopen(): Argument #2 ($mode) must be a valid mode

main/streams/php_stream_plain_wrapper.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,6 @@ PHPAPI FILE * _php_stream_open_wrapper_as_file(char * path, char * mode, int opt
4949
#define php_stream_open_wrapper_as_file(path, mode, options, opened_path) _php_stream_open_wrapper_as_file((path), (mode), (options), (opened_path) STREAMS_CC)
5050

5151
/* parse standard "fopen" modes into open() flags */
52-
PHPAPI int php_stream_parse_fopen_modes(const char *mode, int *open_flags);
52+
PHPAPI int php_stream_parse_fopen_modes(const char *mode);
5353

5454
END_EXTERN_C()

main/streams/plain_wrapper.c

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ extern int php_get_gid_by_name(const char *name, gid_t *gid);
6969
# endif
7070
#endif
7171

72-
/* parse standard "fopen" modes into open() flags */
73-
PHPAPI int php_stream_parse_fopen_modes(const char *mode, int *open_flags)
72+
/* parse standard "fopen" modes into open() flags. Returns -1 on failure, open flags on success. */
73+
PHPAPI int php_stream_parse_fopen_modes(const char *mode)
7474
{
7575
int flags;
7676

@@ -92,7 +92,7 @@ PHPAPI int php_stream_parse_fopen_modes(const char *mode, int *open_flags)
9292
break;
9393
default:
9494
/* unknown mode */
95-
return FAILURE;
95+
return -1;
9696
}
9797

9898
if (strchr(mode, '+')) {
@@ -123,8 +123,7 @@ PHPAPI int php_stream_parse_fopen_modes(const char *mode, int *open_flags)
123123
}
124124
#endif
125125

126-
*open_flags = flags;
127-
return SUCCESS;
126+
return flags;
128127
}
129128

130129

@@ -1166,15 +1165,15 @@ static php_stream *php_plain_files_dir_opener(php_stream_wrapper *wrapper, const
11661165
PHPAPI php_stream *_php_stream_fopen(const char *filename, const char *mode, zend_string **opened_path, int options STREAMS_DC)
11671166
{
11681167
char realpath[MAXPATHLEN];
1169-
int open_flags;
11701168
int fd;
11711169
php_stream *ret;
11721170
int persistent = options & STREAM_OPEN_PERSISTENT;
11731171
char *persistent_id = NULL;
11741172

1175-
if (FAILURE == php_stream_parse_fopen_modes(mode, &open_flags)) {
1173+
int open_flags = php_stream_parse_fopen_modes(mode);
1174+
if (UNEXPECTED(open_flags == -1)) {
11761175
php_stream_wrapper_log_warn(&php_plain_files_wrapper, NULL, options,
1177-
InvalidMode, "`%s' is not a valid mode for fopen", mode);
1176+
InvalidMode, "\"%s\" is not a valid mode for fopen", mode);
11781177
return NULL;
11791178
}
11801179

main/streams/streams.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1611,13 +1611,15 @@ PHPAPI zend_result _php_stream_copy_to_stream_ex(php_stream *src, php_stream *de
16111611
src->writepos == src->readpos) {
16121612
/* both php_stream instances are backed by a file descriptor, are not filtered and the
16131613
* read buffer is empty: we can use copy_file_range() */
1614-
int src_fd, dest_fd, dest_open_flags = 0;
1614+
int src_fd, dest_fd = 0;
1615+
1616+
/* get dest open flags to check if the stream is open in append mode */
1617+
int dest_open_flags = php_stream_parse_fopen_modes(dest->mode);
1618+
ZEND_ASSERT(dest_open_flags > -1 && "Must be able to parse stream open flag mode if we have a valid stream");
16151619

16161620
/* copy_file_range does not work with O_APPEND */
16171621
if (php_stream_cast(src, PHP_STREAM_AS_FD, (void*)&src_fd, 0) == SUCCESS &&
16181622
php_stream_cast(dest, PHP_STREAM_AS_FD, (void*)&dest_fd, 0) == SUCCESS &&
1619-
/* get dest open flags to check if the stream is open in append mode */
1620-
php_stream_parse_fopen_modes(dest->mode, &dest_open_flags) == SUCCESS &&
16211623
!(dest_open_flags & O_APPEND)) {
16221624

16231625
/* clamp to INT_MAX to avoid EOVERFLOW */

win32/ioutil.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,7 @@ PW32IO FILE *php_win32_ioutil_fopen_w(const wchar_t *path, const wchar_t *mode)
691691
{/*{{{*/
692692
FILE *ret;
693693
char modea[16] = {0};
694-
int err = 0, fd, flags, i = 0;
694+
int err = 0, fd, i = 0;
695695

696696
PHP_WIN32_IOUTIL_CHECK_PATH_W(path, NULL, 0)
697697

@@ -700,7 +700,9 @@ PW32IO FILE *php_win32_ioutil_fopen_w(const wchar_t *path, const wchar_t *mode)
700700
modea[i] = (char)mode[i];
701701
i++;
702702
}
703-
if (SUCCESS != php_stream_parse_fopen_modes(modea, &flags)) {
703+
704+
int flags = php_stream_parse_fopen_modes(modea);
705+
if (UNEXPECTED(flags == -1)) {
704706
SET_ERRNO_FROM_WIN32_CODE(ERROR_INVALID_PARAMETER);
705707
return NULL;
706708
}

0 commit comments

Comments
 (0)