Skip to content

Commit a496a62

Browse files
committed
Start addressing initial review comments
1 parent bbaa263 commit a496a62

File tree

5 files changed

+28
-31
lines changed

5 files changed

+28
-31
lines changed

ext/phar/phar.c

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -590,31 +590,29 @@ void phar_metadata_tracker_try_ensure_has_serialized_data(phar_metadata_tracker
590590
if (!metadata_str.s) {
591591
return;
592592
}
593-
if (persistent) {
594-
tracker->str = zend_string_init(ZSTR_VAL(metadata_str.s), ZSTR_LEN(metadata_str.s), persistent);
595-
zend_string_release(metadata_str.s);
596-
} else {
597-
tracker->str = metadata_str.s;
598-
}
593+
tracker->str = metadata_str.s;
599594
}
600595
/* }}} */
601596

602597
/**
603598
* Parse out metadata when phar_metadata_tracker_has_data is true
604599
*/
605-
int phar_metadata_tracker_unserialize_or_copy(phar_metadata_tracker *tracker, zval *metadata, int persistent, zval *unserialize_options, const char* method_name) /* {{{ */
600+
int phar_metadata_tracker_unserialize_or_copy(phar_metadata_tracker *tracker, zval *metadata, int persistent, HashTable *unserialize_options, const char* method_name) /* {{{ */
606601
{
607-
const zend_bool has_unserialize_options = unserialize_options != NULL && Z_TYPE_P(unserialize_options) == IS_ARRAY &&
608-
zend_array_count(Z_ARRVAL_P(unserialize_options)) > 0;
609-
if (EG(exception)) {
610-
return FAILURE;
611-
}
602+
const zend_bool has_unserialize_options = unserialize_options != NULL && zend_array_count(unserialize_options) > 0;
603+
/* It should be impossible to create a zval in a persistent phar/entry. */
604+
ZEND_ASSERT(!persistent || Z_ISUNDEF(tracker->val));
612605

613-
if (Z_ISUNDEF(tracker->val) || persistent || has_unserialize_options) {
606+
if (Z_ISUNDEF(tracker->val) || has_unserialize_options) {
607+
if (EG(exception)) {
608+
/* Because other parts of the phar code haven't been updated to check for exceptions after doing something that may throw,
609+
* check for exceptions before potentially serializing/unserializing instead. */
610+
return FAILURE;
611+
}
612+
/* Persistent phars should always be unserialized. */
614613
const char *start;
615614
/* Assert it should not be possible to create raw data in a persistent phar (i.e. from cache_list) */
616615

617-
ZEND_ASSERT(has_unserialize_options || Z_ISUNDEF(tracker->val));
618616
if (has_unserialize_options && tracker->str == NULL) {
619617
/* If this specifies any unserialization options, assume that this will need to serialize the data to unserialize it later */
620618
smart_str main_metadata_str = {0};
@@ -633,7 +631,7 @@ int phar_metadata_tracker_unserialize_or_copy(phar_metadata_tracker *tracker, zv
633631
ZVAL_NULL(metadata);
634632
start = ZSTR_VAL(tracker->str);
635633

636-
unserialize_with_options(metadata, start, ZSTR_LEN(tracker->str), unserialize_options, method_name);
634+
php_unserialize_with_options(metadata, start, ZSTR_LEN(tracker->str), unserialize_options, method_name);
637635
if (EG(exception)) {
638636
zval_ptr_dtor(metadata);
639637
ZVAL_UNDEF(metadata);
@@ -700,15 +698,15 @@ void phar_metadata_tracker_copy(phar_metadata_tracker *dest, const phar_metadata
700698
*/
701699
void phar_metadata_tracker_clone(phar_metadata_tracker *tracker) /* {{{ */
702700
{
703-
zval_copy_ctor(&tracker->val);
701+
Z_TRY_ADDREF_P(&tracker->val);
704702
if (tracker->str) {
705703
tracker->str = zend_string_copy(tracker->str);
706704
}
707705
}
708706
/* }}} */
709707

710708
/**
711-
* Parse out metadata from the manifest for a single file, saving it into a Z_PTR (or setting metadata to Z_UNDEF)
709+
* Parse out metadata from the manifest for a single file, saving it into a string.
712710
*
713711
* Meta-data is in this format:
714712
* [len32][data...]

ext/phar/phar_internal.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,7 @@ void phar_metadata_tracker_free(phar_metadata_tracker* val, int persistent);
559559
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);
562-
int phar_metadata_tracker_unserialize_or_copy(phar_metadata_tracker* tracker, zval *value, int persistent, zval *unserialize_options, const char* method_name);
562+
int phar_metadata_tracker_unserialize_or_copy(phar_metadata_tracker* tracker, zval *value, int persistent, HashTable *unserialize_options, const char* method_name);
563563
void phar_release_entry_metadata(phar_entry_info *entry);
564564
void phar_release_archive_metadata(phar_archive_data *phar);
565565
void destroy_phar_manifest_entry(zval *zv);

ext/phar/phar_object.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3947,13 +3947,13 @@ PHP_METHOD(Phar, hasMetadata)
39473947
/* {{{ Returns the global metadata of the phar */
39483948
PHP_METHOD(Phar, getMetadata)
39493949
{
3950-
zval *unserialize_options = NULL;
3950+
HashTable *unserialize_options = NULL;
39513951
phar_metadata_tracker *tracker;
39523952
PHAR_ARCHIVE_OBJECT();
39533953

39543954
ZEND_PARSE_PARAMETERS_START(0, 1)
39553955
Z_PARAM_OPTIONAL
3956-
Z_PARAM_ARRAY(unserialize_options)
3956+
Z_PARAM_ARRAY_HT(unserialize_options)
39573957
ZEND_PARSE_PARAMETERS_END();
39583958

39593959
tracker = &phar_obj->archive->metadata_tracker;
@@ -4635,13 +4635,13 @@ PHP_METHOD(PharFileInfo, hasMetadata)
46354635
/* {{{ Returns the metadata of the entry */
46364636
PHP_METHOD(PharFileInfo, getMetadata)
46374637
{
4638-
zval *unserialize_options = NULL;
4638+
HashTable *unserialize_options = NULL;
46394639
phar_metadata_tracker *tracker;
46404640
PHAR_ENTRY_OBJECT();
46414641

46424642
ZEND_PARSE_PARAMETERS_START(0, 1)
46434643
Z_PARAM_OPTIONAL
4644-
Z_PARAM_ARRAY(unserialize_options)
4644+
Z_PARAM_ARRAY_HT(unserialize_options)
46454645
ZEND_PARSE_PARAMETERS_END();
46464646

46474647
tracker = &entry_obj->entry->metadata_tracker;

ext/standard/php_var.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ PHPAPI zend_long php_var_unserialize_get_cur_depth(php_unserialize_data_t d);
5959
#define PHP_VAR_UNSERIALIZE_DESTROY(d) \
6060
php_var_unserialize_destroy(d)
6161

62-
PHPAPI void unserialize_with_options(zval *return_value, const char *buf, const size_t buf_len, zval *options, const char* function_name);
62+
PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, const size_t buf_len, HashTable *options, const char* function_name);
6363

6464
PHPAPI void var_replace(php_unserialize_data_t *var_hash, zval *ozval, zval *nzval);
6565
PHPAPI void var_push_dtor(php_unserialize_data_t *var_hash, zval *val);

ext/standard/var.c

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,8 +1171,8 @@ PHP_FUNCTION(serialize)
11711171
}
11721172
/* }}} */
11731173

1174-
/* {{{ Takes a string representation of variable and recreates it, subject to the optional unserialize options array */
1175-
PHPAPI void unserialize_with_options(zval *return_value, const char *buf, const size_t buf_len, zval *options, const char* function_name)
1174+
/* {{{ Takes a string representation of variable and recreates it, subject to the optional unserialize options HashTable */
1175+
PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, const size_t buf_len, HashTable *options, const char* function_name)
11761176
{
11771177
const unsigned char *p;
11781178
php_unserialize_data_t var_hash;
@@ -1191,10 +1191,9 @@ PHPAPI void unserialize_with_options(zval *return_value, const char *buf, const
11911191
prev_max_depth = php_var_unserialize_get_max_depth(var_hash);
11921192
prev_cur_depth = php_var_unserialize_get_cur_depth(var_hash);
11931193
if (options != NULL) {
1194-
ZEND_ASSERT(Z_TYPE_P(options) == IS_ARRAY);
11951194
zval *classes, *max_depth;
11961195

1197-
classes = zend_hash_str_find_deref(Z_ARRVAL_P(options), "allowed_classes", sizeof("allowed_classes")-1);
1196+
classes = zend_hash_str_find_deref(options, "allowed_classes", sizeof("allowed_classes")-1);
11981197
if (classes && Z_TYPE_P(classes) != IS_ARRAY && Z_TYPE_P(classes) != IS_TRUE && Z_TYPE_P(classes) != IS_FALSE) {
11991198
php_error_docref(NULL, E_WARNING, "allowed_classes option should be array or boolean");
12001199
RETVAL_FALSE;
@@ -1223,7 +1222,7 @@ PHPAPI void unserialize_with_options(zval *return_value, const char *buf, const
12231222
}
12241223
php_var_unserialize_set_allowed_classes(var_hash, class_hash);
12251224

1226-
max_depth = zend_hash_str_find_deref(Z_ARRVAL_P(options), "max_depth", sizeof("max_depth") - 1);
1225+
max_depth = zend_hash_str_find_deref(options, "max_depth", sizeof("max_depth") - 1);
12271226
if (max_depth) {
12281227
if (Z_TYPE_P(max_depth) != IS_LONG) {
12291228
zend_type_error("%s(): \"max_depth\" option must be of type int, %s given", function_name, zend_zval_type_name(max_depth));
@@ -1288,15 +1287,15 @@ PHP_FUNCTION(unserialize)
12881287
{
12891288
char *buf = NULL;
12901289
size_t buf_len;
1291-
zval *options = NULL;
1290+
HashTable *options = NULL;
12921291

12931292
ZEND_PARSE_PARAMETERS_START(1, 2)
12941293
Z_PARAM_STRING(buf, buf_len)
12951294
Z_PARAM_OPTIONAL
1296-
Z_PARAM_ARRAY(options)
1295+
Z_PARAM_ARRAY_HT(options)
12971296
ZEND_PARSE_PARAMETERS_END();
12981297

1299-
unserialize_with_options(return_value, buf, buf_len, options, "unserialize");
1298+
php_unserialize_with_options(return_value, buf, buf_len, options, "unserialize");
13001299
}
13011300
/* }}} */
13021301

0 commit comments

Comments
 (0)