From d6d60c4880a12a99e42856c43a4f4a5587cede5a Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 20 Jul 2026 22:17:04 +0100 Subject: [PATCH 1/3] streams: refactor _php_stream_open_wrapper_ex() to use early returns --- main/streams/streams.c | 140 ++++++++++++++++++++++++----------------- 1 file changed, 83 insertions(+), 57 deletions(-) diff --git a/main/streams/streams.c b/main/streams/streams.c index 3d8830d7291f..55f091633aca 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -2111,14 +2111,28 @@ PHPAPI php_stream *_php_stream_open_wrapper_ex(const char *path, const char *mod path_to_open = path; wrapper = php_stream_locate_url_wrapper(path, &path_to_open, options); - if ((options & STREAM_USE_URL) && (!wrapper || !wrapper->is_url)) { - if (wrapper) { - php_stream_wrapper_warn(wrapper, context, options, - ProtocolUnsupported, - "This function may only be used against URLs"); - } else { - php_error_docref(NULL, E_WARNING, "This function may only be used against URLs"); + if (UNEXPECTED(!wrapper)) { + php_stream_wrapper_warn_name(PHP_STREAM_ERROR_WRAPPER_DEFAULT_NAME, context, options, OpenFailed, + "Failed to open stream: no suitable wrapper could be found"); + if (resolved_path) { + zend_string_release_ex(resolved_path, 0); } + return NULL; + } + if ((options & STREAM_USE_URL) && !wrapper->is_url) { + php_stream_wrapper_warn(wrapper, context, options, + ProtocolUnsupported, + "This function may only be used against URLs"); + if (resolved_path) { + zend_string_release_ex(resolved_path, 0); + } + return NULL; + } + + if (!wrapper->wops->stream_opener) { + php_stream_wrapper_warn(wrapper, context, options, + NoOpener, + "wrapper does not support stream open"); if (resolved_path) { zend_string_release_ex(resolved_path, 0); } @@ -2127,56 +2141,74 @@ PHPAPI php_stream *_php_stream_open_wrapper_ex(const char *path, const char *mod /* wrapper name needs to be stored as wrapper can be removed in opener (user stream) */ char *wrapper_name = pestrdup(PHP_STREAM_ERROR_WRAPPER_NAME(wrapper), persistent); - if (wrapper) { - if (!wrapper->wops->stream_opener) { - php_stream_wrapper_log_warn(wrapper, context, options & ~REPORT_ERRORS, - NoOpener, - "wrapper does not support stream open"); - } else { - stream = wrapper->wops->stream_opener(wrapper, - path_to_open, mode, options & ~REPORT_ERRORS, - opened_path, context STREAMS_REL_CC); - } + stream = wrapper->wops->stream_opener(wrapper, + path_to_open, mode, options & ~REPORT_ERRORS, + opened_path, context STREAMS_REL_CC); - /* if the caller asked for a persistent stream but the wrapper did not - * return one, force an error here */ - if (stream && persistent && !stream->is_persistent) { - php_stream_wrapper_log_warn(wrapper, context, options & ~REPORT_ERRORS, - PersistentNotSupported, - "wrapper does not support persistent streams"); - php_stream_close(stream); - stream = NULL; + if (UNEXPECTED(!stream)) { + if (options & REPORT_ERRORS) { + php_stream_display_wrapper_name_errors(wrapper_name, context, PHP_STREAM_EC(OpenFailed), + "Failed to open stream"); + if (opened_path && *opened_path) { + zend_string_release_ex(*opened_path, 0); + *opened_path = NULL; + } } - - if (stream) { - stream->wrapper = wrapper; + php_stream_tidy_wrapper_name_error_log(wrapper_name); + pefree(wrapper_name, persistent); + if (resolved_path) { + zend_string_release_ex(resolved_path, 0); } + return NULL; } - if (stream) { - if (opened_path && !*opened_path && resolved_path) { - *opened_path = resolved_path; - resolved_path = NULL; + /* if the caller asked for a persistent stream but the wrapper did not + * return one, force an error here */ + if (persistent && !stream->is_persistent) { + php_stream_wrapper_log_warn(wrapper, context, options & ~REPORT_ERRORS, + PersistentNotSupported, + "wrapper does not support persistent streams"); + php_stream_close(stream); + if (options & REPORT_ERRORS) { + php_stream_display_wrapper_name_errors(wrapper_name, context, PHP_STREAM_EC(OpenFailed), + "Failed to open stream"); + if (opened_path && *opened_path) { + zend_string_release_ex(*opened_path, 0); + *opened_path = NULL; + } } - if (stream->orig_path) { - pefree(stream->orig_path, persistent); + php_stream_tidy_wrapper_name_error_log(wrapper_name); + pefree(wrapper_name, persistent); + if (resolved_path) { + zend_string_release_ex(resolved_path, 0); } - stream->orig_path = pestrdup(path, persistent); + return NULL; + } + + stream->wrapper = wrapper; + + if (opened_path && !*opened_path && resolved_path) { + *opened_path = resolved_path; + resolved_path = NULL; + } + if (stream->orig_path) { + pefree(stream->orig_path, persistent); + } + stream->orig_path = pestrdup(path, persistent); #if ZEND_DEBUG stream->open_filename = __zend_orig_filename ? __zend_orig_filename : __zend_filename; stream->open_lineno = __zend_orig_lineno ? __zend_orig_lineno : __zend_lineno; #endif - /* Attach an explicitly provided context to the stream, but never the - * default context: sharing it by reference would let a later - * stream_context_set_option() on the stream mutate the global default - * context, leaking options into every other stream. Stream errors fall - * back to the default context on their own when the stream has none. */ - if (stream->ctx == NULL && context != NULL && context != FG(default_context) && !persistent) { - php_stream_context_set(stream, context); - } + /* Attach an explicitly provided context to the stream, but never the + * default context: sharing it by reference would let a later + * stream_context_set_option() on the stream mutate the global default + * context, leaking options into every other stream. Stream errors fall + * back to the default context on their own when the stream has none. */ + if (stream->ctx == NULL && context != NULL && context != FG(default_context) && !persistent) { + php_stream_context_set(stream, context); } - if (stream != NULL && (options & STREAM_MUST_SEEK)) { + if (options & STREAM_MUST_SEEK) { php_stream *newstream; switch(php_stream_make_seekable_rel(stream, &newstream, @@ -2200,16 +2232,19 @@ PHPAPI php_stream *_php_stream_open_wrapper_ex(const char *path, const char *mod return newstream; default: php_stream_close(stream); - stream = NULL; php_stream_wrapper_warn(wrapper, context, options, SeekNotSupported, "could not make seekable - %s", path); - /* We do not want multiple errors so we negate it */ - options &= ~REPORT_ERRORS; + php_stream_tidy_wrapper_name_error_log(wrapper_name); + pefree(wrapper_name, persistent); + if (resolved_path) { + zend_string_release_ex(resolved_path, 0); + } + return NULL; } } - if (stream && stream->ops->seek && (stream->flags & PHP_STREAM_FLAG_NO_SEEK) == 0 && strchr(mode, 'a') && stream->position == 0) { + if (stream->ops->seek && (stream->flags & PHP_STREAM_FLAG_NO_SEEK) == 0 && strchr(mode, 'a') && stream->position == 0) { zend_off_t newpos = 0; /* if opened for append, we need to revise our idea of the initial file position */ @@ -2218,15 +2253,6 @@ PHPAPI php_stream *_php_stream_open_wrapper_ex(const char *path, const char *mod } } - if (stream == NULL && (options & REPORT_ERRORS)) { - php_stream_display_wrapper_name_errors(wrapper_name, context, PHP_STREAM_EC(OpenFailed), - "Failed to open stream"); - if (opened_path && *opened_path) { - zend_string_release_ex(*opened_path, 0); - *opened_path = NULL; - } - } - php_stream_tidy_wrapper_name_error_log(wrapper_name); pefree(wrapper_name, persistent); if (resolved_path) { zend_string_release_ex(resolved_path, 0); From 75df92922bdf153050855eda31a21ff4cf94b777 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 20 Jul 2026 22:23:52 +0100 Subject: [PATCH 2/3] streams: use php_stream_wrapper_warn() in _php_stream_open_wrapper_ex() Instead of the old way of logging errors and displaying them use the new APIs. --- main/streams/streams.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/main/streams/streams.c b/main/streams/streams.c index 55f091633aca..983c6ccde86d 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -2130,9 +2130,8 @@ PHPAPI php_stream *_php_stream_open_wrapper_ex(const char *path, const char *mod } if (!wrapper->wops->stream_opener) { - php_stream_wrapper_warn(wrapper, context, options, - NoOpener, - "wrapper does not support stream open"); + php_stream_wrapper_warn(wrapper, context, options, NoOpener, + "Failed to open stream: wrapper does not support stream open"); if (resolved_path) { zend_string_release_ex(resolved_path, 0); } @@ -2165,19 +2164,15 @@ PHPAPI php_stream *_php_stream_open_wrapper_ex(const char *path, const char *mod /* if the caller asked for a persistent stream but the wrapper did not * return one, force an error here */ if (persistent && !stream->is_persistent) { - php_stream_wrapper_log_warn(wrapper, context, options & ~REPORT_ERRORS, - PersistentNotSupported, - "wrapper does not support persistent streams"); + php_stream_wrapper_warn(wrapper, context, options, PersistentNotSupported, + "Failed to open stream: wrapper does not support persistent streams"); php_stream_close(stream); if (options & REPORT_ERRORS) { - php_stream_display_wrapper_name_errors(wrapper_name, context, PHP_STREAM_EC(OpenFailed), - "Failed to open stream"); if (opened_path && *opened_path) { zend_string_release_ex(*opened_path, 0); *opened_path = NULL; } } - php_stream_tidy_wrapper_name_error_log(wrapper_name); pefree(wrapper_name, persistent); if (resolved_path) { zend_string_release_ex(resolved_path, 0); @@ -2235,7 +2230,6 @@ PHPAPI php_stream *_php_stream_open_wrapper_ex(const char *path, const char *mod php_stream_wrapper_warn(wrapper, context, options, SeekNotSupported, "could not make seekable - %s", path); - php_stream_tidy_wrapper_name_error_log(wrapper_name); pefree(wrapper_name, persistent); if (resolved_path) { zend_string_release_ex(resolved_path, 0); From 0d530e5979899db7ea6f43273e0afbcfe23d2a39 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 21 Jul 2026 00:39:19 +0100 Subject: [PATCH 3/3] streams: use common cleanup code in _php_stream_open_wrapper_ex() --- main/streams/streams.c | 73 ++++++++++++------------------------------ 1 file changed, 21 insertions(+), 52 deletions(-) diff --git a/main/streams/streams.c b/main/streams/streams.c index 983c6ccde86d..64651b1c7e53 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -2101,10 +2101,7 @@ PHPAPI php_stream *_php_stream_open_wrapper_ex(const char *path, const char *mod options &= ~USE_PATH; } if (EG(exception)) { - if (resolved_path) { - zend_string_release_ex(resolved_path, false); - } - return NULL; + goto cleanup_no_wrapper_name; } } @@ -2114,28 +2111,19 @@ PHPAPI php_stream *_php_stream_open_wrapper_ex(const char *path, const char *mod if (UNEXPECTED(!wrapper)) { php_stream_wrapper_warn_name(PHP_STREAM_ERROR_WRAPPER_DEFAULT_NAME, context, options, OpenFailed, "Failed to open stream: no suitable wrapper could be found"); - if (resolved_path) { - zend_string_release_ex(resolved_path, 0); - } - return NULL; + goto cleanup_no_wrapper_name; } if ((options & STREAM_USE_URL) && !wrapper->is_url) { php_stream_wrapper_warn(wrapper, context, options, ProtocolUnsupported, "This function may only be used against URLs"); - if (resolved_path) { - zend_string_release_ex(resolved_path, 0); - } - return NULL; + goto cleanup_no_wrapper_name; } if (!wrapper->wops->stream_opener) { php_stream_wrapper_warn(wrapper, context, options, NoOpener, "Failed to open stream: wrapper does not support stream open"); - if (resolved_path) { - zend_string_release_ex(resolved_path, 0); - } - return NULL; + goto cleanup_no_wrapper_name; } /* wrapper name needs to be stored as wrapper can be removed in opener (user stream) */ @@ -2148,17 +2136,9 @@ PHPAPI php_stream *_php_stream_open_wrapper_ex(const char *path, const char *mod if (options & REPORT_ERRORS) { php_stream_display_wrapper_name_errors(wrapper_name, context, PHP_STREAM_EC(OpenFailed), "Failed to open stream"); - if (opened_path && *opened_path) { - zend_string_release_ex(*opened_path, 0); - *opened_path = NULL; - } } php_stream_tidy_wrapper_name_error_log(wrapper_name); - pefree(wrapper_name, persistent); - if (resolved_path) { - zend_string_release_ex(resolved_path, 0); - } - return NULL; + goto cleanup; } /* if the caller asked for a persistent stream but the wrapper did not @@ -2167,17 +2147,8 @@ PHPAPI php_stream *_php_stream_open_wrapper_ex(const char *path, const char *mod php_stream_wrapper_warn(wrapper, context, options, PersistentNotSupported, "Failed to open stream: wrapper does not support persistent streams"); php_stream_close(stream); - if (options & REPORT_ERRORS) { - if (opened_path && *opened_path) { - zend_string_release_ex(*opened_path, 0); - *opened_path = NULL; - } - } - pefree(wrapper_name, persistent); - if (resolved_path) { - zend_string_release_ex(resolved_path, 0); - } - return NULL; + stream = NULL; + goto cleanup; } stream->wrapper = wrapper; @@ -2210,31 +2181,21 @@ PHPAPI php_stream *_php_stream_open_wrapper_ex(const char *path, const char *mod (options & STREAM_WILL_CAST) ? PHP_STREAM_PREFER_STDIO : PHP_STREAM_NO_PREFERENCE)) { case PHP_STREAM_UNCHANGED: - if (resolved_path) { - zend_string_release_ex(resolved_path, 0); - } - pefree(wrapper_name, persistent); - return stream; + goto cleanup; case PHP_STREAM_RELEASED: if (newstream->orig_path) { pefree(newstream->orig_path, persistent); } newstream->orig_path = pestrdup(path, persistent); - if (resolved_path) { - zend_string_release_ex(resolved_path, 0); - } - pefree(wrapper_name, persistent); - return newstream; + stream = newstream; + goto cleanup; default: - php_stream_close(stream); php_stream_wrapper_warn(wrapper, context, options, SeekNotSupported, "could not make seekable - %s", path); - pefree(wrapper_name, persistent); - if (resolved_path) { - zend_string_release_ex(resolved_path, 0); - } - return NULL; + php_stream_close(stream); + stream = NULL; + goto cleanup; } } @@ -2247,10 +2208,18 @@ PHPAPI php_stream *_php_stream_open_wrapper_ex(const char *path, const char *mod } } +cleanup: pefree(wrapper_name, persistent); +cleanup_no_wrapper_name: if (resolved_path) { zend_string_release_ex(resolved_path, 0); } + if (stream == NULL) { + if (opened_path && *opened_path) { + zend_string_release_ex(*opened_path, 0); + *opened_path = NULL; + } + } return stream; } /* }}} */