Skip to content

Commit ebe0109

Browse files
committed
Revert "ext/phar: refactor phar_split_fname() to return zend_string* rather than out params"
This reverts commit 3224499.
1 parent f3dc867 commit ebe0109

File tree

7 files changed

+157
-148
lines changed

7 files changed

+157
-148
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;
348+
char *error, *arch;
349+
size_t arch_len;
349350
php_url *resource = NULL;
350351

351352
/* pre-readonly check, we need to know if this is a data phar */
352-
zend_string *arch = phar_split_fname(url_from, strlen(url_from), NULL, NULL, 2, 2);
353-
if (!arch) {
353+
if (FAILURE == phar_split_fname(url_from, strlen(url_from), &arch, &arch_len, NULL, NULL, 2, 2)) {
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, ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL)) {
358+
if (FAILURE == phar_get_archive(&phar, arch, arch_len, NULL, 0, NULL)) {
359359
phar = NULL;
360360
}
361361

362-
zend_string_release_ex(arch, false);
362+
efree(arch);
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;
474+
char *error, *arch;
475+
size_t arch_len;
475476
php_url *resource = NULL;
476477

477478
/* pre-readonly check, we need to know if this is a data phar */
478-
zend_string *arch = phar_split_fname(url, strlen(url), NULL, NULL, 2, 2);
479-
if (!arch) {
479+
if (FAILURE == phar_split_fname(url, strlen(url), &arch, &arch_len, NULL, NULL, 2, 2)) {
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, ZSTR_VAL(arch), ZSTR_LEN(arch), NULL, 0, NULL)) {
484+
if (FAILURE == phar_get_archive(&phar, arch, arch_len, NULL, 0, NULL)) {
485485
phar = NULL;
486486
}
487487

488-
zend_string_release_ex(arch, false);
488+
efree(arch);
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: 41 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ PHP_FUNCTION(phar_opendir) /* {{{ */
3838
}
3939

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

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

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

5960
if (ZSTR_VAL(entry)[0] == '/') {
60-
spprintf(&name, 4096, "phar://%s%s", ZSTR_VAL(arch), ZSTR_VAL(entry));
61+
spprintf(&name, 4096, "phar://%s%s", arch, ZSTR_VAL(entry));
6162
} else {
62-
spprintf(&name, 4096, "phar://%s/%s", ZSTR_VAL(arch), ZSTR_VAL(entry));
63+
spprintf(&name, 4096, "phar://%s/%s", arch, ZSTR_VAL(entry));
6364
}
6465
zend_string_release_ex(entry, false);
65-
zend_string_release_ex(arch, false);
66+
efree(arch);
6667
if (zcontext) {
6768
context = php_stream_context_from_zval(zcontext, 0);
6869
}
@@ -83,6 +84,8 @@ PHP_FUNCTION(phar_opendir) /* {{{ */
8384

8485
static zend_string* phar_get_name_for_relative_paths(zend_string *filename, bool using_include_path)
8586
{
87+
char *arch;
88+
size_t arch_len;
8689
zend_string *fname = zend_get_executed_filename_ex();
8790

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

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

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

107109
zend_string *name = NULL;
108110
if (using_include_path) {
109111
if (!(name = phar_find_in_include_path(filename, NULL))) {
110112
/* this file is not in the phar, use the original path */
111-
zend_string_release_ex(arch, false);
113+
efree(arch);
112114
return NULL;
113115
}
114116
} else {
@@ -122,24 +124,24 @@ static zend_string* phar_get_name_for_relative_paths(zend_string *filename, bool
122124
/* this file is not in the phar, use the original path */
123125
if (!is_in_phar) {
124126
zend_string_release_ex(entry, false);
125-
zend_string_release_ex(arch, false);
127+
efree(arch);
126128
return NULL;
127129
}
128130
/* auto-convert to phar:// */
129131
if (ZSTR_VAL(entry)[0] == '/') {
130-
ZEND_ASSERT(strlen("phar://") + ZSTR_LEN(arch) + ZSTR_LEN(entry) < 4096);
132+
ZEND_ASSERT(strlen("phar://") + arch_len + ZSTR_LEN(entry) < 4096);
131133
name = zend_string_concat3(
132134
"phar://", strlen("phar://"),
133-
ZSTR_VAL(arch), ZSTR_LEN(arch),
135+
arch, arch_len,
134136
ZSTR_VAL(entry), ZSTR_LEN(entry)
135137
);
136138
} else {
137-
name = strpprintf(4096, "phar://%s/%s", ZSTR_VAL(arch), ZSTR_VAL(entry));
139+
name = strpprintf(4096, "phar://%s/%s", arch, ZSTR_VAL(entry));
138140
}
139141
zend_string_release_ex(entry, false);
140142
}
141143

142-
zend_string_release_ex(arch, false);
144+
efree(arch);
143145
return name;
144146
}
145147

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

475477
if (!IS_ABSOLUTE_PATH(filename, filename_length) && !strstr(filename, "://")) {
476-
zend_string *arch = NULL;
478+
char *arch;
479+
size_t arch_len;
477480
zend_string *fname;
478481
zend_stat_t sb = {0};
479482
phar_archive_data *phar;
@@ -487,16 +490,16 @@ static void phar_file_stat(const char *filename, size_t filename_length, int typ
487490
}
488491

489492
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);
490495
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
491496
phar = PHAR_G(last_phar);
492497
goto splitted;
493498
}
494-
arch = phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), NULL, NULL, 2, 0);
495-
if (arch) {
499+
if (SUCCESS == phar_split_fname(ZSTR_VAL(fname), ZSTR_LEN(fname), &arch, &arch_len, NULL, NULL, 2, 0)) {
496500
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
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) {
501+
if (FAILURE == phar_get_archive(&phar, arch, arch_len, NULL, 0, NULL)) {
502+
efree(arch);
500503
goto skip_phar;
501504
}
502505
splitted:;
@@ -517,6 +520,7 @@ splitted:;
517520
}
518521
if (zend_hash_exists(&(phar->virtual_dirs), entry)) {
519522
zend_string_release_ex(entry, false);
523+
efree(arch);
520524
if (IS_EXISTS_CHECK(type)) {
521525
RETURN_TRUE;
522526
}
@@ -546,6 +550,7 @@ splitted:;
546550
PHAR_G(cwd_len) = save_len;
547551
zend_string_release_ex(entry, false);
548552
if (IS_EXISTS_CHECK(type)) {
553+
efree(arch);
549554
RETURN_TRUE;
550555
}
551556
goto stat_entry;
@@ -554,6 +559,7 @@ splitted:;
554559
PHAR_G(cwd) = save;
555560
PHAR_G(cwd_len) = save_len;
556561
zend_string_release_ex(entry, false);
562+
efree(arch);
557563
if (IS_EXISTS_CHECK(type)) {
558564
RETURN_TRUE;
559565
}
@@ -568,13 +574,15 @@ splitted:;
568574
PHAR_G(cwd) = save;
569575
PHAR_G(cwd_len) = save_len;
570576
zend_string_release_ex(entry, false);
577+
efree(arch);
571578
/* Error Occurred */
572579
if (!IS_EXISTS_CHECK(type)) {
573580
php_error_docref(NULL, E_WARNING, "%sstat failed for %s", IS_LINK_OPERATION(type) ? "L" : "", filename);
574581
}
575582
RETURN_FALSE;
576583
}
577584
stat_entry:
585+
efree(arch);
578586
if (!data->is_dir) {
579587
sb.st_size = data->uncompressed_filesize;
580588
sb.st_mode = data->flags & PHAR_ENT_PERM_MASK;
@@ -719,6 +727,8 @@ PHP_FUNCTION(phar_is_file) /* {{{ */
719727
goto skip_phar;
720728
}
721729
if (!IS_ABSOLUTE_PATH(filename, filename_len) && !strstr(filename, "://")) {
730+
char *arch;
731+
size_t arch_len;
722732
zend_string *fname = zend_get_executed_filename_ex();
723733

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

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

734743
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
735744
/* retrieving a file within the current directory, so use this if possible */
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) {
745+
if (SUCCESS == phar_get_archive(&phar, arch, arch_len, NULL, 0, NULL)) {
739746
phar_entry_info *etemp;
740747

741748
zend_string *entry = phar_fix_filepath(filename, filename_len, true);
@@ -746,10 +753,12 @@ PHP_FUNCTION(phar_is_file) /* {{{ */
746753
}
747754
zend_string_release_ex(entry, false);
748755
if (etemp) {
756+
efree(arch);
749757
RETURN_BOOL(!etemp->is_dir);
750758
}
751759
/* this file is not in the current directory, use the original path */
752760
}
761+
efree(arch);
753762
RETURN_FALSE;
754763
}
755764
}
@@ -776,6 +785,8 @@ PHP_FUNCTION(phar_is_link) /* {{{ */
776785
goto skip_phar;
777786
}
778787
if (!IS_ABSOLUTE_PATH(filename, filename_len) && !strstr(filename, "://")) {
788+
char *arch;
789+
size_t arch_len;
779790
zend_string *fname = zend_get_executed_filename_ex();
780791

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

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

791801
/* fopen within phar, if :// is not in the url, then prepend phar://<archive>/ */
792802
/* retrieving a file within the current directory, so use this if possible */
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) {
803+
if (SUCCESS == phar_get_archive(&phar, arch, arch_len, NULL, 0, NULL)) {
796804
phar_entry_info *etemp;
797805

798806
zend_string *entry = phar_fix_filepath(filename, filename_len, true);
@@ -803,9 +811,11 @@ PHP_FUNCTION(phar_is_link) /* {{{ */
803811
}
804812
zend_string_release_ex(entry, false);
805813
if (etemp) {
814+
efree(arch);
806815
RETURN_BOOL(etemp->link);
807816
}
808817
}
818+
efree(arch);
809819
RETURN_FALSE;
810820
}
811821
}

