Skip to content

Commit b33fee5

Browse files
GirgiasGirgias
authored
ext/phar: refactor phar_entry_info.link field (#21790)
- Rename field to symlink so that what this field represents is clear - Convert it from a char* to a zend_string* - Refactor the implementation of phar_get_link_location() to always return a "new" zend_string* - Refactor the implementation of phar_get_link_source() to make it more legible
1 parent a287430 commit b33fee5

File tree

7 files changed

+61
-60
lines changed

7 files changed

+61
-60
lines changed

‎ext/phar/func_interceptors.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ splitted:;
578578
if (!data->is_dir) {
579579
sb.st_size = data->uncompressed_filesize;
580580
sb.st_mode = data->flags & PHAR_ENT_PERM_MASK;
581-
if (data->link) {
581+
if (data->symlink) {
582582
sb.st_mode |= S_IFREG|S_IFLNK; /* regular file */
583583
} else {
584584
sb.st_mode |= S_IFREG; /* regular file */
@@ -591,7 +591,7 @@ splitted:;
591591
sb.st_size = 0;
592592
sb.st_mode = data->flags & PHAR_ENT_PERM_MASK;
593593
sb.st_mode |= S_IFDIR; /* regular directory */
594-
if (data->link) {
594+
if (data->symlink) {
595595
sb.st_mode |= S_IFLNK;
596596
}
597597
/* timestamp is just the timestamp when this was added to the phar */
@@ -803,7 +803,7 @@ PHP_FUNCTION(phar_is_link) /* {{{ */
803803
}
804804
zend_string_release_ex(entry, false);
805805
if (etemp) {
806-
RETURN_BOOL(etemp->link);
806+
RETURN_BOOL(etemp->symlink);
807807
}
808808
}
809809
RETURN_FALSE;

‎ext/phar/phar.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,9 +374,9 @@ void destroy_phar_manifest_entry_int(phar_entry_info *entry) /* {{{ */
374374

375375
zend_string_release_ex(entry->filename, entry->is_persistent);
376376

377-
if (entry->link) {
378-
pefree(entry->link, entry->is_persistent);
379-
entry->link = 0;
377+
if (entry->symlink) {
378+
zend_string_release_ex(entry->symlink, entry->is_persistent);
379+
entry->symlink = NULL;
380380
}
381381

382382
if (entry->tmp) {

‎ext/phar/phar_internal.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ typedef struct _phar_entry_info {
217217
unsigned int fileinfo_lock_count;
218218
char *tmp;
219219
phar_archive_data *phar;
220-
char *link; /* symbolic link to another file */
220+
zend_string *symlink;
221221
char tar_type;
222222
/* position in the manifest */
223223
uint32_t manifest_pos;

‎ext/phar/phar_object.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1902,7 +1902,7 @@ static zend_result phar_copy_file_contents(phar_entry_info *entry, php_stream *f
19021902
char *error;
19031903
zend_off_t offset;
19041904

1905-
ZEND_ASSERT(!entry->link);
1905+
ZEND_ASSERT(!entry->symlink);
19061906

19071907
if (FAILURE == phar_open_entry_fp(entry, &error, true)) {
19081908
if (error) {
@@ -2243,8 +2243,8 @@ static zend_object *phar_convert_to_other(phar_archive_data *source, int convert
22432243

22442244
newentry = *entry;
22452245

2246-
if (newentry.link) {
2247-
newentry.link = estrdup(newentry.link);
2246+
if (newentry.symlink) {
2247+
zend_string_addref(newentry.symlink);
22482248
goto no_copy;
22492249
}
22502250

‎ext/phar/stream.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ static ssize_t phar_stream_read(php_stream *stream, char *buf, size_t count) /*
368368
ssize_t got;
369369
phar_entry_info *entry;
370370

371-
if (data->internal_file->link) {
371+
if (data->internal_file->symlink) {
372372
entry = phar_get_link_source(data->internal_file);
373373
} else {
374374
entry = data->internal_file;
@@ -400,7 +400,7 @@ static int phar_stream_seek(php_stream *stream, zend_off_t offset, int whence, z
400400
int res;
401401
zend_ulong temp;
402402

403-
if (data->internal_file->link) {
403+
if (data->internal_file->symlink) {
404404
entry = phar_get_link_source(data->internal_file);
405405
} else {
406406
entry = data->internal_file;
@@ -838,7 +838,8 @@ static int phar_wrapper_rename(php_stream_wrapper *wrapper, const char *url_from
838838
entry->is_deleted = 1;
839839
entry->fp = NULL;
840840
ZVAL_UNDEF(&entry->metadata_tracker.val);
841-
entry->link = entry->tmp = NULL;
841+
entry->symlink = NULL;
842+
entry->tmp = NULL;
842843
source = entry;
843844

844845
/* add to the manifest, and then store the pointer to the new guy in entry

‎ext/phar/tar.c‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -492,8 +492,8 @@ zend_result phar_parse_tarfile(php_stream* fp, const char *fname, size_t fname_l
492492
entry.is_dir = 0;
493493
}
494494

495-
entry.link = NULL;
496-
/* link field is null-terminated unless it has 100 non-null chars.
495+
entry.symlink = NULL;
496+
/* linkname field is null-terminated unless it has 100 non-null chars.
497497
* Thus we cannot use strlen. */
498498
linkname_len = zend_strnlen(hdr->linkname, 100);
499499
if (entry.tar_type == TAR_LINK) {
@@ -506,9 +506,9 @@ zend_result phar_parse_tarfile(php_stream* fp, const char *fname, size_t fname_l
506506
phar_destroy_phar_data(myphar);
507507
return FAILURE;
508508
}
509-
entry.link = estrndup(hdr->linkname, linkname_len);
509+
entry.symlink = zend_string_init(hdr->linkname, linkname_len, false);
510510
} else if (entry.tar_type == TAR_SYMLINK) {
511-
entry.link = estrndup(hdr->linkname, linkname_len);
511+
entry.symlink = zend_string_init(hdr->linkname, linkname_len, false);
512512
}
513513
phar_set_inode(&entry);
514514

@@ -779,10 +779,10 @@ static int phar_tar_writeheaders_int(phar_entry_info *entry, void *argument) /*
779779
/* calc checksum */
780780
header.typeflag = entry->tar_type;
781781

782-
if (entry->link) {
783-
if (strlcpy(header.linkname, entry->link, sizeof(header.linkname)) >= sizeof(header.linkname)) {
782+
if (entry->symlink) {
783+
if (strlcpy(header.linkname, ZSTR_VAL(entry->symlink), sizeof(header.linkname)) >= sizeof(header.linkname)) {
784784
if (fp->error) {
785-
spprintf(fp->error, 4096, "tar-based phar \"%s\" cannot be created, link \"%s\" is too long for format", entry->phar->fname, entry->link);
785+
spprintf(fp->error, 4096, "tar-based phar \"%s\" cannot be created, link \"%s\" is too long for format", entry->phar->fname, ZSTR_VAL(entry->symlink));
786786
}
787787
return ZEND_HASH_APPLY_STOP;
788788
}

‎ext/phar/util.c‎

Lines changed: 40 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -38,50 +38,50 @@ static zend_result phar_call_openssl_signverify(bool is_sign, php_stream *fp, ze
3838
#endif
3939

4040
/* for links to relative location, prepend cwd of the entry */
41-
static char *phar_get_link_location(phar_entry_info *entry) /* {{{ */
41+
static zend_string *phar_get_link_location(phar_entry_info *entry) /* {{{ */
4242
{
43-
char *p, *ret = NULL;
43+
ZEND_ASSERT(entry->symlink);
4444

45-
ZEND_ASSERT(entry->link);
46-
47-
if (entry->link[0] == '/') {
48-
return estrdup(entry->link + 1);
45+
if (ZSTR_VAL(entry->symlink)[0] == '/') {
46+
return zend_string_init(ZSTR_VAL(entry->symlink) + 1, ZSTR_LEN(entry->symlink) - 1, false);
4947
}
50-
p = strrchr(ZSTR_VAL(entry->filename), '/');
48+
const char *p = strrchr(ZSTR_VAL(entry->filename), '/');
5149
if (p) {
5250
/* Important: don't modify the original `p` data because it is a shared string. */
5351
zend_string *new_name = zend_string_init(ZSTR_VAL(entry->filename), p - ZSTR_VAL(entry->filename), false);
54-
spprintf(&ret, 0, "%s/%s", ZSTR_VAL(new_name), entry->link);
52+
zend_string *location = zend_string_concat3(
53+
ZSTR_VAL(new_name), ZSTR_LEN(new_name),
54+
ZEND_STRL("/"),
55+
ZSTR_VAL(entry->symlink), ZSTR_LEN(entry->symlink)
56+
);
5557
zend_string_release(entry->filename);
5658
entry->filename = new_name;
57-
return ret;
59+
return location;
5860
}
59-
return entry->link;
61+
return zend_string_copy(entry->symlink);
6062
}
6163
/* }}} */
6264

6365
phar_entry_info *phar_get_link_source(phar_entry_info *entry) /* {{{ */
6466
{
6567
phar_entry_info *link_entry;
66-
char *link;
6768

68-
if (!entry->link) {
69+
if (!entry->symlink) {
6970
return entry;
7071
}
7172

72-
link = phar_get_link_location(entry);
73-
if (NULL != (link_entry = zend_hash_str_find_ptr(&(entry->phar->manifest), entry->link, strlen(entry->link))) ||
74-
NULL != (link_entry = zend_hash_str_find_ptr(&(entry->phar->manifest), link, strlen(link)))) {
75-
if (link != entry->link) {
76-
efree(link);
77-
}
73+
link_entry = zend_hash_find_ptr(&(entry->phar->manifest), entry->symlink);
74+
if (link_entry) {
7875
return phar_get_link_source(link_entry);
79-
} else {
80-
if (link != entry->link) {
81-
efree(link);
82-
}
83-
return NULL;
8476
}
77+
78+
zend_string *link = phar_get_link_location(entry);
79+
link_entry = zend_hash_find_ptr(&(entry->phar->manifest), link);
80+
zend_string_release(link);
81+
if (link_entry) {
82+
return phar_get_link_source(link_entry);
83+
}
84+
return NULL;
8585
}
8686
/* }}} */
8787

@@ -96,7 +96,7 @@ static php_stream *phar_get_entrypufp(const phar_entry_info *entry)
9696
/* retrieve a phar_entry_info's current file pointer for reading contents */
9797
php_stream *phar_get_efp(phar_entry_info *entry, bool follow_links) /* {{{ */
9898
{
99-
if (follow_links && entry->link) {
99+
if (follow_links && entry->symlink) {
100100
phar_entry_info *link_entry = phar_get_link_source(entry);
101101

102102
if (link_entry && link_entry != entry) {
@@ -387,9 +387,9 @@ static ZEND_ATTRIBUTE_NONNULL zend_result phar_create_writeable_entry(phar_archi
387387
}
388388

389389
/* open a new temp file for writing */
390-
if (entry->link) {
391-
efree(entry->link);
392-
entry->link = NULL;
390+
if (entry->symlink) {
391+
zend_string_release(entry->symlink);
392+
entry->symlink = NULL;
393393
entry->tar_type = (entry->is_tar ? TAR_FILE : '\0');
394394
}
395395

@@ -444,9 +444,9 @@ ZEND_ATTRIBUTE_NONNULL static zend_result phar_separate_entry_fp(phar_entry_info
444444
return FAILURE;
445445
}
446446

447-
if (entry->link) {
448-
efree(entry->link);
449-
entry->link = NULL;
447+
if (entry->symlink) {
448+
zend_string_release(entry->symlink);
449+
entry->symlink = NULL;
450450
entry->tar_type = (entry->is_tar ? TAR_FILE : '\0');
451451
}
452452

@@ -559,9 +559,9 @@ ZEND_ATTRIBUTE_NONNULL zend_result phar_get_entry_data(phar_entry_data **ret, co
559559
}
560560
} else {
561561
if (for_write) {
562-
if (entry->link) {
563-
efree(entry->link);
564-
entry->link = NULL;
562+
if (entry->symlink) {
563+
zend_string_release(entry->symlink);
564+
entry->symlink = NULL;
565565
entry->tar_type = (entry->is_tar ? TAR_FILE : '\0');
566566
}
567567

@@ -586,7 +586,7 @@ ZEND_ATTRIBUTE_NONNULL zend_result phar_get_entry_data(phar_entry_data **ret, co
586586
(*ret)->phar = phar;
587587
(*ret)->internal_file = entry;
588588
(*ret)->fp = phar_get_efp(entry, true);
589-
if (entry->link) {
589+
if (entry->symlink) {
590590
phar_entry_info *link = phar_get_link_source(entry);
591591
if(!link) {
592592
efree(*ret);
@@ -740,9 +740,9 @@ ZEND_ATTRIBUTE_NONNULL zend_result phar_copy_entry_fp(phar_entry_info *source, p
740740
return FAILURE;
741741
}
742742

743-
if (dest->link) {
744-
efree(dest->link);
745-
dest->link = NULL;
743+
if (dest->symlink) {
744+
zend_string_release(dest->symlink);
745+
dest->symlink = NULL;
746746
dest->tar_type = (dest->is_tar ? TAR_FILE : '\0');
747747
}
748748

@@ -807,7 +807,7 @@ ZEND_ATTRIBUTE_NONNULL zend_result phar_open_entry_fp(phar_entry_info *entry, ch
807807
php_stream *ufp;
808808
phar_entry_data dummy;
809809

810-
if (follow_links && entry->link) {
810+
if (follow_links && entry->symlink) {
811811
phar_entry_info *link_entry = phar_get_link_source(entry);
812812
if (link_entry && link_entry != entry) {
813813
return phar_open_entry_fp(link_entry, error, true);
@@ -1995,8 +1995,8 @@ static int phar_update_cached_entry(zval *data, void *argument) /* {{{ */
19951995

19961996
entry->phar = (phar_archive_data *)argument;
19971997

1998-
if (entry->link) {
1999-
entry->link = estrdup(entry->link);
1998+
if (entry->symlink) {
1999+
zend_string_addref(entry->symlink);
20002000
}
20012001

20022002
if (entry->tmp) {

0 commit comments

Comments
 (0)