Skip to content

Commit c6d348b

Browse files
GirgiasGirgias
committed
ext/phar: refactor phar_split_fname() to return zend_string* rather than out params
1 parent c3d80f7 commit c6d348b

File tree

7 files changed

+147
-156
lines changed

7 files changed

+147
-156
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, NULL, 2, 2)) {
352+
zend_string *arch = phar_split_fname(url_from, strlen(url_from), NULL, 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, NULL, 2, 2)) {
478+
zend_string *arch = phar_split_fname(url, strlen(url), NULL, 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: 31 additions & 41 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, NULL, 2, 0)) {
49+
zend_string *arch = phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), NULL, 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, NULL, 2, 0)) {
94+
zend_string *arch = phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), NULL, 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

@@ -475,8 +473,7 @@ static void phar_file_stat(const char *filename, size_t filename_length, int typ
475473
}
476474

477475
if (!IS_ABSOLUTE_PATH(filename, filename_length) && !strstr(filename, "://")) {
478-
char *arch;
479-
size_t arch_len;
476+
zend_string *arch = NULL;
480477
zend_string *fname;
481478
zend_stat_t sb = {0};
482479
phar_archive_data *phar;
@@ -490,16 +487,16 @@ static void phar_file_stat(const char *filename, size_t filename_length, int typ
490487
}
491488

492489
if (PHAR_G(last_phar) && ZSTR_LEN(fname) - 7 >= PHAR_G(last_phar_name_len) && !memcmp(ZSTR_VAL(fname) + 7, PHAR_G(last_phar_name), PHAR_G(last_phar_name_len))) {
493-
arch = estrndup(PHAR_G(last_phar_name), PHAR_G(last_phar_name_len));
494-
arch_len = PHAR_G(last_phar_name_len);
495490
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
496491
phar = PHAR_G(last_phar);
497492
goto splitted;
498493
}
499-
if (SUCCESS == phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), &arch, &arch_len, NULL, NULL, 2, 0)) {
494+
arch = phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), NULL, NULL, 2, 0);
495+
if (arch) {
500496
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
501-
if (FAILURE == phar_get_archive(&phar, arch, arch_len, NULL, 0, NULL)) {
502-
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);
499+
if (has_archive == FAILURE) {
503500
goto skip_phar;
504501
}
505502
splitted:
@@ -520,7 +517,6 @@ static void phar_file_stat(const char *filename, size_t filename_length, int typ
520517
}
521518
if (zend_hash_exists(&(phar->virtual_dirs), entry)) {
522519
zend_string_release_ex(entry, false);
523-
efree(arch);
524520
if (IS_EXISTS_CHECK(type)) {
525521
RETURN_TRUE;
526522
}
@@ -550,7 +546,6 @@ static void phar_file_stat(const char *filename, size_t filename_length, int typ
550546
PHAR_G(cwd_len) = save_len;
551547
zend_string_release_ex(entry, false);
552548
if (IS_EXISTS_CHECK(type)) {
553-
efree(arch);
554549
RETURN_TRUE;
555550
}
556551
goto stat_entry;
@@ -559,7 +554,6 @@ static void phar_file_stat(const char *filename, size_t filename_length, int typ
559554
PHAR_G(cwd) = save;
560555
PHAR_G(cwd_len) = save_len;
561556
zend_string_release_ex(entry, false);
562-
efree(arch);
563557
if (IS_EXISTS_CHECK(type)) {
564558
RETURN_TRUE;
565559
}
@@ -574,15 +568,13 @@ static void phar_file_stat(const char *filename, size_t filename_length, int typ
574568
PHAR_G(cwd) = save;
575569
PHAR_G(cwd_len) = save_len;
576570
zend_string_release_ex(entry, false);
577-
efree(arch);
578571
/* Error Occurred */
579572
if (!IS_EXISTS_CHECK(type)) {
580573
php_error_docref(NULL, E_WARNING, "%sstat failed for %s", IS_LINK_OPERATION(type) ? "L" : "", filename);
581574
}
582575
RETURN_FALSE;
583576
}
584577
stat_entry:
585-
efree(arch);
586578
if (!data->is_dir) {
587579
sb.st_size = data->uncompressed_filesize;
588580
sb.st_mode = data->flags & PHAR_ENT_PERM_MASK;
@@ -727,8 +719,6 @@ PHP_FUNCTION(phar_is_file) /* {{{ */
727719
goto skip_phar;
728720
}
729721
if (!IS_ABSOLUTE_PATH(filename, filename_len) && !strstr(filename, "://")) {
730-
char *arch;
731-
size_t arch_len;
732722
zend_string *fname = zend_get_executed_filename_ex();
733723

734724
/* we are checking for existence of a file within the relative path. Chances are good that this is
@@ -737,12 +727,15 @@ PHP_FUNCTION(phar_is_file) /* {{{ */
737727
goto skip_phar;
738728
}
739729

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

743734
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
744735
/* retrieving a file within the current directory, so use this if possible */
745-
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) {
746739
phar_entry_info *etemp;
747740

748741
zend_string *entry = phar_fix_filepath(filename, filename_len, true);
@@ -753,12 +746,10 @@ PHP_FUNCTION(phar_is_file) /* {{{ */
753746
}
754747
zend_string_release_ex(entry, false);
755748
if (etemp) {
756-
efree(arch);
757749
RETURN_BOOL(!etemp->is_dir);
758750
}
759751
/* this file is not in the current directory, use the original path */
760752
}
761-
efree(arch);
762753
RETURN_FALSE;
763754
}
764755
}
@@ -785,8 +776,6 @@ PHP_FUNCTION(phar_is_link) /* {{{ */
785776
goto skip_phar;
786777
}
787778
if (!IS_ABSOLUTE_PATH(filename, filename_len) && !strstr(filename, "://")) {
788-
char *arch;
789-
size_t arch_len;
790779
zend_string *fname = zend_get_executed_filename_ex();
791780

792781
/* we are checking for existence of a file within the relative path. Chances are good that this is
@@ -795,12 +784,15 @@ PHP_FUNCTION(phar_is_link) /* {{{ */
795784
goto skip_phar;
796785
}
797786

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

