Skip to content

Commit 8d28c05

Browse files
GirgiasGirgias
authored
ext/phar: refactor phar_split_fname() to return zend_string* rather than out params (#21777)
1 parent 6031497 commit 8d28c05

File tree

7 files changed

+147
-152
lines changed

7 files changed

+147
-152
lines changed

‎ext/phar/dirstream.c‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -345,21 +345,21 @@ int phar_wrapper_mkdir(php_stream_wrapper *wrapper, const char *url_from, int mo
345345
{
346346
phar_entry_info entry, *e;
347347
phar_archive_data *phar = NULL;
348-
char *error, *arch;
349-
size_t arch_len;
348+
char *error;
350349
php_url *resource = NULL;
351350

352351
/* pre-readonly check, we need to know if this is a data phar */
353-
if (FAILURE == phar_split_fname(url_from, strlen(url_from), &arch, &arch_len, NULL, 2, 2)) {
352+
zend_string *arch = phar_split_fname(url_from, strlen(url_from), NULL, 2, 2);
353+
if (!arch) {
354354
php_stream_wrapper_log_error(wrapper, options, "phar error: cannot create directory \"%s\", no phar archive specified", url_from);
355355
return 0;
356356
}
357357

358-
if (FAILURE == phar_get_archive(&phar, arch, arch_len, NULL, 0, NULL)) {
358+
if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL)) {
359359
phar = NULL;
360360
}
361361

362-
efree(arch);
362+
zend_string_release_ex(arch, false);
363363

364364
if (PHAR_G(readonly) && (!phar || !phar->is_data)) {
365365
php_stream_wrapper_log_error(wrapper, options, "phar error: cannot create directory \"%s\", write operations disabled", url_from);
@@ -471,21 +471,21 @@ int phar_wrapper_rmdir(php_stream_wrapper *wrapper, const char *url, int options
471471
{
472472
phar_entry_info *entry;
473473
phar_archive_data *phar = NULL;
474-
char *error, *arch;
475-
size_t arch_len;
474+
char *error;
476475
php_url *resource = NULL;
477476

478477
/* pre-readonly check, we need to know if this is a data phar */
479-
if (FAILURE == phar_split_fname(url, strlen(url), &arch, &arch_len, NULL, 2, 2)) {
478+
zend_string *arch = phar_split_fname(url, strlen(url), NULL, 2, 2);
479+
if (!arch) {
480480
php_stream_wrapper_log_error(wrapper, options, "phar error: cannot remove directory \"%s\", no phar archive specified, or phar archive does not exist", url);
481481
return 0;
482482
}
483483

484-
if (FAILURE == phar_get_archive(&phar, arch, arch_len, NULL, 0, NULL)) {
484+
if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL)) {
485485
phar = NULL;
486486
}
487487

488-
efree(arch);
488+
zend_string_release_ex(arch, false);
489489

490490
if (PHAR_G(readonly) && (!phar || !phar->is_data)) {
491491
php_stream_wrapper_log_error(wrapper, options, "phar error: cannot rmdir directory \"%s\", write operations disabled", url);

‎ext/phar/func_interceptors.c‎

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ PHP_FUNCTION(phar_opendir) /* {{{ */
3838
}
3939

4040
if (!IS_ABSOLUTE_PATH(filename, filename_len) && !strstr(filename, "://")) {
41-
char *arch;
42-
size_t arch_len;
4341
zend_string *fname = zend_get_executed_filename_ex();
4442

4543
/* we are checking for existence of a file within the relative path. Chances are good that this is
@@ -48,7 +46,8 @@ PHP_FUNCTION(phar_opendir) /* {{{ */
4846
goto skip_phar;
4947
}
5048

51-
if (SUCCESS == phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), &arch, &arch_len, NULL, 2, 0)) {
49+
zend_string *arch = phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), NULL, 2, 0);
50+
if (arch) {
5251
php_stream_context *context = NULL;
5352
php_stream *stream;
5453
char *name;
@@ -58,12 +57,12 @@ PHP_FUNCTION(phar_opendir) /* {{{ */
5857
zend_string *entry = phar_fix_filepath(filename, filename_len, true);
5958

6059
if (ZSTR_VAL(entry)[0] == '/') {
61-
spprintf(&name, 4096, "phar://%s%s", arch, ZSTR_VAL(entry));
60+
spprintf(&name, 4096, "phar://%s%s", ZSTR_VAL(arch), ZSTR_VAL(entry));
6261
} else {
63-
spprintf(&name, 4096, "phar://%s/%s", arch, ZSTR_VAL(entry));
62+
spprintf(&name, 4096, "phar://%s/%s", ZSTR_VAL(arch), ZSTR_VAL(entry));
6463
}
6564
zend_string_release_ex(entry, false);
66-
efree(arch);
65+
zend_string_release_ex(arch, false);
6766
if (zcontext) {
6867
context = php_stream_context_from_zval(zcontext, 0);
6968
}
@@ -84,8 +83,6 @@ PHP_FUNCTION(phar_opendir) /* {{{ */
8483

8584
static zend_string* phar_get_name_for_relative_paths(zend_string *filename, bool using_include_path)
8685
{
87-
char *arch;
88-
size_t arch_len;
8986
zend_string *fname = zend_get_executed_filename_ex();
9087

9188
/* we are checking for existence of a file within the relative path. Chances are good that this is
@@ -94,23 +91,24 @@ static zend_string* phar_get_name_for_relative_paths(zend_string *filename, bool
9491
return NULL;
9592
}
9693

97-
if (FAILURE == phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), &arch, &arch_len, NULL, 2, 0)) {
94+
zend_string *arch = phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), NULL, 2, 0);
95+
if (!arch) {
9896
return NULL;
9997
}
10098

10199
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
102100
/* retrieving a file defaults to within the current directory, so use this if possible */
103101
phar_archive_data *phar;
104-
if (FAILURE == phar_get_archive(&phar, arch, arch_len, NULL, 0, NULL)) {
105-
efree(arch);
102+
if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL)) {
103+
zend_string_release_ex(arch, false);
106104
return NULL;
107105
}
108106

109107
zend_string *name = NULL;
110108
if (using_include_path) {
111109
if (!(name = phar_find_in_include_path(filename, NULL))) {
112110
/* this file is not in the phar, use the original path */
113-
efree(arch);
111+
zend_string_release_ex(arch, false);
114112
return NULL;
115113
}
116114
} else {
@@ -124,24 +122,24 @@ static zend_string* phar_get_name_for_relative_paths(zend_string *filename, bool
124122
/* this file is not in the phar, use the original path */
125123
if (!is_in_phar) {
126124
zend_string_release_ex(entry, false);
127-
efree(arch);
125+
zend_string_release_ex(arch, false);
128126
return NULL;
129127
}
130128
/* auto-convert to phar:// */
131129
if (ZSTR_VAL(entry)[0] == '/') {
132-
ZEND_ASSERT(strlen("phar://") + arch_len + ZSTR_LEN(entry) < 4096);
130+
ZEND_ASSERT(strlen("phar://") + ZSTR_LEN(arch) + ZSTR_LEN(entry) < 4096);
133131
name = zend_string_concat3(
134132
"phar://", strlen("phar://"),
135-
arch, arch_len,
133+
ZSTR_VAL(arch), ZSTR_LEN(arch),
136134
ZSTR_VAL(entry), ZSTR_LEN(entry)
137135
);
138136
} else {
139-
name = strpprintf(4096, "phar://%s/%s", arch, ZSTR_VAL(entry));
137+
name = strpprintf(4096, "phar://%s/%s", ZSTR_VAL(arch), ZSTR_VAL(entry));
140138
}
141139
zend_string_release_ex(entry, false);
142140
}
143141

144-
efree(arch);
142+
zend_string_release_ex(arch, false);
145143
return name;
146144
}
147145

@@ -492,12 +490,12 @@ static void phar_file_stat(const char *filename, size_t filename_length, int typ
492490
phar = PHAR_G(last_phar);
493491
goto splitted;
494492
}
495-
char *arch;
496-
size_t arch_len;
497-
if (SUCCESS == phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), &arch, &arch_len, NULL, 2, 0)) {
493+
494+
zend_string *arch = phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), NULL, 2, 0);
495+
if (arch) {
498496
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
499-
zend_result has_archive = phar_get_archive(&phar, arch, arch_len, NULL, 0, NULL);
500-
efree(arch);
497+
zend_result has_archive = phar_get_archive(&phar, ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL);
498+
zend_string_release_ex(arch, false);
501499
if (FAILURE == has_archive) {
502500
goto skip_phar;
503501
}
@@ -721,8 +719,6 @@ PHP_FUNCTION(phar_is_file) /* {{{ */
721719
goto skip_phar;
722720
}
723721
if (!IS_ABSOLUTE_PATH(filename, filename_len) && !strstr(filename, "://")) {
724-
char *arch;
725-
size_t arch_len;
726722
zend_string *fname = zend_get_executed_filename_ex();
727723

728724
/* we are checking for existence of a file within the relative path. Chances are good that this is
@@ -731,12 +727,15 @@ PHP_FUNCTION(phar_is_file) /* {{{ */
731727
goto skip_phar;
732728
}
733729

734-
if (SUCCESS == phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), &arch, &arch_len, NULL, 2, 0)) {
730+
zend_string *arch = phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), NULL, 2, 0);
731+
if (arch) {
735732
phar_archive_data *phar;
736733

737734
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
738735
/* retrieving a file within the current directory, so use this if possible */
739-
if (SUCCESS == phar_get_archive(&phar, arch, arch_len, NULL, 0, NULL)) {
736+
zend_result has_archive = phar_get_archive(&phar, ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL);
737+
zend_string_release_ex(arch, false);
738+
if (has_archive == SUCCESS) {
740739
phar_entry_info *etemp;
741740

742741
zend_string *entry = phar_fix_filepath(filename, filename_len, true);
@@ -747,12 +746,10 @@ PHP_FUNCTION(phar_is_file) /* {{{ */
747746
}
748747
zend_string_release_ex(entry, false);
749748
if (etemp) {
750-
efree(arch);
751749
RETURN_BOOL(!etemp->is_dir);
752750
}
753751
/* this file is not in the current directory, use the original path */
754752
}
755-
efree(arch);
756753
RETURN_FALSE;
757754
}
758755
}
@@ -779,8 +776,6 @@ PHP_FUNCTION(phar_is_link) /* {{{ */
779776
goto skip_phar;
780777
}
781778
if (!IS_ABSOLUTE_PATH(filename, filename_len) && !strstr(filename, "://")) {
782-
char *arch;
783-
size_t arch_len;
784779
zend_string *fname = zend_get_executed_filename_ex();
785780

786781
/* we are checking for existence of a file within the relative path. Chances are good that this is
@@ -789,12 +784,15 @@ PHP_FUNCTION(phar_is_link) /* {{{ */
789784
goto skip_phar;
790785
}
791786

