Skip to content

Commit 880ea80

Browse files
TysonAndreTysonAndre
committed
Add additional assertions about usage of persistent phars
1 parent a9178eb commit 880ea80

File tree

6 files changed

+47
-38
lines changed

6 files changed

+47
-38
lines changed

‎ext/phar/phar.c

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ void phar_destroy_phar_data(phar_archive_data *phar) /* {{{ */
229229
HT_INVALIDATE(&phar->virtual_dirs);
230230
}
231231

232-
phar_metadata_tracker_free(&phar->metadata_tracker);
232+
phar_metadata_tracker_free(&phar->metadata_tracker, phar->is_persistent);
233233

234234
if (phar->fp) {
235235
php_stream_close(phar->fp);
@@ -370,7 +370,7 @@ void destroy_phar_manifest_entry_int(phar_entry_info *entry) /* {{{ */
370370
entry->fp = 0;
371371
}
372372

373-
phar_metadata_tracker_free(&entry->metadata_tracker);
373+
phar_metadata_tracker_free(&entry->metadata_tracker, entry->is_persistent);
374374

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

@@ -580,6 +580,9 @@ void phar_metadata_tracker_try_ensure_has_serialized_data(phar_metadata_tracker
580580
/* Already has serialized the value or there is no value */
581581
return;
582582
}
583+
/* Assert it should not be possible to create raw zvals in a persistent phar (i.e. from cache_list) */
584+
ZEND_ASSERT(!persistent);
585+
583586
PHP_VAR_SERIALIZE_INIT(metadata_hash);
584587
php_var_serialize(&metadata_str, &tracker->val, &metadata_hash);
585588
PHP_VAR_SERIALIZE_DESTROY(metadata_hash);
@@ -600,7 +603,9 @@ void phar_metadata_tracker_try_ensure_has_serialized_data(phar_metadata_tracker
600603
*/
601604
int phar_metadata_tracker_unserialize_or_copy(const phar_metadata_tracker *tracker, zval *metadata, int persistent) /* {{{ */
602605
{
603-
if (Z_TYPE(tracker->val) == IS_UNDEF || persistent) {
606+
if (Z_ISUNDEF(tracker->val) || persistent) {
607+
/* Assert it should not be possible to create raw data in a persistent phar (i.e. from cache_list) */
608+
ZEND_ASSERT(Z_ISUNDEF(tracker->val));
604609
const unsigned char *start;
605610
php_unserialize_data_t var_hash;
606611
/* precondition: This has data */
@@ -629,18 +634,20 @@ int phar_metadata_tracker_unserialize_or_copy(const phar_metadata_tracker *track
629634
/**
630635
* Check if this has any data, serialized or as a raw value.
631636
*/
632-
zend_bool phar_metadata_tracker_has_data(const phar_metadata_tracker *tracker) /* {{{ */
637+
zend_bool phar_metadata_tracker_has_data(const phar_metadata_tracker *tracker, int persistent) /* {{{ */
633638
{
634-
return Z_TYPE(tracker->val) != IS_UNDEF || tracker->str != NULL;
639+
ZEND_ASSERT(!persistent || Z_ISUNDEF(tracker->val));
640+
return !Z_ISUNDEF(tracker->val) || tracker->str != NULL;
635641
}
636642
/* }}} */
637643

638644
/**
639645
* Free memory used to track the metadata and set all fields to be null/undef.
640646
*/
641-
void phar_metadata_tracker_free(phar_metadata_tracker *tracker) /* {{{ */
647+
void phar_metadata_tracker_free(phar_metadata_tracker *tracker, int persistent) /* {{{ */
642648
{
643-
if (Z_TYPE(tracker->val) != IS_UNDEF) {
649+
if (!Z_ISUNDEF(tracker->val)) {
650+
ZEND_ASSERT(!persistent);
644651
zval_ptr_dtor(&tracker->val);
645652
ZVAL_UNDEF(&tracker->val);
646653
}
@@ -654,12 +661,13 @@ void phar_metadata_tracker_free(phar_metadata_tracker *tracker) /* {{{ */
654661
/**
655662
* Free memory used to track the metadata and set all fields to be null/undef.
656663
*/
657-
void phar_metadata_tracker_copy(phar_metadata_tracker *dest, const phar_metadata_tracker *source) /* {{{ */
664+
void phar_metadata_tracker_copy(phar_metadata_tracker *dest, const phar_metadata_tracker *source, int persistent) /* {{{ */
658665
{
659666
ZEND_ASSERT(dest != source);
660-
phar_metadata_tracker_free(dest);
667+
phar_metadata_tracker_free(dest, persistent);
661668

662-
if (Z_TYPE(source->val) != IS_UNDEF) {
669+
if (!Z_ISUNDEF(source->val)) {
670+
ZEND_ASSERT(!persistent);
663671
ZVAL_COPY(&dest->val, &source->val);
664672
}
665673
if (source->str) {
@@ -690,7 +698,7 @@ void phar_metadata_tracker_clone(phar_metadata_tracker *tracker) /* {{{ */
690698
*/
691699
void phar_parse_metadata_lazy(const char *buffer, phar_metadata_tracker *tracker, uint32_t zip_metadata_len, int persistent) /* {{{ */
692700
{
693-
phar_metadata_tracker_free(tracker);
701+
phar_metadata_tracker_free(tracker, persistent);
694702
if (zip_metadata_len) {
695703
/* lazy init metadata */
696704
tracker->str = zend_string_init(buffer, zip_metadata_len, persistent);
@@ -1183,21 +1191,21 @@ static int phar_parse_pharfile(php_stream *fp, char *fname, size_t fname_len, ch
11831191
switch (entry.flags & PHAR_ENT_COMPRESSION_MASK) {
11841192
case PHAR_ENT_COMPRESSED_GZ:
11851193
if (!PHAR_G(has_zlib)) {
1186-
phar_metadata_tracker_free(&entry.metadata_tracker);
1194+
phar_metadata_tracker_free(&entry.metadata_tracker, entry.is_persistent);
11871195
pefree(entry.filename, entry.is_persistent);
11881196
MAPPHAR_FAIL("zlib extension is required for gz compressed .phar file \"%s\"");
11891197
}
11901198
break;
11911199
case PHAR_ENT_COMPRESSED_BZ2:
11921200
if (!PHAR_G(has_bz2)) {
1193-
phar_metadata_tracker_free(&entry.metadata_tracker);
1201+
phar_metadata_tracker_free(&entry.metadata_tracker, entry.is_persistent);
11941202
pefree(entry.filename, entry.is_persistent);
11951203
MAPPHAR_FAIL("bz2 extension is required for bzip2 compressed .phar file \"%s\"");
11961204
}
11971205
break;
11981206
default:
11991207
if (entry.uncompressed_filesize != entry.compressed_filesize) {
1200-
phar_metadata_tracker_free(&entry.metadata_tracker);
1208+
phar_metadata_tracker_free(&entry.metadata_tracker, entry.is_persistent);
12011209
pefree(entry.filename, entry.is_persistent);
12021210
MAPPHAR_FAIL("internal corruption of phar \"%s\" (compressed and uncompressed size does not match for uncompressed entry)");
12031211
}
@@ -2744,7 +2752,8 @@ int phar_flush(phar_archive_data *phar, char *user_stub, zend_long len, int conv
27442752
/* we use this to calculate API version, 1.1.1 is used for phars with directories */
27452753
has_dirs = 1;
27462754
}
2747-
if (Z_TYPE(entry->metadata_tracker.val) != IS_UNDEF && !entry->metadata_tracker.str) {
2755+
if (!Z_ISUNDEF(entry->metadata_tracker.val) && !entry->metadata_tracker.str) {
2756+
ZEND_ASSERT(!entry->is_persistent);
27482757
/* Assume serialization will succeed. TODO: Set error and throw if EG(exception) != NULL */
27492758
smart_str buf = {0};
27502759
PHP_VAR_SERIALIZE_INIT(metadata_hash);

‎ext/phar/phar_internal.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -553,10 +553,10 @@ zend_string *phar_find_in_include_path(char *file, size_t file_len, phar_archive
553553
char *phar_fix_filepath(char *path, size_t *new_len, int use_cwd);
554554
phar_entry_info * phar_open_jit(phar_archive_data *phar, phar_entry_info *entry, char **error);
555555
void phar_parse_metadata_lazy(const char *buffer, phar_metadata_tracker *tracker, uint32_t zip_metadata_len, int persistent);
556+
zend_bool phar_metadata_tracker_has_data(const phar_metadata_tracker* tracker, int persistent);
556557
/* If this has data, free it and set all values to undefined. */
557-
zend_bool phar_metadata_tracker_has_data(const phar_metadata_tracker* tracker);
558-
void phar_metadata_tracker_free(phar_metadata_tracker* val);
559-
void phar_metadata_tracker_copy(phar_metadata_tracker* dest, const phar_metadata_tracker *source);
558+
void phar_metadata_tracker_free(phar_metadata_tracker* val, int persistent);
559+
void phar_metadata_tracker_copy(phar_metadata_tracker* dest, const phar_metadata_tracker *source, int persistent);
560560
void phar_metadata_tracker_clone(phar_metadata_tracker* tracker);
561561
void phar_metadata_tracker_try_ensure_has_serialized_data(phar_metadata_tracker* tracker, int persistent);
562562
int phar_metadata_tracker_unserialize_or_copy(const phar_metadata_tracker* tracker, zval *value, int persistent);

‎ext/phar/phar_object.c

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2266,7 +2266,7 @@ static zend_object *phar_convert_to_other(phar_archive_data *source, int convert
22662266
phar->is_temporary_alias = source->is_temporary_alias;
22672267
phar->alias = source->alias;
22682268

2269-
phar_metadata_tracker_copy(&phar->metadata_tracker, &source->metadata_tracker);
2269+
phar_metadata_tracker_copy(&phar->metadata_tracker, &source->metadata_tracker, phar->is_persistent);
22702270

22712271
/* first copy each file's uncompressed contents to a temporary file and set per-file flags */
22722272
ZEND_HASH_FOREACH_PTR(&source->manifest, entry) {
@@ -3940,7 +3940,7 @@ PHP_METHOD(Phar, hasMetadata)
39403940
RETURN_THROWS();
39413941
}
39423942

3943-
RETURN_BOOL(phar_metadata_tracker_has_data(&phar_obj->archive->metadata_tracker));
3943+
RETURN_BOOL(phar_metadata_tracker_has_data(&phar_obj->archive->metadata_tracker, phar_obj->archive->is_persistent));
39443944
}
39453945
/* }}} */
39463946

@@ -3955,7 +3955,7 @@ PHP_METHOD(Phar, getMetadata)
39553955
}
39563956

39573957
tracker = &phar_obj->archive->metadata_tracker;
3958-
if (phar_metadata_tracker_has_data(tracker)) {
3958+
if (phar_metadata_tracker_has_data(tracker, phar_obj->archive->is_persistent)) {
39593959
/* assume success, we would have failed before */
39603960
phar_metadata_tracker_unserialize_or_copy(tracker, return_value, phar_obj->archive->is_persistent);
39613961
}
@@ -3983,7 +3983,7 @@ PHP_METHOD(Phar, setMetadata)
39833983
zend_throw_exception_ex(phar_ce_PharException, 0, "phar \"%s\" is persistent, unable to copy on write", phar_obj->archive->fname);
39843984
RETURN_THROWS();
39853985
}
3986-
phar_metadata_tracker_free(&phar_obj->archive->metadata_tracker);
3986+
phar_metadata_tracker_free(&phar_obj->archive->metadata_tracker, phar_obj->archive->is_persistent);
39873987

39883988
ZVAL_COPY(&phar_obj->archive->metadata_tracker.val, metadata);
39893989
phar_obj->archive->is_modified = 1;
@@ -4012,11 +4012,11 @@ PHP_METHOD(Phar, delMetadata)
40124012
RETURN_THROWS();
40134013
}
40144014

4015-
if (!phar_metadata_tracker_has_data(&phar_obj->archive->metadata_tracker)) {
4015+
if (!phar_metadata_tracker_has_data(&phar_obj->archive->metadata_tracker, phar_obj->archive->is_persistent)) {
40164016
RETURN_TRUE;
40174017
}
40184018

4019-
phar_metadata_tracker_free(&phar_obj->archive->metadata_tracker);
4019+
phar_metadata_tracker_free(&phar_obj->archive->metadata_tracker, phar_obj->archive->is_persistent);
40204020
phar_obj->archive->is_modified = 1;
40214021
phar_flush(phar_obj->archive, 0, 0, 0, &error);
40224022

@@ -4606,7 +4606,7 @@ PHP_METHOD(PharFileInfo, hasMetadata)
46064606
RETURN_THROWS();
46074607
}
46084608

4609-
RETURN_BOOL(phar_metadata_tracker_has_data(&entry_obj->entry->metadata_tracker));
4609+
RETURN_BOOL(phar_metadata_tracker_has_data(&entry_obj->entry->metadata_tracker, entry_obj->entry->is_persistent));
46104610
}
46114611
/* }}} */
46124612

@@ -4621,7 +4621,7 @@ PHP_METHOD(PharFileInfo, getMetadata)
46214621
}
46224622

46234623
tracker = &entry_obj->entry->metadata_tracker;
4624-
if (phar_metadata_tracker_has_data(tracker)) {
4624+
if (phar_metadata_tracker_has_data(tracker, entry_obj->entry->is_persistent)) {
46254625
phar_metadata_tracker_unserialize_or_copy(tracker, return_value, entry_obj->entry->is_persistent);
46264626
}
46274627
}
@@ -4660,7 +4660,7 @@ PHP_METHOD(PharFileInfo, setMetadata)
46604660
/* re-populate after copy-on-write */
46614661
entry_obj->entry = zend_hash_str_find_ptr(&phar->manifest, entry_obj->entry->filename, entry_obj->entry->filename_len);
46624662
}
4663-
phar_metadata_tracker_free(&entry_obj->entry->metadata_tracker);
4663+
phar_metadata_tracker_free(&entry_obj->entry->metadata_tracker, entry_obj->entry->is_persistent);
46644664

46654665
ZVAL_COPY(&entry_obj->entry->metadata_tracker.val, metadata);
46664666

@@ -4698,7 +4698,7 @@ PHP_METHOD(PharFileInfo, delMetadata)
46984698
}
46994699

47004700
// XXX this should check if there is a string?
4701-
if (phar_metadata_tracker_has_data(&entry_obj->entry->metadata_tracker)) {
4701+
if (phar_metadata_tracker_has_data(&entry_obj->entry->metadata_tracker, entry_obj->entry->is_persistent)) {
47024702
if (entry_obj->entry->is_persistent) {
47034703
phar_archive_data *phar = entry_obj->entry->phar;
47044704

@@ -4710,7 +4710,7 @@ PHP_METHOD(PharFileInfo, delMetadata)
47104710
entry_obj->entry = zend_hash_str_find_ptr(&phar->manifest, entry_obj->entry->filename, entry_obj->entry->filename_len);
47114711
}
47124712
// multiple values may reference this.
4713-
phar_metadata_tracker_free(&entry_obj->entry->metadata_tracker);
4713+
phar_metadata_tracker_free(&entry_obj->entry->metadata_tracker, entry_obj->entry->is_persistent);
47144714
entry_obj->entry->is_modified = 1;
47154715
entry_obj->entry->phar->is_modified = 1;
47164716

‎ext/phar/stream.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha
223223
idata->internal_file->flags |= Z_LVAL_P(pzoption);
224224
}
225225
if ((pzoption = zend_hash_str_find(pharcontext, "metadata", sizeof("metadata")-1)) != NULL) {
226-
phar_metadata_tracker_free(&idata->internal_file->metadata_tracker);
226+
phar_metadata_tracker_free(&idata->internal_file->metadata_tracker, idata->internal_file->is_persistent);
227227

228228
metadata = pzoption;
229229
ZVAL_COPY_DEREF(&idata->internal_file->metadata_tracker.val, metadata);

‎ext/phar/tar.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -177,15 +177,15 @@ static int phar_tar_process_metadata(phar_entry_info *entry, php_stream *fp) /*
177177
phar_parse_metadata_lazy(metadata, &entry->metadata_tracker, entry->uncompressed_filesize, entry->is_persistent);
178178

179179
if (entry->filename_len == sizeof(".phar/.metadata.bin")-1 && !memcmp(entry->filename, ".phar/.metadata.bin", sizeof(".phar/.metadata.bin")-1)) {
180-
if (phar_metadata_tracker_has_data(&entry->phar->metadata_tracker)) {
180+
if (phar_metadata_tracker_has_data(&entry->phar->metadata_tracker, entry->phar->is_persistent)) {
181181
efree(metadata);
182182
return FAILURE;
183183
}
184184
entry->phar->metadata_tracker = entry->metadata_tracker;
185185
entry->metadata_tracker.str = NULL;
186186
ZVAL_UNDEF(&entry->metadata_tracker.val);
187187
} else if (entry->filename_len >= sizeof(".phar/.metadata/") + sizeof("/.metadata.bin") - 1 && NULL != (mentry = zend_hash_str_find_ptr(&(entry->phar->manifest), entry->filename + sizeof(".phar/.metadata/") - 1, entry->filename_len - (sizeof("/.metadata.bin") - 1 + sizeof(".phar/.metadata/") - 1)))) {
188-
if (phar_metadata_tracker_has_data(&mentry->metadata_tracker)) {
188+
if (phar_metadata_tracker_has_data(&mentry->metadata_tracker, mentry->is_persistent)) {
189189
efree(metadata);
190190
return FAILURE;
191191
}
@@ -866,7 +866,7 @@ int phar_tar_setmetadata(const phar_metadata_tracker *tracker, phar_entry_info *
866866
{
867867
/* Copy the metadata from tracker to the new entry being written out to temporary files */
868868
const zend_string *serialized_str;
869-
phar_metadata_tracker_copy(&entry->metadata_tracker, tracker);
869+
phar_metadata_tracker_copy(&entry->metadata_tracker, tracker, entry->is_persistent);
870870
phar_metadata_tracker_try_ensure_has_serialized_data(&entry->metadata_tracker, entry->is_persistent);
871871
serialized_str = entry->metadata_tracker.str;
872872

@@ -922,7 +922,7 @@ static int phar_tar_setupmetadata(zval *zv, void *argument) /* {{{ */
922922
/* now we are dealing with regular files, so look for metadata */
923923
lookfor_len = spprintf(&lookfor, 0, ".phar/.metadata/%s/.metadata.bin", entry->filename);
924924

925-
if (!phar_metadata_tracker_has_data(&entry->metadata_tracker)) {
925+
if (!phar_metadata_tracker_has_data(&entry->metadata_tracker, entry->is_persistent)) {
926926
zend_hash_str_del(&(entry->phar->manifest), lookfor, lookfor_len);
927927
efree(lookfor);
928928
return ZEND_HASH_APPLY_KEEP;
@@ -1160,7 +1160,7 @@ int phar_tar_flush(phar_archive_data *phar, char *user_stub, zend_long len, int
11601160
pass.free_fp = 1;
11611161
pass.free_ufp = 1;
11621162

1163-
if (phar_metadata_tracker_has_data(&phar->metadata_tracker)) {
1163+
if (phar_metadata_tracker_has_data(&phar->metadata_tracker, phar->is_persistent)) {
11641164
phar_entry_info *mentry;
11651165
if (NULL != (mentry = zend_hash_str_find_ptr(&(phar->manifest), ".phar/.metadata.bin", sizeof(".phar/.metadata.bin")-1))) {
11661166
if (ZEND_HASH_APPLY_KEEP != phar_tar_setmetadata(&phar->metadata_tracker, mentry, error)) {

‎ext/phar/zip.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ int phar_parse_zipfile(php_stream *fp, char *fname, size_t fname_len, char *alia
299299
zend_hash_destroy(&mydata->virtual_dirs); \
300300
HT_INVALIDATE(&mydata->virtual_dirs); \
301301
php_stream_close(fp); \
302-
phar_metadata_tracker_free(&mydata->metadata_tracker); \
302+
phar_metadata_tracker_free(&mydata->metadata_tracker, mydata->is_persistent); \
303303
if (mydata->signature) { \
304304
efree(mydata->signature); \
305305
} \
@@ -321,7 +321,7 @@ int phar_parse_zipfile(php_stream *fp, char *fname, size_t fname_len, char *alia
321321
zend_hash_destroy(&mydata->virtual_dirs); \
322322
HT_INVALIDATE(&mydata->virtual_dirs); \
323323
php_stream_close(fp); \
324-
phar_metadata_tracker_free(&mydata->metadata_tracker); \
324+
phar_metadata_tracker_free(&mydata->metadata_tracker, mydata->is_persistent); \
325325
if (mydata->signature) { \
326326
efree(mydata->signature); \
327327
} \
@@ -958,7 +958,7 @@ static int phar_zip_changed_apply_int(phar_entry_info *entry, void *arg) /* {{{
958958
PHAR_SET_32(local.crc32, entry->crc32);
959959
continue_dir:
960960
/* set file metadata */
961-
if (phar_metadata_tracker_has_data(&entry->metadata_tracker)) {
961+
if (phar_metadata_tracker_has_data(&entry->metadata_tracker, entry->is_persistent)) {
962962
phar_metadata_tracker_try_ensure_has_serialized_data(&entry->metadata_tracker, entry->is_persistent);
963963
PHAR_SET_16(central.comment_len, entry->metadata_tracker.str ? ZSTR_LEN(entry->metadata_tracker.str) : 0);
964964
}

0 commit comments

Comments
 (0)