Skip to content

Commit

Permalink
Make buildFromIterator() work with custom SplFileInfo objects
Browse files Browse the repository at this point in the history
While it is possible to return a custom SplFileInfo object in the
iterator used by buildFromIterator(), the data is not actually used from
that object, instead the data from the underlying internal structure is
used. This makes it impossible to override some metadata such as the
path name and modification time.

The main motivation comes from two reasons:
- Consistency. We expect our custom methods to be called when having a
  custom object.
- Support reproducibility. This is the original use case as requested in
  [1].

Add support for this by calling the getMTime() and getPathname() methods
if they're overriden by a user class.

[1] theseer/Autoload#114.
  • Loading branch information
nielsdos committed May 5, 2024
1 parent af4f6e2 commit 373d8cf
Show file tree
Hide file tree
Showing 5 changed files with 385 additions and 19 deletions.
101 changes: 82 additions & 19 deletions ext/phar/phar_object.c
Original file line number Diff line number Diff line change
Expand Up @@ -1382,6 +1382,44 @@ struct _phar_t {
int count;
};

/* This is the same as phar_get_or_create_entry_data(), but allows overriding metadata via SplFileInfo. */
static phar_entry_data *phar_build_entry_data(char *fname, size_t fname_len, char *path, size_t path_len, char **error, zval *file_info)
{
bool override_timestamp = false;
uint32_t timestamp;

/* Expects an instance of SplFileInfo if it is an object, which is verified in phar_build(). */
if (Z_TYPE_P(file_info) == IS_OBJECT && Z_OBJCE_P(file_info)->type == ZEND_USER_CLASS) {
zval rv;
/* Phars only have a single timestamp.
* Use modification time to be consistent with the zip and tar file format. */
zend_call_method_with_0_params(Z_OBJ_P(file_info), Z_OBJCE_P(file_info), NULL, "getMTime", &rv);

if (UNEXPECTED(Z_TYPE(rv) != IS_LONG)) {
/* Either an exception happened, or the function returned false to indicate failure. */
ZEND_ASSERT(Z_TYPE(rv) == IS_UNDEF || Z_TYPE(rv) == IS_FALSE);
*error = estrdup("getMTime() failed");
return NULL;
}

/* Sanity check bounds. See GH-14141. */
if (Z_LVAL(rv) > UINT32_MAX) {
*error = estrdup("timestamp is limited to 32-bit");
return NULL;
}

override_timestamp = true;
timestamp = Z_LVAL(rv);
}

phar_entry_data *data = phar_get_or_create_entry_data(fname, fname_len, path, path_len, "w+b", 0, error, 1);
if (data && override_timestamp) {
data->internal_file->timestamp = timestamp;
}

return data;
}

static int phar_build(zend_object_iterator *iter, void *puser) /* {{{ */
{
zval *value;
Expand Down Expand Up @@ -1459,26 +1497,48 @@ static int phar_build(zend_object_iterator *iter, void *puser) /* {{{ */
return ZEND_HASH_APPLY_STOP;
}

