Skip to content

ext/phar: Use HASH_FOREACH macros and other refactorings #16073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Sep 26, 2024
Merged

ext/phar: Use HASH_FOREACH macros and other refactorings #16073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f85e515
ext/phar: Use HASH_FOREACH macro in phar_wrapper_rmdir()
Girgias Sep 26, 2024
18e5388
ext/phar: Refer to zend_string* length directly
Girgias Sep 26, 2024
a4f2ca3
ext/phar: Use HASH_FOREACH macro in phar_wrapper_open_dir()
Girgias Sep 26, 2024
554ede5
ext/phar: Prevent duplicate strlen() computation in phar_wrapper_open…
Girgias Sep 26, 2024
4909973
ext/phar: Use HASH_FOREACH macro in phar_make_dirstream()
Girgias Sep 26, 2024
cd64aab
ext/phar: Remove unnecessary memory duplication for phar_make_dirstre…
Girgias Sep 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
195 changes: 70 additions & 125 deletions ext/phar/dirstream.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,22 +134,6 @@ static int phar_dir_flush(php_stream *stream) /* {{{ */
}
/* }}} */

/**
* add an empty element with a char * key to a hash table, avoiding duplicates
*
* This is used to get a unique listing of virtual directories within a phar,
* for iterating over opendir()ed phar directories.
*/
static int phar_add_empty(HashTable *ht, char *arKey, uint32_t nKeyLength) /* {{{ */
{
zval dummy;

ZVAL_NULL(&dummy);
zend_hash_str_update(ht, arKey, nKeyLength, &dummy);
return SUCCESS;
}
/* }}} */