ext/phar/phar.c

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2163,19 +2163,16 @@ 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_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) /* {{{ */
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) /* {{{ */
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-
}
21772174
if (zend_char_has_nul_byte(filename, filename_len)) {
2178-
return NULL;
2175+
return FAILURE;
21792176
}
21802177

21812178
if (!strncasecmp(filename, "phar://", 7)) {
@@ -2193,12 +2190,12 @@ zend_string* phar_split_fname_ex(const char *filename, size_t filename_len, char
21932190
#endif
21942191
if (phar_detect_phar_fname_ext(filename, filename_len, &ext_str, &ext_len, executable, for_create, false) == FAILURE) {
21952192
if (ext_len != -1) {
2196-
if (!ext_str && error) {
2193+
if (!ext_str) {
21972194
/* no / detected, restore arch for error message */
21982195
#ifdef PHP_WIN32
2199-
*error = save;
2196+
*arch = save;
22002197
#else
2201-
*error = filename;
2198+
*arch = (char*)filename;
22022199
#endif
22032200
}
22042201

@@ -2207,22 +2204,22 @@ zend_string* phar_split_fname_ex(const char *filename, size_t filename_len, char
22072204
efree((char *)filename);
22082205
}
22092206
#endif
2210-
return NULL;
2207+
return FAILURE;
22112208
}
22122209

22132210
ext_len = 0;
22142211
/* no extension detected - instead we are dealing with an alias */
22152212
}
22162213

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

22202217
if (entry) {
22212218
if (ext_str[ext_len]) {
2222-
size_t computed_entry_len = filename_len - arch_len;
2219+
size_t computed_entry_len = filename_len - *arch_len;
22232220
#ifdef PHP_WIN32
22242221
/* TODO: can we handle the unixify path in phar_fix_filepath() directly ? */
2225-
char *fixed_path_for_windows = estrndup(ext_str+ext_len, computed_entry_len);
2222+
char *fixed_path_for_windows = estrndup(ext_str+ext_len, computed_entry_len)
22262223
phar_unixify_path_separators(fixed_path_for_windows, computed_entry_len);
22272224
zend_string *entry_str = phar_fix_filepath(fixed_path_for_windows, computed_entry_len, false);
22282225
*entry = estrndup(ZSTR_VAL(entry_str), ZSTR_LEN(entry_str));
@@ -2247,14 +2244,10 @@ zend_string* phar_split_fname_ex(const char *filename, size_t filename_len, char
22472244
}
22482245
#endif
22492246

2250-
return arch;
2247+
return SUCCESS;
22512248
}
22522249
/* }}} */
22532250

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-
22582251
/**
22592252
* Invoked when a user calls Phar::mapPhar() from within an executing .phar
22602253
* to set up its manifest directly

ext/phar/phar_internal.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -474,8 +474,7 @@ 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_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);
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);
479478

480479
typedef enum {
481480
pcr_use_query,

0 commit comments

Comments
 (0)