Skip to content

Commit 1dfc39d

Browse files
TysonAndreTysonAndre
committed
Support getMetadata(array $unserialize_options=[])
Also, add more tests of edge cases that can be encountered when modifying metadata of phars. Attempt to serialize immediately when setMetadata is called, don't modify the original value if that fails.
1 parent 497a0fb commit 1dfc39d

11 files changed

+228
-48
lines changed

‎ext/phar/phar.c

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "phar_internal.h"
2222
#include "SAPI.h"
2323
#include "func_interceptors.h"
24+
#include "ext/standard/php_var.h"
2425

2526
static void destroy_phar_data(zval *zv);
2627

@@ -601,29 +602,47 @@ void phar_metadata_tracker_try_ensure_has_serialized_data(phar_metadata_tracker
601602
/**
602603
* Parse out metadata when phar_metadata_tracker_has_data is true
603604
*/
604-
int phar_metadata_tracker_unserialize_or_copy(const phar_metadata_tracker *tracker, zval *metadata, int persistent) /* {{{ */
605+
int phar_metadata_tracker_unserialize_or_copy(phar_metadata_tracker *tracker, zval *metadata, int persistent, zval *unserialize_options, const char* method_name) /* {{{ */
605606
{
606-
if (Z_ISUNDEF(tracker->val) || persistent) {
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+
}
612+
613+
if (Z_ISUNDEF(tracker->val) || persistent || has_unserialize_options) {
614+
const char *start;
607615
/* 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));
609-
const unsigned char *start;
610-
php_unserialize_data_t var_hash;
616+
617+
ZEND_ASSERT(has_unserialize_options || Z_ISUNDEF(tracker->val));
618+
if (has_unserialize_options && tracker->str == NULL) {
619+
/* If this specifies any unserialization options, assume that this will need to serialize the data to unserialize it later */
620+
smart_str main_metadata_str = {0};
621+
php_serialize_data_t metadata_hash;
622+
ZEND_ASSERT(!persistent);
623+
PHP_VAR_SERIALIZE_INIT(metadata_hash);
624+
php_var_serialize(&main_metadata_str, &tracker->val, &metadata_hash);
625+
PHP_VAR_SERIALIZE_DESTROY(metadata_hash);
626+
if (EG(exception)) {
627+
return FAILURE;
628+
}
629+
tracker->str = main_metadata_str.s;
630+
}
611631
/* precondition: This has data */
612632
ZEND_ASSERT(tracker->str != NULL);
613633
ZVAL_NULL(metadata);
614-
PHP_VAR_UNSERIALIZE_INIT(var_hash);
615-
start = (const unsigned char*)ZSTR_VAL(tracker->str);
634+
start = ZSTR_VAL(tracker->str);
616635

617-
if (!php_var_unserialize(metadata, &start, start + ZSTR_LEN(tracker->str), &var_hash)) {
618-
PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
636+
unserialize_with_options(metadata, start, ZSTR_LEN(tracker->str), unserialize_options, method_name);
637+
if (EG(exception)) {
619638
zval_ptr_dtor(metadata);
620639
ZVAL_UNDEF(metadata);
621640
return FAILURE;
622641
}
623-
PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
624642
return SUCCESS;
625643
} else {
626-
/* TODO: what is the current/expected behavior when fetching an object with getMetadata() and modifying a property? Previously, it was underdefined, and probably unimportant to support */
644+
/* TODO: what is the current/expected behavior when fetching an object set with setMetadata then getting it
645+
* with getMetadata() and modifying a property? Previously, it was underdefined, and probably unimportant to support. */
627646
ZVAL_COPY(metadata, &tracker->val);
628647
}
629648

@@ -1100,7 +1119,6 @@ static int phar_parse_pharfile(php_stream *fp, char *fname, size_t fname_len, ch
11001119
MAPPHAR_FAIL("internal corruption of phar \"%s\" (trying to read past buffer end)");
11011120
}
11021121
/* Don't implicitly call unserialize() on potentially untrusted input unless getMetadata() is called directly. */
1103-
// XXX where is the raw data to unserialize being set
11041122
phar_parse_metadata_lazy(buffer, &mydata->metadata_tracker, len, mydata->is_persistent);
11051123
buffer += len;
11061124

‎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(const phar_metadata_tracker* tracker, zval *value, 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);
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: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3947,17 +3947,18 @@ PHP_METHOD(Phar, hasMetadata)
39473947
/* {{{ Returns the global metadata of the phar */
39483948
PHP_METHOD(Phar, getMetadata)
39493949
{
3950+
zval *unserialize_options = NULL;
39503951
phar_metadata_tracker *tracker;
39513952
PHAR_ARCHIVE_OBJECT();
39523953

3953-
if (zend_parse_parameters_none() == FAILURE) {
3954-
RETURN_THROWS();
3955-
}
3954+
ZEND_PARSE_PARAMETERS_START(0, 1)
3955+
Z_PARAM_OPTIONAL
3956+
Z_PARAM_ARRAY(unserialize_options)
3957+
ZEND_PARSE_PARAMETERS_END();
39563958

39573959
tracker = &phar_obj->archive->metadata_tracker;
39583960
if (phar_metadata_tracker_has_data(tracker, phar_obj->archive->is_persistent)) {
3959-
/* assume success, we would have failed before */
3960-
phar_metadata_tracker_unserialize_or_copy(tracker, return_value, phar_obj->archive->is_persistent);
3961+
phar_metadata_tracker_unserialize_or_copy(tracker, return_value, phar_obj->archive->is_persistent, unserialize_options, "Phar::getMetadata");
39613962
}
39623963
}
39633964
/* }}} */
@@ -3967,6 +3968,8 @@ PHP_METHOD(Phar, setMetadata)
39673968
{
39683969
char *error;
39693970
zval *metadata;
3971+
php_serialize_data_t metadata_hash;
3972+
smart_str main_metadata_str = {0};
39703973

39713974
PHAR_ARCHIVE_OBJECT();
39723975

@@ -3983,9 +3986,24 @@ PHP_METHOD(Phar, setMetadata)
39833986
zend_throw_exception_ex(phar_ce_PharException, 0, "phar \"%s\" is persistent, unable to copy on write", phar_obj->archive->fname);
39843987
RETURN_THROWS();
39853988
}
3989+
3990+
ZEND_ASSERT(!phar_obj->archive->is_persistent); /* Should no longer be persistent */
3991+
39863992
phar_metadata_tracker_free(&phar_obj->archive->metadata_tracker, phar_obj->archive->is_persistent);
3993+
if (EG(exception)) {
3994+
/* Destructor can throw. */
3995+
return;
3996+
}
39873997

3998+
PHP_VAR_SERIALIZE_INIT(metadata_hash);
3999+
php_var_serialize(&main_metadata_str, metadata, &metadata_hash);
4000+
PHP_VAR_SERIALIZE_DESTROY(metadata_hash);
4001+
if (EG(exception)) {
4002+
/* Serialization can throw. Don't overwrite the original string. */
4003+
return;
4004+
}
39884005
ZVAL_COPY(&phar_obj->archive->metadata_tracker.val, metadata);
4006+
phar_obj->archive->metadata_tracker.str = main_metadata_str.s;
39894007
phar_obj->archive->is_modified = 1;
39904008
phar_flush(phar_obj->archive, 0, 0, 0, &error);
39914009

@@ -4617,16 +4635,18 @@ PHP_METHOD(PharFileInfo, hasMetadata)
46174635
/* {{{ Returns the metadata of the entry */
46184636
PHP_METHOD(PharFileInfo, getMetadata)
46194637
{
4638+
zval *unserialize_options = NULL;
46204639
phar_metadata_tracker *tracker;
46214640
PHAR_ENTRY_OBJECT();
46224641

4623-
if (zend_parse_parameters_none() == FAILURE) {
4624-
RETURN_THROWS();
4625-
}
4642+
ZEND_PARSE_PARAMETERS_START(0, 1)
4643+
Z_PARAM_OPTIONAL
4644+
Z_PARAM_ARRAY(unserialize_options)
4645+
ZEND_PARSE_PARAMETERS_END();
46264646

46274647
tracker = &entry_obj->entry->metadata_tracker;
46284648
if (phar_metadata_tracker_has_data(tracker, entry_obj->entry->is_persistent)) {
4629-
phar_metadata_tracker_unserialize_or_copy(tracker, return_value, entry_obj->entry->is_persistent);
4649+
phar_metadata_tracker_unserialize_or_copy(tracker, return_value, entry_obj->entry->is_persistent, unserialize_options, "PharFileInfo::getMetadata");
46304650
}
46314651
}
46324652
/* }}} */
@@ -4701,7 +4721,6 @@ PHP_METHOD(PharFileInfo, delMetadata)
47014721
RETURN_THROWS();
47024722
}
47034723

4704-
// XXX this should check if there is a string?
47054724
if (phar_metadata_tracker_has_data(&entry_obj->entry->metadata_tracker, entry_obj->entry->is_persistent)) {
47064725
if (entry_obj->entry->is_persistent) {
47074726
phar_archive_data *phar = entry_obj->entry->phar;
@@ -4713,7 +4732,7 @@ PHP_METHOD(PharFileInfo, delMetadata)
47134732
/* re-populate after copy-on-write */
47144733
entry_obj->entry = zend_hash_str_find_ptr(&phar->manifest, entry_obj->entry->filename, entry_obj->entry->filename_len);
47154734
}
4716-
// multiple values may reference this.
4735+
/* multiple values may reference the metadata */
47174736
phar_metadata_tracker_free(&entry_obj->entry->metadata_tracker, entry_obj->entry->is_persistent);
47184737
entry_obj->entry->is_modified = 1;
47194738
entry_obj->entry->phar->is_modified = 1;

‎ext/phar/phar_object.stub.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public function getAlias() {}
6767
public function getPath() {}
6868

6969
/** @return mixed */
70-
public function getMetadata() {}
70+
public function getMetadata(array $unserialize_options = []) {}
7171

7272
/** @return bool */
7373
public function getModified() {}
@@ -303,7 +303,7 @@ public function getPath() {}
303303
* @return mixed
304304
* @alias Phar::getMetadata
305305
*/
306-
public function getMetadata() {}
306+
public function getMetadata(array $unserialize_options = []) {}
307307

308308
/**
309309
* @return bool
@@ -510,7 +510,7 @@ public function getCRC32() {}
510510
public function getContent() {}
511511

512512
/** @return mixed */
513-
public function getMetadata() {}
513+
public function getMetadata(array $unserialize_options = []) {}
514514

515515
/** @return int */
516516
public function getPharFlags() {}

‎ext/phar/phar_object_arginfo.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* This is a generated file, edit the .stub.php file instead.
2-
* Stub hash: fd4f05b74248e4f7efb234cac8e3a90e17037ee0 */
2+
* Stub hash: 586c79f097e9153b70f6c6e208daa08acc0ce1d4 */
33

44
ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Phar___construct, 0, 0, 1)
55
ZEND_ARG_TYPE_INFO(0, filename, IS_STRING, 0)
@@ -82,7 +82,9 @@ ZEND_END_ARG_INFO()
8282

8383
#define arginfo_class_Phar_getPath arginfo_class_Phar___destruct
8484

85-
#define arginfo_class_Phar_getMetadata arginfo_class_Phar___destruct
85+
ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Phar_getMetadata, 0, 0, 0)
86+
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, unserialize_options, IS_ARRAY, 0, "[]")
87+
ZEND_END_ARG_INFO()
8688