792-
if (SUCCESS == phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), &arch, &arch_len, NULL, 2, 0)) {
787+
zend_string *arch = phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), NULL, 2, 0);
788+
if (arch) {
793789
phar_archive_data *phar;
794790

795791
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
796792
/* retrieving a file within the current directory, so use this if possible */
797-
if (SUCCESS == phar_get_archive(&phar, arch, arch_len, NULL, 0, NULL)) {
793+
zend_result has_archive = phar_get_archive(&phar, ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL);
794+
zend_string_release_ex(arch, false);
795+
if (has_archive == SUCCESS) {
798796
phar_entry_info *etemp;
799797

800798
zend_string *entry = phar_fix_filepath(filename, filename_len, true);
@@ -805,11 +803,9 @@ PHP_FUNCTION(phar_is_link) /* {{{ */
805803
}
806804
zend_string_release_ex(entry, false);
807805
if (etemp) {
808-
efree(arch);
809806
RETURN_BOOL(etemp->link);
810807
}
811808
}
812-
efree(arch);
813809
RETURN_FALSE;
814810
}
815811
}

‎ext/phar/phar.c‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2164,16 +2164,19 @@ zend_string* phar_fix_filepath(const char *path, size_t path_length, bool use_cw
21642164
*
21652165
* This is used by phar_parse_url()
21662166
*/
2167-
zend_result phar_split_fname(const char *filename, size_t filename_len, char **arch, size_t *arch_len, zend_string **entry, int executable, int for_create) /* {{{ */
2167+
zend_string* phar_split_fname_ex(const char *filename, size_t filename_len, zend_string **entry, int executable, int for_create, const char **error) /* {{{ */
21682168
{
21692169
const char *ext_str;
21702170
#ifdef PHP_WIN32
21712171
char *save;
21722172
#endif
21732173
size_t ext_len;
21742174

2175+
if (error) {
2176+
*error = NULL;
2177+
}
21752178
if (zend_char_has_nul_byte(filename, filename_len)) {
2176-
return FAILURE;
2179+
return NULL;
21772180
}
21782181

21792182
if (!strncasecmp(filename, "phar://", 7)) {
@@ -2191,12 +2194,12 @@ zend_result phar_split_fname(const char *filename, size_t filename_len, char **a
21912194
#endif
21922195
if (phar_detect_phar_fname_ext(filename, filename_len, &ext_str, &ext_len, executable, for_create, false) == FAILURE) {
21932196
if (ext_len != -1) {
2194-
if (!ext_str) {
2197+
if (!ext_str && error) {
21952198
/* no / detected, restore arch for error message */
21962199
#ifdef PHP_WIN32
2197-
*arch = save;
2200+
*error = save;
21982201
#else
2199-
*arch = (char*)filename;
2202+
*error = filename;
22002203
#endif
22012204
}
22022205

@@ -2205,19 +2208,19 @@ zend_result phar_split_fname(const char *filename, size_t filename_len, char **a
22052208
efree((char *)filename);
22062209
}
22072210
#endif
2208-
return FAILURE;
2211+
return NULL;
22092212
}
22102213

22112214
ext_len = 0;
22122215
/* no extension detected - instead we are dealing with an alias */
22132216
}
22142217

2215-
*arch_len = ext_str - filename + ext_len;
2216-
*arch = estrndup(filename, *arch_len);
2218+
size_t arch_len = ext_str - filename + ext_len;
2219+
zend_string *arch = zend_string_init(filename, arch_len, false);
22172220

22182221
if (entry) {
22192222
if (ext_str[ext_len]) {
2220-
size_t computed_entry_len = filename_len - *arch_len;
2223+
size_t computed_entry_len = filename_len - arch_len;
22212224
/* We don't need to unixify the path on Windows,
22222225
* as ext_str is derived from filename that was already unixify */
22232226
*entry = phar_fix_filepath(ext_str+ext_len, computed_entry_len, false);
@@ -2232,10 +2235,14 @@ zend_result phar_split_fname(const char *filename, size_t filename_len, char **a
22322235
}
22332236
#endif
22342237

2235-
return SUCCESS;
2238+
return arch;
22362239
}
22372240
/* }}} */
22382241

2242+
zend_string* phar_split_fname(const char *filename, size_t filename_len, zend_string **entry, int executable, int for_create) {
2243+
return phar_split_fname_ex(filename, filename_len, entry, executable, for_create, NULL);
2244+
}
2245+
22392246
/**
22402247
* Invoked when a user calls Phar::mapPhar() from within an executing .phar
22412248
* to set up its manifest directly

‎ext/phar/phar_internal.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,8 @@ ZEND_ATTRIBUTE_NONNULL zend_result phar_get_entry_data(phar_entry_data **ret, co
474474
ZEND_ATTRIBUTE_NONNULL_ARGS(1, 4) int phar_flush_ex(phar_archive_data *archive, zend_string *user_stub, bool is_default_stub, char **error);
475475
ZEND_ATTRIBUTE_NONNULL int phar_flush(phar_archive_data *archive, char **error);
476476
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, bool is_complete);
477-
zend_result phar_split_fname(const char *filename, size_t filename_len, char **arch, size_t *arch_len, zend_string **entry, int executable, int for_create);
477+
zend_string* phar_split_fname_ex(const char *filename, size_t filename_len, zend_string **entry, int executable, int for_create, const char **error);
478+
zend_string* phar_split_fname(const char *filename, size_t filename_len, zend_string **entry, int executable, int for_create);
478479

479480
typedef enum {
480481
pcr_use_query,

0 commit comments

Comments
 (0)