switch (intern->type) {
case SPL_FS_DIR: {
char *tmp;
zend_string *test_str = spl_filesystem_object_get_path(intern);
fname_len = spprintf(&tmp, 0, "%s%c%s", ZSTR_VAL(test_str), DEFAULT_SLASH, intern->u.dir.entry.d_name);
zend_string_release_ex(test_str, /* persistent */ false);
if (php_stream_stat_path(tmp, &ssb) == 0 && S_ISDIR(ssb.sb.st_mode)) {
/* ignore directories */
efree(tmp);
return ZEND_HASH_APPLY_KEEP;
zend_string *tmp_dir_str = NULL;

/* Take into account that SplFileObject may be overridden.
* The purpose here is to grab the path name of the file to add. */
if (Z_OBJCE_P(value)->type == ZEND_USER_CLASS) {
zval rv;
zend_call_method_with_0_params(Z_OBJ_P(value), Z_OBJCE_P(value), NULL, "getPathname", &rv);
if (UNEXPECTED(Z_TYPE(rv) != IS_STRING)) {
ZEND_ASSERT(EG(exception));
return ZEND_HASH_APPLY_STOP;
}
tmp_dir_str = Z_STR(rv);
} else {
/* Not a user class, so we can grab the data internally quickly. */
switch (intern->type) {
case SPL_FS_DIR: {
zend_string *test_str = spl_filesystem_object_get_path(intern);
const char slash = DEFAULT_SLASH;
tmp_dir_str = zend_string_concat3(
ZSTR_VAL(test_str), ZSTR_LEN(test_str),
&slash, 1,
intern->u.dir.entry.d_name, strlen(intern->u.dir.entry.d_name)
);
zend_string_release_ex(test_str, /* persistent */ false);
break;
}
case SPL_FS_INFO:
case SPL_FS_FILE:
fname = expand_filepath(ZSTR_VAL(intern->file_name), NULL);
break;
}
}

fname = expand_filepath(tmp, NULL);
efree(tmp);
break;
if (tmp_dir_str) {
if (php_stream_stat_path(ZSTR_VAL(tmp_dir_str), &ssb) == 0 && S_ISDIR(ssb.sb.st_mode)) {
/* ignore directories */
zend_string_release(tmp_dir_str);
return ZEND_HASH_APPLY_KEEP;
}
case SPL_FS_INFO:
case SPL_FS_FILE:
fname = expand_filepath(ZSTR_VAL(intern->file_name), NULL);
break;

fname = expand_filepath(ZSTR_VAL(tmp_dir_str), NULL);
zend_string_release_ex(tmp_dir_str, /* persistent */ false);
}

if (!fname) {
Expand Down Expand Up @@ -1619,8 +1679,11 @@ static int phar_build(zend_object_iterator *iter, void *puser) /* {{{ */
return ZEND_HASH_APPLY_KEEP;
}

if (!(data = phar_get_or_create_entry_data(phar_obj->archive->fname, phar_obj->archive->fname_len, str_key, str_key_len, "w+b", 0, &error, 1))) {
zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Entry %s cannot be created: %s", str_key, error);
if (!(data = phar_build_entry_data(phar_obj->archive->fname, phar_obj->archive->fname_len, str_key, str_key_len, &error, value))) {
if (!EG(exception)) {
/* User methods could've already thrown an exception. */
zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Entry %s cannot be created: %s", str_key, error);
}
efree(error);

if (save) {
Expand Down
87 changes: 87 additions & 0 deletions ext/phar/tests/buildFromIterator_user_overrides/001.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
--TEST--
buildFromIterator with user overrides 001 - getMTime()
--EXTENSIONS--
phar
--INI--
phar.readonly=0
phar.require_hash=0
--CREDITS--
Arne Blankerts
Niels Dossche
--FILE--
<?php

class MySplFileInfo extends SplFileInfo {
public function getPathname(): string {
echo "[Pathname]\n";
return parent::getPathname();
}

public function getMTime(): int|false {
echo "[MTime]\n";
return 123;
}

public function getCTime(): int {
// This should not get called
echo "[CTime]\n";
return 0;
}

public function getATime(): int {
// This should not get called
echo "[ATime]\n";
return 0;
}
}

class MyIterator extends RecursiveDirectoryIterator {
public function current(): SplFileInfo {
echo "[ Found: " . parent::current()->getPathname() . " ]\n";
return new MySplFileInfo(parent::current()->getPathname());
}
}

$workdir = __DIR__.'/001';
mkdir($workdir . '/content', recursive: true);
file_put_contents($workdir . '/content/hello.txt', "Hello world.");

$phar = new \Phar($workdir . '/test.phar');
$phar->startBuffering();
$phar->buildFromIterator(
new RecursiveIteratorIterator(
new MyIterator($workdir . '/content', FilesystemIterator::SKIP_DOTS)
),
$workdir
);
$phar->stopBuffering();


$result = new \Phar($workdir . '/test.phar', 0, 'test.phar');
var_dump($result['content/hello.txt']);
var_dump($result['content/hello.txt']->getATime());
var_dump($result['content/hello.txt']->getMTime());
var_dump($result['content/hello.txt']->getCTime());

?>
--CLEAN--
<?php
$workdir = __DIR__.'/001';
@unlink($workdir . '/test.phar');
@unlink($workdir . '/content/hello.txt');
@rmdir($workdir . '/content');
@rmdir($workdir);
?>
--EXPECTF--
[ Found: %shello.txt ]
[Pathname]
[MTime]
object(PharFileInfo)#%d (2) {
["pathName":"SplFileInfo":private]=>
string(%d) "phar://%shello.txt"
["fileName":"SplFileInfo":private]=>
string(%d) "hello.txt"
}
int(123)
int(123)
int(123)
95 changes: 95 additions & 0 deletions ext/phar/tests/buildFromIterator_user_overrides/002.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
--TEST--
buildFromIterator with user overrides 002 - errors in getMTime()
--EXTENSIONS--
phar
--SKIPIF--
<?php
if (PHP_INT_SIZE != 8) die("skip: 64-bit only");
?>
--INI--
phar.readonly=0
phar.require_hash=0
--CREDITS--
Arne Blankerts
Niels Dossche
--FILE--
<?php

class MySplFileInfo1 extends SplFileInfo {
public function getMTime(): int|false {
echo "[MTime]\n";
return PHP_INT_MAX;
}
}

class MySplFileInfo2 extends SplFileInfo {
public function getMTime(): int|false {
echo "[MTime]\n";
return false;
}
}

class MySplFileInfo3 extends SplFileInfo {
public function getMTime(): int|false {
echo "[MTime]\n";
throw new Error('Throwing an exception inside getMTime()');
}
}

class MyIterator extends RecursiveDirectoryIterator {
public function current(): SplFileInfo {
static $counter = 0;
$counter++;
echo "[ Found: " . parent::current()->getPathname() . " ]\n";
if ($counter === 1) {
return new MySplFileInfo1(parent::current()->getPathname());
} else if ($counter === 2) {
return new MySplFileInfo2(parent::current()->getPathname());
} else if ($counter === 3) {
return new MySplFileInfo3(parent::current()->getPathname());
}
}
}

$workdir = __DIR__.'/002';
mkdir($workdir . '/content', recursive: true);
file_put_contents($workdir . '/content/hello.txt', "Hello world.");

for ($i = 0; $i < 3; $i++) {
echo "--- Iteration $i ---\n";
try {
$phar = new \Phar($workdir . "/test$i.phar");
$phar->startBuffering();
$phar->buildFromIterator(
new RecursiveIteratorIterator(
new MyIterator($workdir . '/content', FilesystemIterator::SKIP_DOTS)
),
$workdir
);
$phar->stopBuffering();
} catch (Throwable $e) {
echo $e->getMessage(), "\n";
}
}

?>
--CLEAN--
<?php
$workdir = __DIR__.'/002';
@unlink($workdir . '/content/hello.txt');
@rmdir($workdir . '/content');
@rmdir($workdir);
?>
--EXPECTF--
--- Iteration 0 ---
[ Found: %shello.txt ]
[MTime]
Entry content/hello.txt cannot be created: timestamp is limited to 32-bit
--- Iteration 1 ---
[ Found: %shello.txt ]
[MTime]
Entry content/hello.txt cannot be created: getMTime() failed
--- Iteration 2 ---
[ Found: %shello.txt ]
[MTime]
Throwing an exception inside getMTime()
57 changes: 57 additions & 0 deletions ext/phar/tests/buildFromIterator_user_overrides/003.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
--TEST--
buildFromIterator with user overrides 003 - errors in getPathname()
--EXTENSIONS--
phar
--INI--
phar.readonly=0
phar.require_hash=0
--CREDITS--
Arne Blankerts
Niels Dossche
--FILE--
<?php

class MyGlobIterator extends GlobIterator {
public function getPathname(): string {
var_dump(parent::getPathname());
throw new Error('exception in getPathname()');
}
}

class MyIterator extends RecursiveDirectoryIterator {
public function current(): SplFileInfo {
echo "[ Found: " . parent::current()->getPathname() . " ]\n";
return new MyGlobIterator(parent::current()->getPath() . '/*');
}
}

$workdir = __DIR__.'/003';
mkdir($workdir . '/content', recursive: true);
file_put_contents($workdir . '/content/hello.txt', "Hello world.");

$phar = new \Phar($workdir . "/test.phar");
$phar->startBuffering();
try {
$phar->buildFromIterator(
new RecursiveIteratorIterator(
new MyIterator($workdir . '/content', FilesystemIterator::SKIP_DOTS)
),
$workdir
);
} catch (Throwable $e) {
echo $e->getMessage(), "\n";
}
$phar->stopBuffering();

?>
--CLEAN--
<?php
$workdir = __DIR__.'/003';
@unlink($workdir . '/content/hello.txt');
@rmdir($workdir . '/content');
@rmdir($workdir);
?>
--EXPECTF--
[ Found: %shello.txt ]
string(%d) "%shello.txt"
exception in getPathname()
Loading

0 comments on commit 373d8cf

Please sign in to comment.