8789
#define arginfo_class_Phar_getModified arginfo_class_Phar___destruct
8890

@@ -252,7 +254,7 @@ ZEND_END_ARG_INFO()
252254

253255
#define arginfo_class_PharData_getPath arginfo_class_Phar___destruct
254256

255-
#define arginfo_class_PharData_getMetadata arginfo_class_Phar___destruct
257+
#define arginfo_class_PharData_getMetadata arginfo_class_Phar_getMetadata
256258

257259
#define arginfo_class_PharData_getModified arginfo_class_Phar___destruct
258260

@@ -346,7 +348,7 @@ ZEND_END_ARG_INFO()
346348

347349
#define arginfo_class_PharFileInfo_getContent arginfo_class_Phar___destruct
348350

349-
#define arginfo_class_PharFileInfo_getMetadata arginfo_class_Phar___destruct
351+
#define arginfo_class_PharFileInfo_getMetadata arginfo_class_Phar_getMetadata
350352

351353
#define arginfo_class_PharFileInfo_getPharFlags arginfo_class_Phar___destruct
352354

‎ext/phar/tests/phar_metadata_write2.phpt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ if (!extension_loaded("phar")) die("skip");
77
--INI--
88
phar.require_hash=0
99
phar.readonly=0
10-
phar.sanitize_object_metadata=0
1110
--FILE--
1211
<?php
1312
$fname = __DIR__ . '/' . basename(__FILE__, '.php') . '.phar.php';

