From 9a49de470653ab776c4c38b86a32563e917c31b6 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 15 Feb 2025 13:13:55 +0100 Subject: [PATCH] Reduce number of string duplications in phar We don't need to duplicate these strings from the resource, we can just use them with an offset. To prove this was safe, I had to make the arguments const and then propagate that everywhere, so this patch also introduces some more constness. --- ext/phar/phar_internal.h | 14 +++++++------- ext/phar/phar_object.c | 4 ++-- ext/phar/phar_path_check.re | 2 +- ext/phar/stream.c | 21 +++++---------------- ext/phar/util.c | 12 ++++++------ 5 files changed, 21 insertions(+), 32 deletions(-) diff --git a/ext/phar/phar_internal.h b/ext/phar/phar_internal.h index 736226783bef1..3a9af15ca31ee 100644 --- a/ext/phar/phar_internal.h +++ b/ext/phar/phar_internal.h @@ -420,8 +420,8 @@ char *phar_decompress_filter(phar_entry_info * entry, int return_unknown); char *phar_compress_filter(phar_entry_info * entry, int return_unknown); /* void phar_remove_virtual_dirs(phar_archive_data *phar, char *filename, size_t filename_len); */ -void phar_add_virtual_dirs(phar_archive_data *phar, char *filename, size_t filename_len); -zend_result phar_mount_entry(phar_archive_data *phar, char *filename, size_t filename_len, char *path, size_t path_len); +void phar_add_virtual_dirs(phar_archive_data *phar, const char *filename, size_t filename_len); +zend_result phar_mount_entry(phar_archive_data *phar, char *filename, size_t filename_len, const char *path, size_t path_len); zend_string *phar_find_in_include_path(zend_string *file, phar_archive_data **pphar); char *phar_fix_filepath(char *path, size_t *new_len, int use_cwd); phar_entry_info * phar_open_jit(phar_archive_data *phar, phar_entry_info *entry, char **error); @@ -463,10 +463,10 @@ extern HashTable cached_alias; bool phar_archive_delref(phar_archive_data *phar); void phar_entry_delref(phar_entry_data *idata); -phar_entry_info *phar_get_entry_info(phar_archive_data *phar, char *path, size_t path_len, char **error, int security); -phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, char *path, size_t path_len, char dir, char **error, int security); -phar_entry_data *phar_get_or_create_entry_data(char *fname, size_t fname_len, char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security); -zend_result phar_get_entry_data(phar_entry_data **ret, char *fname, size_t fname_len, char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security); +phar_entry_info *phar_get_entry_info(phar_archive_data *phar, const char *path, size_t path_len, char **error, int security); +phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, const char *path, size_t path_len, char dir, char **error, int security); +phar_entry_data *phar_get_or_create_entry_data(char *fname, size_t fname_len, const char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security); +zend_result phar_get_entry_data(phar_entry_data **ret, char *fname, size_t fname_len, const char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security); void phar_flush_ex(phar_archive_data *archive, zend_string *user_stub, bool is_default_stub, char **error); void phar_flush(phar_archive_data *archive, char **error); zend_result phar_detect_phar_fname_ext(const char *filename, size_t filename_len, const char **ext_str, size_t *ext_len, int executable, int for_create, int is_complete); @@ -484,6 +484,6 @@ typedef enum { pcr_err_empty_entry } phar_path_check_result; -phar_path_check_result phar_path_check(char **p, size_t *len, const char **error); +phar_path_check_result phar_path_check(const char **p, size_t *len, const char **error); END_EXTERN_C() diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c index ee94ab7575d5d..c91c16838f893 100644 --- a/ext/phar/phar_object.c +++ b/ext/phar/phar_object.c @@ -1976,7 +1976,7 @@ static zend_result phar_copy_file_contents(phar_entry_info *entry, php_stream *f } /* }}} */ -static zend_object *phar_rename_archive(phar_archive_data **sphar, char *ext) /* {{{ */ +static zend_object *phar_rename_archive(phar_archive_data **sphar, const char *ext) /* {{{ */ { const char *oldname = NULL; phar_archive_data *phar = *sphar; @@ -3484,7 +3484,7 @@ PHP_METHOD(Phar, copy) } size_t tmp_len = ZSTR_LEN(new_file); - char *tmp_new_file = ZSTR_VAL(new_file); + const char *tmp_new_file = ZSTR_VAL(new_file); if (phar_path_check(&tmp_new_file, &tmp_len, &pcr_error) > pcr_is_ok) { zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0, "file \"%s\" contains invalid characters %s, cannot be copied from \"%s\" in phar %s", ZSTR_VAL(new_file), pcr_error, ZSTR_VAL(old_file), phar_obj->archive->fname); diff --git a/ext/phar/phar_path_check.re b/ext/phar/phar_path_check.re index 77aa0b1953d3c..dfc3af5160744 100644 --- a/ext/phar/phar_path_check.re +++ b/ext/phar/phar_path_check.re @@ -18,7 +18,7 @@ #include "phar_internal.h" -phar_path_check_result phar_path_check(char **s, size_t *len, const char **error) +phar_path_check_result phar_path_check(const char **s, size_t *len, const char **error) { const unsigned char *p = (const unsigned char*)*s; const unsigned char *m; diff --git a/ext/phar/stream.c b/ext/phar/stream.c index 0c4b12fd73d53..824caca4235fa 100644 --- a/ext/phar/stream.c +++ b/ext/phar/stream.c @@ -162,7 +162,7 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha { phar_archive_data *phar; phar_entry_data *idata; - char *internal_file; + const char *internal_file; char *error; HashTable *pharcontext; php_url *resource = NULL; @@ -189,7 +189,7 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha phar_request_initialize(); /* strip leading "/" */ - internal_file = estrndup(ZSTR_VAL(resource->path) + 1, ZSTR_LEN(resource->path) - 1); + internal_file = ZSTR_VAL(resource->path) + 1; if (mode[0] == 'w' || (mode[0] == 'r' && mode[1] == '+')) { if (NULL == (idata = phar_get_or_create_entry_data(ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), internal_file, strlen(internal_file), mode, 0, &error, 1))) { if (error) { @@ -198,7 +198,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha } else { php_stream_wrapper_log_error(wrapper, options, "phar error: file \"%s\" could not be created in phar \"%s\"", internal_file, ZSTR_VAL(resource->host)); } - efree(internal_file); php_url_free(resource); return NULL; } @@ -207,7 +206,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha } fpf = php_stream_alloc(&phar_ops, idata, NULL, mode); php_url_free(resource); - efree(internal_file); if (context && Z_TYPE(context->options) != IS_UNDEF && (pzoption = zend_hash_str_find(HASH_OF(&context->options), "phar", sizeof("phar")-1)) != NULL) { pharcontext = HASH_OF(pzoption); @@ -237,7 +235,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha /* retrieve the stub */ if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), NULL, 0, NULL)) { php_stream_wrapper_log_error(wrapper, options, "file %s is not a valid phar archive", ZSTR_VAL(resource->host)); - efree(internal_file); php_url_free(resource); return NULL; } @@ -245,7 +242,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha if ((FAILURE == phar_get_entry_data(&idata, ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), ".phar/stub.php", sizeof(".phar/stub.php")-1, "r", 0, &error, 0)) || !idata) { goto idata_error; } - efree(internal_file); if (opened_path) { *opened_path = strpprintf(MAXPATHLEN, "%s", phar->fname); } @@ -256,7 +252,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha if (stream == NULL) { if (UNEXPECTED(FAILURE == phar_open_archive_fp(phar))) { php_stream_wrapper_log_error(wrapper, options, "phar error: could not reopen phar \"%s\"", ZSTR_VAL(resource->host)); - efree(internal_file); php_url_free(resource); return NULL; } @@ -285,7 +280,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha if (opened_path) { *opened_path = strpprintf(MAXPATHLEN, "%s", phar->fname); } - efree(internal_file); goto phar_stub; } } @@ -298,7 +292,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha } else { php_stream_wrapper_log_error(wrapper, options, "phar error: \"%s\" is not a file in phar \"%s\"", internal_file, ZSTR_VAL(resource->host)); } - efree(internal_file); php_url_free(resource); return NULL; } @@ -317,7 +310,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha php_stream_wrapper_log_error(wrapper, options, "%s", error); efree(error); phar_entry_delref(idata); - efree(internal_file); return NULL; } @@ -340,7 +332,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha if (opened_path) { *opened_path = zend_strpprintf_unchecked(MAXPATHLEN, "phar://%s/%S", idata->phar->fname, idata->internal_file->filename); } - efree(internal_file); phar_stub: fpf = php_stream_alloc(&phar_ops, idata, NULL, mode); return fpf; @@ -662,7 +653,8 @@ static int phar_wrapper_stat(php_stream_wrapper *wrapper, const char *url, int f static int phar_wrapper_unlink(php_stream_wrapper *wrapper, const char *url, int options, php_stream_context *context) /* {{{ */ { php_url *resource; - char *internal_file, *error; + const char *internal_file; + char *error; int internal_file_len; phar_entry_data *idata; phar_archive_data *pphar; @@ -695,7 +687,7 @@ static int phar_wrapper_unlink(php_stream_wrapper *wrapper, const char *url, int } /* need to copy to strip leading "/", will get touched again */ - internal_file = estrndup(ZSTR_VAL(resource->path) + 1, ZSTR_LEN(resource->path) - 1); + internal_file = ZSTR_VAL(resource->path) + 1; internal_file_len = ZSTR_LEN(resource->path) - 1; if (FAILURE == phar_get_entry_data(&idata, ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), internal_file, internal_file_len, "r", 0, &error, 1)) { /* constraints of fp refcount were not met */ @@ -705,7 +697,6 @@ static int phar_wrapper_unlink(php_stream_wrapper *wrapper, const char *url, int } else { php_stream_wrapper_log_error(wrapper, options, "unlink of \"%s\" failed, file does not exist", url); } - efree(internal_file); php_url_free(resource); return 0; } @@ -715,13 +706,11 @@ static int phar_wrapper_unlink(php_stream_wrapper *wrapper, const char *url, int if (idata->internal_file->fp_refcount > 1) { /* more than just our fp resource is open for this file */ php_stream_wrapper_log_error(wrapper, options, "phar error: \"%s\" in phar \"%s\", has open file pointers, cannot unlink", internal_file, ZSTR_VAL(resource->host)); - efree(internal_file); php_url_free(resource); phar_entry_delref(idata); return 0; } php_url_free(resource); - efree(internal_file); phar_entry_remove(idata, &error); if (error) { php_stream_wrapper_log_error(wrapper, options, "%s", error); diff --git a/ext/phar/util.c b/ext/phar/util.c index 06394813574b7..34acaa6939e8a 100644 --- a/ext/phar/util.c +++ b/ext/phar/util.c @@ -189,7 +189,7 @@ int phar_seek_efp(phar_entry_info *entry, zend_off_t offset, int whence, zend_of /* }}} */ /* mount an absolute path or uri to a path internal to the phar archive */ -zend_result phar_mount_entry(phar_archive_data *phar, char *filename, size_t filename_len, char *path, size_t path_len) /* {{{ */ +zend_result phar_mount_entry(phar_archive_data *phar, char *filename, size_t filename_len, const char *path, size_t path_len) /* {{{ */ { phar_entry_info entry = {0}; php_stream_statbuf ssb; @@ -472,7 +472,7 @@ static zend_result phar_separate_entry_fp(phar_entry_info *entry, char **error) * appended, truncated, or read. For read, if the entry is marked unmodified, it is * assumed that the file pointer, if present, is opened for reading */ -zend_result phar_get_entry_data(phar_entry_data **ret, char *fname, size_t fname_len, char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security) /* {{{ */ +zend_result phar_get_entry_data(phar_entry_data **ret, char *fname, size_t fname_len, const char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security) /* {{{ */ { phar_archive_data *phar; phar_entry_info *entry; @@ -632,7 +632,7 @@ zend_result phar_get_entry_data(phar_entry_data **ret, char *fname, size_t fname /** * Create a new dummy file slot within a writeable phar for a newly created file */ -phar_entry_data *phar_get_or_create_entry_data(char *fname, size_t fname_len, char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security) /* {{{ */ +phar_entry_data *phar_get_or_create_entry_data(char *fname, size_t fname_len, const char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security) /* {{{ */ { phar_archive_data *phar; phar_entry_info *entry, etemp; @@ -1234,7 +1234,7 @@ char * phar_decompress_filter(phar_entry_info * entry, int return_unknown) /* {{ /** * retrieve information on a file contained within a phar, or null if it ain't there */ -phar_entry_info *phar_get_entry_info(phar_archive_data *phar, char *path, size_t path_len, char **error, int security) /* {{{ */ +phar_entry_info *phar_get_entry_info(phar_archive_data *phar, const char *path, size_t path_len, char **error, int security) /* {{{ */ { return phar_get_entry_info_dir(phar, path, path_len, 0, error, security); } @@ -1245,7 +1245,7 @@ phar_entry_info *phar_get_entry_info(phar_archive_data *phar, char *path, size_t * valid pre-existing empty directory entries */ // TODO: convert this to use zend_string too -phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, char *path, size_t path_len, char dir, char **error, int security) /* {{{ */ +phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, const char *path, size_t path_len, char dir, char **error, int security) /* {{{ */ { const char *pcr_error; phar_entry_info *entry; @@ -2006,7 +2006,7 @@ zend_result phar_create_signature(phar_archive_data *phar, php_stream *fp, char /* }}} */ // TODO: convert this to zend_string too -void phar_add_virtual_dirs(phar_archive_data *phar, char *filename, size_t filename_len) /* {{{ */ +void phar_add_virtual_dirs(phar_archive_data *phar, const char *filename, size_t filename_len) /* {{{ */ { const char *s; zend_string *str;