/**
* Used for sorting directories alphabetically
*/
Expand All @@ -166,118 +150,97 @@ static int phar_compare_dir_name(Bucket *f, Bucket *s) /* {{{ */
* files in a phar and retrieving its relative path. From this, construct
* a list of files/directories that are "in" the directory represented by dir
*/
static php_stream *phar_make_dirstream(char *dir, HashTable *manifest) /* {{{ */
static php_stream *phar_make_dirstream(const char *dir, size_t dirlen, const HashTable *manifest) /* {{{ */
{
HashTable *data;
size_t dirlen = strlen(dir);
char *entry, *found, *save;
zend_string *str_key;
size_t keylen;
zend_ulong unused;
char *entry;

ALLOC_HASHTABLE(data);
zend_hash_init(data, 64, NULL, NULL, 0);

if ((*dir == '/' && dirlen == 1 && (manifest->nNumOfElements == 0)) || (dirlen >= sizeof(".phar")-1 && !memcmp(dir, ".phar", sizeof(".phar")-1))) {
/* make empty root directory for empty phar */
/* make empty directory for .phar magic directory */
efree(dir);
return php_stream_alloc(&phar_dir_ops, data, NULL, "r");
}

zend_hash_internal_pointer_reset(manifest);

while (FAILURE != zend_hash_has_more_elements(manifest)) {
if (HASH_KEY_NON_EXISTENT == zend_hash_get_current_key(manifest, &str_key, &unused)) {
break;
}

keylen = ZSTR_LEN(str_key);
zend_string *str_key;
ZEND_HASH_MAP_FOREACH_STR_KEY(manifest, str_key) {
size_t keylen = ZSTR_LEN(str_key);
if (keylen <= dirlen) {
if (keylen == 0 || keylen < dirlen || !strncmp(ZSTR_VAL(str_key), dir, dirlen)) {
Comment on lines 170 to 171
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't exactly understand what these conditions are doing, but it feels like it can be refactored to use one of the zend_string APIs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have to dig deeper to understand how this works, but let's cut the PR here.

if (SUCCESS != zend_hash_move_forward(manifest)) {
break;
}
continue;
}
}

if (*dir == '/') {
/* root directory */
if (keylen >= sizeof(".phar")-1 && !memcmp(ZSTR_VAL(str_key), ".phar", sizeof(".phar")-1)) {
if (zend_string_starts_with_literal(str_key, ".phar")) {
/* do not add any magic entries to this directory */
if (SUCCESS != zend_hash_move_forward(manifest)) {
break;
}
continue;
}

if (NULL != (found = (char *) memchr(ZSTR_VAL(str_key), '/', keylen))) {
const char *has_slash = memchr(ZSTR_VAL(str_key), '/', keylen);
if (has_slash) {
/* the entry has a path separator and is a subdirectory */
entry = (char *) safe_emalloc(found - ZSTR_VAL(str_key), 1, 1);
memcpy(entry, ZSTR_VAL(str_key), found - ZSTR_VAL(str_key));
keylen = found - ZSTR_VAL(str_key);
entry[keylen] = '\0';
} else {
entry = (char *) safe_emalloc(keylen, 1, 1);
memcpy(entry, ZSTR_VAL(str_key), keylen);
entry[keylen] = '\0';
keylen = has_slash - ZSTR_VAL(str_key);
}
entry = safe_emalloc(keylen, 1, 1);
memcpy(entry, ZSTR_VAL(str_key), keylen);
entry[keylen] = '\0';

goto PHAR_ADD_ENTRY;
} else {
if (0 != memcmp(ZSTR_VAL(str_key), dir, dirlen)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And ditto here

/* entry in directory not found */
if (SUCCESS != zend_hash_move_forward(manifest)) {
break;
}
continue;
} else {
if (ZSTR_VAL(str_key)[dirlen] != '/') {
if (SUCCESS != zend_hash_move_forward(manifest)) {
break;
}
continue;
}
}
}

save = ZSTR_VAL(str_key);
const char *save = ZSTR_VAL(str_key);
save += dirlen + 1; /* seek to just past the path separator */

if (NULL != (found = (char *) memchr(save, '/', keylen - dirlen - 1))) {
const char *has_slash = memchr(save, '/', keylen - dirlen - 1);
if (has_slash) {
/* is subdirectory */
save -= dirlen + 1;
entry = (char *) safe_emalloc(found - save + dirlen, 1, 1);
memcpy(entry, save + dirlen + 1, found - save - dirlen - 1);
keylen = found - save - dirlen - 1;
entry = safe_emalloc(has_slash - save + dirlen, 1, 1);
memcpy(entry, save + dirlen + 1, has_slash - save - dirlen - 1);
keylen = has_slash - save - dirlen - 1;
entry[keylen] = '\0';
} else {
/* is file */
save -= dirlen + 1;
entry = (char *) safe_emalloc(keylen - dirlen, 1, 1);
entry = safe_emalloc(keylen - dirlen, 1, 1);
memcpy(entry, save + dirlen + 1, keylen - dirlen - 1);
entry[keylen - dirlen - 1] = '\0';
keylen = keylen - dirlen - 1;
}
PHAR_ADD_ENTRY:
if (keylen) {
phar_add_empty(data, entry, keylen);
/**
* Add an empty element to avoid duplicates
*
* This is used to get a unique listing of virtual directories within a phar,
* for iterating over opendir()ed phar directories.
*/
zval dummy;

ZVAL_NULL(&dummy);
zend_hash_str_update(data, entry, keylen, &dummy);
}

efree(entry);

if (SUCCESS != zend_hash_move_forward(manifest)) {
break;
}
}
} ZEND_HASH_FOREACH_END();

if (FAILURE != zend_hash_has_more_elements(data)) {
efree(dir);
zend_hash_sort(data, phar_compare_dir_name, 0);
return php_stream_alloc(&phar_dir_ops, data, NULL, "r");
} else {
efree(dir);
return php_stream_alloc(&phar_dir_ops, data, NULL, "r");
}
}
Expand All @@ -289,12 +252,8 @@ static php_stream *phar_make_dirstream(char *dir, HashTable *manifest) /* {{{ */
php_stream *phar_wrapper_open_dir(php_stream_wrapper *wrapper, const char *path, const char *mode, int options, zend_string **opened_path, php_stream_context *context STREAMS_DC) /* {{{ */
{
php_url *resource = NULL;
php_stream *ret;
char *internal_file, *error;
zend_string *str_key;
zend_ulong unused;
char *error;
phar_archive_data *phar;
phar_entry_info *entry = NULL;

if ((resource = phar_parse_url(wrapper, path, mode, options)) == NULL) {
php_stream_wrapper_log_error(wrapper, options, "phar url \"%s\" is unknown", path);
Expand All @@ -319,11 +278,9 @@ php_stream *phar_wrapper_open_dir(php_stream_wrapper *wrapper, const char *path,
return NULL;
}

size_t host_len = ZSTR_LEN(resource->host);
phar_request_initialize();
internal_file = ZSTR_VAL(resource->path) + 1; /* strip leading "/" */

if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(resource->host), host_len, NULL, 0, &error)) {
if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), NULL, 0, &error)) {
if (error) {
php_stream_wrapper_log_error(wrapper, options, "%s", error);
efree(error);
Expand All @@ -338,51 +295,46 @@ php_stream *phar_wrapper_open_dir(php_stream_wrapper *wrapper, const char *path,
efree(error);
}

if (*internal_file == '\0') {
if (zend_string_equals(resource->path, ZSTR_CHAR('/'))) {
/* root directory requested */
internal_file = estrndup(internal_file - 1, 1);
ret = phar_make_dirstream(internal_file, &phar->manifest);
php_url_free(resource);
return ret;
return phar_make_dirstream("/", strlen("/"), &phar->manifest);
}

if (!HT_IS_INITIALIZED(&phar->manifest)) {
php_url_free(resource);
return NULL;
}

if (NULL != (entry = zend_hash_str_find_ptr(&phar->manifest, internal_file, strlen(internal_file))) && !entry->is_dir) {
const char *internal_file = ZSTR_VAL(resource->path) + 1; /* strip leading "/" */
size_t internal_file_len = ZSTR_LEN(resource->path) - 1;
phar_entry_info *entry = zend_hash_str_find_ptr(&phar->manifest, internal_file, internal_file_len);
php_stream *ret;

if (NULL != entry && !entry->is_dir) {
php_url_free(resource);
return NULL;
} else if (entry && entry->is_dir) {
if (entry->is_mounted) {
ret = php_stream_opendir(entry->tmp, options, context);
php_url_free(resource);
return php_stream_opendir(entry->tmp, options, context);
return ret;
}
internal_file = estrdup(internal_file);
ret = phar_make_dirstream(internal_file, internal_file_len, &phar->manifest);
php_url_free(resource);
return phar_make_dirstream(internal_file, &phar->manifest);
return ret;
} else {
size_t i_len = strlen(internal_file);
zend_string *str_key;

/* search for directory */
zend_hash_internal_pointer_reset(&phar->manifest);
while (FAILURE != zend_hash_has_more_elements(&phar->manifest)) {
if (HASH_KEY_NON_EXISTENT !=
zend_hash_get_current_key(&phar->manifest, &str_key, &unused)) {
if (ZSTR_LEN(str_key) > i_len && 0 == memcmp(ZSTR_VAL(str_key), internal_file, i_len)) {
/* directory found */
internal_file = estrndup(internal_file,
i_len);
php_url_free(resource);
return phar_make_dirstream(internal_file, &phar->manifest);
}
}

if (SUCCESS != zend_hash_move_forward(&phar->manifest)) {
break;
ZEND_HASH_MAP_FOREACH_STR_KEY(&phar->manifest, str_key) {
if (zend_string_starts_with_cstr(str_key, internal_file, internal_file_len)) {
/* directory found */
ret = phar_make_dirstream(internal_file, internal_file_len, &phar->manifest);
php_url_free(resource);
return ret;
}
}
} ZEND_HASH_FOREACH_END();
}

php_url_free(resource);
Expand Down Expand Up @@ -436,9 +388,7 @@ int phar_wrapper_mkdir(php_stream_wrapper *wrapper, const char *url_from, int mo
return 0;
}

size_t host_len = ZSTR_LEN(resource->host);

if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(resource->host), host_len, NULL, 0, &error)) {
if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), NULL, 0, &error)) {
php_stream_wrapper_log_error(wrapper, options, "phar error: cannot create directory \"%s\" in phar \"%s\", error retrieving phar information: %s", ZSTR_VAL(resource->path) + 1, ZSTR_VAL(resource->host), error);
efree(error);
php_url_free(resource);
Expand Down Expand Up @@ -531,8 +481,6 @@ int phar_wrapper_rmdir(php_stream_wrapper *wrapper, const char *url, int options
char *error, *arch, *entry2;
size_t arch_len, entry_len;
php_url *resource = NULL;
zend_string *str_key;
zend_ulong unused;

/* pre-readonly check, we need to know if this is a data phar */
if (FAILURE == phar_split_fname(url, strlen(url), &arch, &arch_len, &entry2, &entry_len, 2, 2)) {
Expand Down Expand Up @@ -569,9 +517,7 @@ int phar_wrapper_rmdir(php_stream_wrapper *wrapper, const char *url, int options
return 0;
}

size_t host_len = ZSTR_LEN(resource->host);

if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(resource->host), host_len, NULL, 0, &error)) {
if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), NULL, 0, &error)) {
php_stream_wrapper_log_error(wrapper, options, "phar error: cannot remove directory \"%s\" in phar \"%s\", error retrieving phar information: %s", ZSTR_VAL(resource->path)+1, ZSTR_VAL(resource->host), error);
efree(error);
php_url_free(resource);
Expand All @@ -592,13 +538,13 @@ int phar_wrapper_rmdir(php_stream_wrapper *wrapper, const char *url, int options
}

if (!entry->is_deleted) {
for (zend_hash_internal_pointer_reset(&phar->manifest);
HASH_KEY_NON_EXISTENT != zend_hash_get_current_key(&phar->manifest, &str_key, &unused);
zend_hash_move_forward(&phar->manifest)
) {
if (ZSTR_LEN(str_key) > path_len &&
memcmp(ZSTR_VAL(str_key), ZSTR_VAL(resource->path)+1, path_len) == 0 &&
IS_SLASH(ZSTR_VAL(str_key)[path_len])) {
zend_string *str_key;

ZEND_HASH_MAP_FOREACH_STR_KEY(&phar->manifest, str_key) {
if (
zend_string_starts_with_cstr(str_key, ZSTR_VAL(resource->path)+1, path_len)
&& IS_SLASH(ZSTR_VAL(str_key)[path_len])
) {
php_stream_wrapper_log_error(wrapper, options, "phar error: Directory not empty");
if (entry->is_temp_dir) {
efree(entry->filename);
Expand All @@ -607,15 +553,14 @@ int phar_wrapper_rmdir(php_stream_wrapper *wrapper, const char *url, int options
php_url_free(resource);
return 0;
}
}

for (zend_hash_internal_pointer_reset(&phar->virtual_dirs);
HASH_KEY_NON_EXISTENT != zend_hash_get_current_key(&phar->virtual_dirs, &str_key, &unused);
zend_hash_move_forward(&phar->virtual_dirs)) {

if (ZSTR_LEN(str_key) > path_len &&
memcmp(ZSTR_VAL(str_key), ZSTR_VAL(resource->path)+1, path_len) == 0 &&
IS_SLASH(ZSTR_VAL(str_key)[path_len])) {
} ZEND_HASH_FOREACH_END();

ZEND_HASH_MAP_FOREACH_STR_KEY(&phar->virtual_dirs, str_key) {
ZEND_ASSERT(str_key);
if (
zend_string_starts_with_cstr(str_key, ZSTR_VAL(resource->path)+1, path_len)
&& IS_SLASH(ZSTR_VAL(str_key)[path_len])
) {
php_stream_wrapper_log_error(wrapper, options, "phar error: Directory not empty");
if (entry->is_temp_dir) {
efree(entry->filename);
Expand All @@ -624,7 +569,7 @@ int phar_wrapper_rmdir(php_stream_wrapper *wrapper, const char *url, int options
php_url_free(resource);
return 0;
}
}
} ZEND_HASH_FOREACH_END();
}

if (entry->is_temp_dir) {
Expand Down
Loading