‎ext/phar/tests/phar_metadata_write3.phpt

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ if (!extension_loaded("phar")) die("skip");
77
--INI--
88
phar.require_hash=0
99
phar.readonly=0
10-
phar.sanitize_object_metadata=0
1110
--FILE--
1211
<?php
1312
class EchoesOnWakeup {
@@ -40,14 +39,27 @@ copy($fname, $fname_new);
4039
$phar = new Phar($fname_new);
4140
echo "Calling getMetadata\n";
4241
var_dump($phar->getMetadata());
42+
echo "Calling getMetadata with no allowed_classes\n";
43+
var_dump($phar->getMetadata(['allowed_classes' => []]));
44+
echo "Calling getMetadata with EchoesOnWakeup allowed\n";
45+
var_dump($phar->getMetadata(['allowed_classes' => [EchoesOnWakeup::class]]));
46+
// Part of this is a test that there are no unexpected behaviors when both selMetadata and getMetadata are used
47+
$phar->setMetaData([new EchoesOnWakeup(), new stdClass()]);
48+
echo "Calling getMetadata with too low max_depth\n";
49+
var_dump($phar->getMetadata(['max_depth' => 1]));
50+
echo "Calling getMetadata with some allowed classes\n";
51+
var_dump($phar->getMetadata(['allowed_classes' => [EchoesOnWakeup::class]]));
52+
echo "Calling getMetadata with no options returns the original metadata value\n";
53+
var_dump($phar->getMetadata());
54+
unset($phar);
4355

4456
?>
4557
--CLEAN--
4658
<?php
4759
unlink(__DIR__ . '/' . basename(__FILE__, '.clean.php') . '.phar.php');
4860
unlink(__DIR__ . '/' . basename(__FILE__, '.clean.php') . '.phar.php.copy.php');
4961
?>
50-
--EXPECT--
62+
--EXPECTF--
5163
Reading file contents through stream wrapper
5264
string(18) "contents of file a"
5365
Original metadata
@@ -56,3 +68,39 @@ Calling getMetadata
5668
In wakeup
5769
object(EchoesOnWakeup)#2 (0) {
5870
}
71+
Calling getMetadata with no allowed_classes
72+
object(__PHP_Incomplete_Class)#2 (1) {
73+
["__PHP_Incomplete_Class_Name"]=>
74+
string(14) "EchoesOnWakeup"
75+
}
76+
Calling getMetadata with EchoesOnWakeup allowed
77+
In wakeup
78+
object(EchoesOnWakeup)#2 (0) {
79+
}
80+
Calling getMetadata with too low max_depth
81+
82+
Warning: Phar::getMetadata(): Maximum depth of 1 exceeded. The depth limit can be changed using the max_depth unserialize() option or the unserialize_max_depth ini setting in %sphar_metadata_write3.php on line 39
83+
84+
Notice: Phar::getMetadata(): Error at offset 34 of 59 bytes in %sphar_metadata_write3.php on line 39
85+
bool(false)
86+
Calling getMetadata with some allowed classes
87+
In wakeup
88+
array(2) {
89+
[0]=>
90+
object(EchoesOnWakeup)#4 (0) {
91+
}
92+
[1]=>
93+
object(__PHP_Incomplete_Class)#5 (1) {
94+
["__PHP_Incomplete_Class_Name"]=>
95+
string(8) "stdClass"
96+
}
97+
}
98+
Calling getMetadata with no options returns the original metadata value
99+
array(2) {
100+
[0]=>
101+
object(EchoesOnWakeup)#2 (0) {
102+
}
103+
[1]=>
104+
object(stdClass)#3 (0) {
105+
}
106+
}

0 commit comments

Comments
 (0)