From 0dcf2c0ab6c640d29c831761bd4cc48987a5a043 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Sun, 19 Jul 2026 15:05:07 +0100 Subject: [PATCH] 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. --- UPGRADING.INTERNALS | 6 ++++++ ext/bz2/tests/002.phpt | 20 ++++++++------------ ext/standard/file.c | 6 ++++++ ext/standard/tests/file/bug76735.phpt | 10 +++++++--- main/streams/php_stream_plain_wrapper.h | 2 +- main/streams/plain_wrapper.c | 15 +++++++-------- main/streams/streams.c | 8 +++++--- win32/ioutil.c | 6 ++++-- 8 files changed, 44 insertions(+), 29 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index 7b7b7c721afb..50757c20d0ca 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -136,6 +136,12 @@ PHP 8.6 INTERNALS UPGRADE NOTES . Added zend_compile_ast(). . Added zend_check_type_ex(). . Added zend_create_partial_closure(). + . The php_stream_parse_fopen_modes() function was changed from the following + PHPAPI int php_stream_parse_fopen_modes(const char *mode, int *open_flags); + to + PHPAPI int php_stream_parse_fopen_modes(const char *mode); + Instead of returning SUCCESS or FAILURE it now returns the open flags or -1 + on failure. ======================== 2. Build system changes diff --git a/ext/bz2/tests/002.phpt b/ext/bz2/tests/002.phpt index f87048f92314..79204777a725 100644 --- a/ext/bz2/tests/002.phpt +++ b/ext/bz2/tests/002.phpt @@ -27,18 +27,18 @@ var_dump(bzopen($fp, "r")); $fp = fopen("bz_open_002.txt", "wb"); var_dump(bzopen($fp, "w")); -$fp = fopen("bz_open_002.txt", "br"); try { + $fp = fopen("bz_open_002.txt", "br"); var_dump(bzopen($fp, "r")); -} catch (\TypeError $e) { - echo $e->getMessage() . \PHP_EOL; +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), \PHP_EOL; } -$fp = fopen("bz_open_002.txt", "br"); try { + $fp = fopen("bz_open_002.txt", "br"); var_dump(bzopen($fp, "w")); -} catch (\TypeError $e) { - echo $e->getMessage() . \PHP_EOL; +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), \PHP_EOL; } $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 bool(false) resource(%d) of type (stream) resource(%d) of type (stream) - -Warning: fopen(bz_open_002.txt): Failed to open stream: `br' is not a valid mode for fopen in %s on line %d -bzopen(): Argument #1 ($file) must be of type string or file-resource, false given - -Warning: fopen(bz_open_002.txt): Failed to open stream: `br' is not a valid mode for fopen in %s on line %d -bzopen(): Argument #1 ($file) must be of type string or file-resource, false given +ValueError: fopen(): Argument #2 ($mode) must be a valid mode +ValueError: fopen(): Argument #2 ($mode) must be a valid mode Warning: bzopen(): cannot write to a stream opened in read only mode in %s on line %d bool(false) diff --git a/ext/standard/file.c b/ext/standard/file.c index fd6f578e4ef0..e4d2754dc0d9 100644 --- a/ext/standard/file.c +++ b/ext/standard/file.c @@ -747,6 +747,12 @@ PHP_FUNCTION(fopen) Z_PARAM_RESOURCE_OR_NULL(zcontext) ZEND_PARSE_PARAMETERS_END(); + int open_flags = php_stream_parse_fopen_modes(mode); + if (UNEXPECTED(open_flags == -1)) { + zend_argument_value_error(2, "must be a valid mode"); + RETURN_THROWS(); + } + php_stream_error_operation_begin(); context = php_stream_context_from_zval(zcontext, 0); diff --git a/ext/standard/tests/file/bug76735.phpt b/ext/standard/tests/file/bug76735.phpt index 8469312801e2..93b9a3ec8b27 100644 --- a/ext/standard/tests/file/bug76735.phpt +++ b/ext/standard/tests/file/bug76735.phpt @@ -2,7 +2,11 @@ Bug #76735 (Incorrect message in fopen on invalid mode) --FILE-- getMessage(), \PHP_EOL; +} ?> ---EXPECTF-- -Warning: fopen(%s): Failed to open stream: `Q' is not a valid mode for fopen in %s on line %d +--EXPECT-- +ValueError: fopen(): Argument #2 ($mode) must be a valid mode diff --git a/main/streams/php_stream_plain_wrapper.h b/main/streams/php_stream_plain_wrapper.h index f44edcb16a97..f8bf70d78af9 100644 --- a/main/streams/php_stream_plain_wrapper.h +++ b/main/streams/php_stream_plain_wrapper.h @@ -49,6 +49,6 @@ PHPAPI FILE * _php_stream_open_wrapper_as_file(char * path, char * mode, int opt #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) /* parse standard "fopen" modes into open() flags */ -PHPAPI int php_stream_parse_fopen_modes(const char *mode, int *open_flags); +PHPAPI int php_stream_parse_fopen_modes(const char *mode); END_EXTERN_C() diff --git a/main/streams/plain_wrapper.c b/main/streams/plain_wrapper.c index 9289b6b5ec6e..9a3627fd08f7 100644 --- a/main/streams/plain_wrapper.c +++ b/main/streams/plain_wrapper.c @@ -69,8 +69,8 @@ extern int php_get_gid_by_name(const char *name, gid_t *gid); # endif #endif -/* parse standard "fopen" modes into open() flags */ -PHPAPI int php_stream_parse_fopen_modes(const char *mode, int *open_flags) +/* parse standard "fopen" modes into open() flags. Returns -1 on failure, open flags on success. */ +PHPAPI int php_stream_parse_fopen_modes(const char *mode) { int flags; @@ -92,7 +92,7 @@ PHPAPI int php_stream_parse_fopen_modes(const char *mode, int *open_flags) break; default: /* unknown mode */ - return FAILURE; + return -1; } if (strchr(mode, '+')) { @@ -123,8 +123,7 @@ PHPAPI int php_stream_parse_fopen_modes(const char *mode, int *open_flags) } #endif - *open_flags = flags; - return SUCCESS; + return flags; } @@ -1166,15 +1165,15 @@ static php_stream *php_plain_files_dir_opener(php_stream_wrapper *wrapper, const PHPAPI php_stream *_php_stream_fopen(const char *filename, const char *mode, zend_string **opened_path, int options STREAMS_DC) { char realpath[MAXPATHLEN]; - int open_flags; int fd; php_stream *ret; int persistent = options & STREAM_OPEN_PERSISTENT; char *persistent_id = NULL; - if (FAILURE == php_stream_parse_fopen_modes(mode, &open_flags)) { + int open_flags = php_stream_parse_fopen_modes(mode); + if (UNEXPECTED(open_flags == -1)) { php_stream_wrapper_log_warn(&php_plain_files_wrapper, NULL, options, - InvalidMode, "`%s' is not a valid mode for fopen", mode); + InvalidMode, "\"%s\" is not a valid mode for fopen", mode); return NULL; } diff --git a/main/streams/streams.c b/main/streams/streams.c index c3bde0a8b109..80d000c65975 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -1611,13 +1611,15 @@ PHPAPI zend_result _php_stream_copy_to_stream_ex(php_stream *src, php_stream *de src->writepos == src->readpos) { /* both php_stream instances are backed by a file descriptor, are not filtered and the * read buffer is empty: we can use copy_file_range() */ - int src_fd, dest_fd, dest_open_flags = 0; + int src_fd, dest_fd = 0; + + /* get dest open flags to check if the stream is open in append mode */ + int dest_open_flags = php_stream_parse_fopen_modes(dest->mode); + ZEND_ASSERT(dest_open_flags > -1 && "Must be able to parse stream open flag mode if we have a valid stream"); /* copy_file_range does not work with O_APPEND */ if (php_stream_cast(src, PHP_STREAM_AS_FD, (void*)&src_fd, 0) == SUCCESS && php_stream_cast(dest, PHP_STREAM_AS_FD, (void*)&dest_fd, 0) == SUCCESS && - /* get dest open flags to check if the stream is open in append mode */ - php_stream_parse_fopen_modes(dest->mode, &dest_open_flags) == SUCCESS && !(dest_open_flags & O_APPEND)) { /* clamp to INT_MAX to avoid EOVERFLOW */ diff --git a/win32/ioutil.c b/win32/ioutil.c index 0d77649796a7..2998c3c061ba 100644 --- a/win32/ioutil.c +++ b/win32/ioutil.c @@ -691,7 +691,7 @@ PW32IO FILE *php_win32_ioutil_fopen_w(const wchar_t *path, const wchar_t *mode) {/*{{{*/ FILE *ret; char modea[16] = {0}; - int err = 0, fd, flags, i = 0; + int err = 0, fd, i = 0; PHP_WIN32_IOUTIL_CHECK_PATH_W(path, NULL, 0) @@ -700,7 +700,9 @@ PW32IO FILE *php_win32_ioutil_fopen_w(const wchar_t *path, const wchar_t *mode) modea[i] = (char)mode[i]; i++; } - if (SUCCESS != php_stream_parse_fopen_modes(modea, &flags)) { + + int flags = php_stream_parse_fopen_modes(modea); + if (UNEXPECTED(flags == -1)) { SET_ERRNO_FROM_WIN32_CODE(ERROR_INVALID_PARAMETER); return NULL; }