801791
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
802792
/* retrieving a file within the current directory, so use this if possible */
803-
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) {
804796
phar_entry_info *etemp;
805797

806798
zend_string *entry = phar_fix_filepath(filename, filename_len, true);
@@ -811,11 +803,9 @@ PHP_FUNCTION(phar_is_link) /* {{{ */
811803
}
812804
zend_string_release_ex(entry, false);
813805
if (etemp) {
814-
efree(arch);
815806
RETURN_BOOL(etemp->link);
816807
}
817808
}
818-
efree(arch);
819809
RETURN_FALSE;
820810
}
821811
}

‎ext/phar/phar.c‎

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

2174+
if (error) {
2175+
*error = NULL;
2176+
}
21742177
if (zend_char_has_nul_byte(filename, filename_len)) {
2175-
return FAILURE;
2178+
return NULL;
21762179
}
21772180

21782181
if (!strncasecmp(filename, "phar://", 7)) {
@@ -2190,12 +2193,12 @@ zend_result phar_split_fname(const char *filename, size_t filename_len, char **a
21902193
#endif
21912194
if (phar_detect_phar_fname_ext(filename, filename_len, &ext_str, &ext_len, executable, for_create, false) == FAILURE) {
21922195
if (ext_len != -1) {
2193-
if (!ext_str) {
2196+
if (!ext_str && error) {
21942197
/* no / detected, restore arch for error message */
21952198
#ifdef PHP_WIN32
2196-
*arch = save;
2199+
*error = save;
21972200
#else
2198-
*arch = (char*)filename;
2201+
*error = filename;
21992202
#endif
22002203
}
22012204

@@ -2204,19 +2207,19 @@ zend_result phar_split_fname(const char *filename, size_t filename_len, char **a
22042207
efree((char *)filename);
22052208
}
22062209
#endif
2207-
return FAILURE;
2210+
return NULL;
22082211
}
22092212

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

2214-
*arch_len = ext_str - filename + ext_len;
2215-
*arch = estrndup(filename, *arch_len);
2217+
size_t arch_len = ext_str - filename + ext_len;
2218+
zend_string *arch = zend_string_init(filename, arch_len, false);
22162219

22172220
if (entry) {
22182221
if (ext_str[ext_len]) {
2219-
size_t computed_entry_len = filename_len - *arch_len;
2222+
size_t computed_entry_len = filename_len - arch_len;
22202223
#ifdef PHP_WIN32
22212224
/* TODO: can we handle the unixify path in phar_fix_filepath() directly ? */
22222225
char *fixed_path_for_windows = estrndup(ext_str+ext_len, computed_entry_len)
@@ -2244,10 +2247,14 @@ zend_result phar_split_fname(const char *filename, size_t filename_len, char **a
22442247
}
22452248
#endif
22462249

2247-
return SUCCESS;
2250+
return arch;
22482251
}
22492252
/* }}} */
22502253

2254+
zend_string* phar_split_fname(const char *filename, size_t filename_len, char **entry, size_t *entry_len, int executable, int for_create) {
2255+
return phar_split_fname_ex(filename, filename_len, entry, entry_len, executable, for_create, NULL);
2256+
}
2257+
22512258
/**
22522259
* Invoked when a user calls Phar::mapPhar() from within an executing .phar
22532260
* 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, ch
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, char **entry, size_t *entry_len, int executable, int for_create);
477+
zend_string* phar_split_fname_ex(const char *filename, size_t filename_len, char **entry, size_t *entry_len, int executable, int for_create, const char **error);
478+
zend_string* phar_split_fname(const char *filename, size_t filename_len, char **entry, size_t *entry_len, int executable, int for_create);
478479

479480
typedef enum {
480481
pcr_use_query,

0 commit comments

Comments
 (0)