Skip to content

Commit 619d293

Browse files
authored
Reduce number of string duplications in phar (#17814)
We don't need to duplicate these strings from the resource, we can just use them with an offset. To prove this was safe, I had to make the arguments const and then propagate that everywhere, so this patch also introduces some more constness.
1 parent c7d62cf commit 619d293

File tree

5 files changed

+21
-32
lines changed

5 files changed

+21
-32
lines changed

ext/phar/phar_internal.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -420,8 +420,8 @@ char *phar_decompress_filter(phar_entry_info * entry, int return_unknown);
420420
char *phar_compress_filter(phar_entry_info * entry, int return_unknown);
421421

422422
/* void phar_remove_virtual_dirs(phar_archive_data *phar, char *filename, size_t filename_len); */
423-
void phar_add_virtual_dirs(phar_archive_data *phar, char *filename, size_t filename_len);
424-
zend_result phar_mount_entry(phar_archive_data *phar, char *filename, size_t filename_len, char *path, size_t path_len);
423+
void phar_add_virtual_dirs(phar_archive_data *phar, const char *filename, size_t filename_len);
424+
zend_result phar_mount_entry(phar_archive_data *phar, char *filename, size_t filename_len, const char *path, size_t path_len);
425425
zend_string *phar_find_in_include_path(zend_string *file, phar_archive_data **pphar);
426426
char *phar_fix_filepath(char *path, size_t *new_len, int use_cwd);
427427
phar_entry_info * phar_open_jit(phar_archive_data *phar, phar_entry_info *entry, char **error);
@@ -463,10 +463,10 @@ extern HashTable cached_alias;
463463
bool phar_archive_delref(phar_archive_data *phar);
464464
void phar_entry_delref(phar_entry_data *idata);
465465

466-
phar_entry_info *phar_get_entry_info(phar_archive_data *phar, char *path, size_t path_len, char **error, int security);
467-
phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, char *path, size_t path_len, char dir, char **error, int security);
468-
phar_entry_data *phar_get_or_create_entry_data(char *fname, size_t fname_len, char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security);
469-
zend_result phar_get_entry_data(phar_entry_data **ret, char *fname, size_t fname_len, char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security);
466+
phar_entry_info *phar_get_entry_info(phar_archive_data *phar, const char *path, size_t path_len, char **error, int security);
467+
phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, const char *path, size_t path_len, char dir, char **error, int security);
468+
phar_entry_data *phar_get_or_create_entry_data(char *fname, size_t fname_len, const char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security);
469+
zend_result phar_get_entry_data(phar_entry_data **ret, char *fname, size_t fname_len, const char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security);
470470
void phar_flush_ex(phar_archive_data *archive, zend_string *user_stub, bool is_default_stub, char **error);
471471
void phar_flush(phar_archive_data *archive, char **error);
472472
zend_result phar_detect_phar_fname_ext(const char *filename, size_t filename_len, const char **ext_str, size_t *ext_len, int executable, int for_create, int is_complete);
@@ -484,6 +484,6 @@ typedef enum {
484484
pcr_err_empty_entry
485485
} phar_path_check_result;
486486

487-
phar_path_check_result phar_path_check(char **p, size_t *len, const char **error);
487+
phar_path_check_result phar_path_check(const char **p, size_t *len, const char **error);
488488

489489
END_EXTERN_C()

ext/phar/phar_object.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1976,7 +1976,7 @@ static zend_result phar_copy_file_contents(phar_entry_info *entry, php_stream *f
19761976
}
19771977
/* }}} */
19781978

1979-
static zend_object *phar_rename_archive(phar_archive_data **sphar, char *ext) /* {{{ */
1979+
static zend_object *phar_rename_archive(phar_archive_data **sphar, const char *ext) /* {{{ */
19801980
{
19811981
const char *oldname = NULL;
19821982
phar_archive_data *phar = *sphar;
@@ -3484,7 +3484,7 @@ PHP_METHOD(Phar, copy)
34843484
}
34853485

34863486
size_t tmp_len = ZSTR_LEN(new_file);
3487-
char *tmp_new_file = ZSTR_VAL(new_file);
3487+
const char *tmp_new_file = ZSTR_VAL(new_file);
34883488
if (phar_path_check(&tmp_new_file, &tmp_len, &pcr_error) > pcr_is_ok) {
34893489
zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0,
34903490
"file \"%s\" contains invalid characters %s, cannot be copied from \"%s\" in phar %s", ZSTR_VAL(new_file), pcr_error, ZSTR_VAL(old_file), phar_obj->archive->fname);

ext/phar/phar_path_check.re

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
#include "phar_internal.h"
2020

21-
phar_path_check_result phar_path_check(char **s, size_t *len, const char **error)
21+
phar_path_check_result phar_path_check(const char **s, size_t *len, const char **error)
2222
{
2323
const unsigned char *p = (const unsigned char*)*s;
2424
const unsigned char *m;

ext/phar/stream.c

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha
162162
{
163163
phar_archive_data *phar;
164164
phar_entry_data *idata;
165-
char *internal_file;
165+
const char *internal_file;
166166
char *error;
167167
HashTable *pharcontext;
168168
php_url *resource = NULL;
@@ -189,7 +189,7 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha
189189
phar_request_initialize();
190190

191191
/* strip leading "/" */
192-
internal_file = estrndup(ZSTR_VAL(resource->path) + 1, ZSTR_LEN(resource->path) - 1);
192+
internal_file = ZSTR_VAL(resource->path) + 1;
193193
if (mode[0] == 'w' || (mode[0] == 'r' && mode[1] == '+')) {
194194
if (NULL == (idata = phar_get_or_create_entry_data(ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), internal_file, strlen(internal_file), mode, 0, &error, 1))) {
195195
if (error) {
@@ -198,7 +198,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha
198198
} else {
199199
php_stream_wrapper_log_error(wrapper, options, "phar error: file \"%s\" could not be created in phar \"%s\"", internal_file, ZSTR_VAL(resource->host));
200200
}
201-
efree(internal_file);
202201
php_url_free(resource);
203202
return NULL;
204203
}
@@ -207,7 +206,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha
207206
}
208207
fpf = php_stream_alloc(&phar_ops, idata, NULL, mode);
209208
php_url_free(resource);
210-
efree(internal_file);
211209

212210
if (context && Z_TYPE(context->options) != IS_UNDEF && (pzoption = zend_hash_str_find(HASH_OF(&context->options), "phar", sizeof("phar")-1)) != NULL) {
213211
pharcontext = HASH_OF(pzoption);
@@ -237,15 +235,13 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha
237235
/* retrieve the stub */
238236
if (FAILURE == phar_get_archive(&phar, ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), NULL, 0, NULL)) {
239237
php_stream_wrapper_log_error(wrapper, options, "file %s is not a valid phar archive", ZSTR_VAL(resource->host));
240-
efree(internal_file);
241238
php_url_free(resource);
242239
return NULL;
243240
}
244241
if (phar->is_tar || phar->is_zip) {
245242
if ((FAILURE == phar_get_entry_data(&idata, ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), ".phar/stub.php", sizeof(".phar/stub.php")-1, "r", 0, &error, 0)) || !idata) {
246243
goto idata_error;
247244
}
248-
efree(internal_file);
249245
if (opened_path) {
250246
*opened_path = strpprintf(MAXPATHLEN, "%s", phar->fname);
251247
}
@@ -256,7 +252,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha
256252
if (stream == NULL) {
257253
if (UNEXPECTED(FAILURE == phar_open_archive_fp(phar))) {
258254
php_stream_wrapper_log_error(wrapper, options, "phar error: could not reopen phar \"%s\"", ZSTR_VAL(resource->host));
259-
efree(internal_file);
260255
php_url_free(resource);
261256
return NULL;
262257
}
@@ -285,7 +280,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha
285280
if (opened_path) {
286281
*opened_path = strpprintf(MAXPATHLEN, "%s", phar->fname);
287282
}
288-
efree(internal_file);
289283
goto phar_stub;
290284
}
291285
}
@@ -298,7 +292,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha
298292
} else {
299293
php_stream_wrapper_log_error(wrapper, options, "phar error: \"%s\" is not a file in phar \"%s\"", internal_file, ZSTR_VAL(resource->host));
300294
}
301-
efree(internal_file);
302295
php_url_free(resource);
303296
return NULL;
304297
}
@@ -317,7 +310,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha
317310
php_stream_wrapper_log_error(wrapper, options, "%s", error);
318311
efree(error);
319312
phar_entry_delref(idata);
320-
efree(internal_file);
321313
return NULL;
322314
}
323315

@@ -340,7 +332,6 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha
340332
if (opened_path) {
341333
*opened_path = zend_strpprintf_unchecked(MAXPATHLEN, "phar://%s/%S", idata->phar->fname, idata->internal_file->filename);
342334
}
343-
efree(internal_file);
344335
phar_stub:
345336
fpf = php_stream_alloc(&phar_ops, idata, NULL, mode);
346337
return fpf;
@@ -662,7 +653,8 @@ static int phar_wrapper_stat(php_stream_wrapper *wrapper, const char *url, int f
662653
static int phar_wrapper_unlink(php_stream_wrapper *wrapper, const char *url, int options, php_stream_context *context) /* {{{ */
663654
{
664655
php_url *resource;
665-
char *internal_file, *error;
656+
const char *internal_file;
657+
char *error;
666658
int internal_file_len;
667659
phar_entry_data *idata;
668660
phar_archive_data *pphar;
@@ -695,7 +687,7 @@ static int phar_wrapper_unlink(php_stream_wrapper *wrapper, const char *url, int
695687
}
696688

697689
/* need to copy to strip leading "/", will get touched again */
698-
internal_file = estrndup(ZSTR_VAL(resource->path) + 1, ZSTR_LEN(resource->path) - 1);
690+
internal_file = ZSTR_VAL(resource->path) + 1;
699691
internal_file_len = ZSTR_LEN(resource->path) - 1;
700692
if (FAILURE == phar_get_entry_data(&idata, ZSTR_VAL(resource->host), ZSTR_LEN(resource->host), internal_file, internal_file_len, "r", 0, &error, 1)) {
701693
/* constraints of fp refcount were not met */
@@ -705,7 +697,6 @@ static int phar_wrapper_unlink(php_stream_wrapper *wrapper, const char *url, int
705697
} else {
706698
php_stream_wrapper_log_error(wrapper, options, "unlink of \"%s\" failed, file does not exist", url);
707699
}
708-
efree(internal_file);
709700
php_url_free(resource);
710701
return 0;
711702
}
@@ -715,13 +706,11 @@ static int phar_wrapper_unlink(php_stream_wrapper *wrapper, const char *url, int
715706
if (idata->internal_file->fp_refcount > 1) {
716707
/* more than just our fp resource is open for this file */
717708
php_stream_wrapper_log_error(wrapper, options, "phar error: \"%s\" in phar \"%s\", has open file pointers, cannot unlink", internal_file, ZSTR_VAL(resource->host));
718-
efree(internal_file);
719709
php_url_free(resource);
720710
phar_entry_delref(idata);
721711
return 0;
722712
}
723713
php_url_free(resource);
724-
efree(internal_file);
725714
phar_entry_remove(idata, &error);
726715
if (error) {
727716
php_stream_wrapper_log_error(wrapper, options, "%s", error);

ext/phar/util.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ int phar_seek_efp(phar_entry_info *entry, zend_off_t offset, int whence, zend_of
189189
/* }}} */
190190

191191
/* mount an absolute path or uri to a path internal to the phar archive */
192-
zend_result phar_mount_entry(phar_archive_data *phar, char *filename, size_t filename_len, char *path, size_t path_len) /* {{{ */
192+
zend_result phar_mount_entry(phar_archive_data *phar, char *filename, size_t filename_len, const char *path, size_t path_len) /* {{{ */
193193
{
194194
phar_entry_info entry = {0};
195195
php_stream_statbuf ssb;
@@ -472,7 +472,7 @@ static zend_result phar_separate_entry_fp(phar_entry_info *entry, char **error)
472472
* appended, truncated, or read. For read, if the entry is marked unmodified, it is
473473
* assumed that the file pointer, if present, is opened for reading
474474
*/
475-
zend_result phar_get_entry_data(phar_entry_data **ret, char *fname, size_t fname_len, char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security) /* {{{ */
475+
zend_result phar_get_entry_data(phar_entry_data **ret, char *fname, size_t fname_len, const char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security) /* {{{ */
476476
{
477477
phar_archive_data *phar;
478478
phar_entry_info *entry;
@@ -632,7 +632,7 @@ zend_result phar_get_entry_data(phar_entry_data **ret, char *fname, size_t fname
632632
/**
633633
* Create a new dummy file slot within a writeable phar for a newly created file
634634
*/
635-
phar_entry_data *phar_get_or_create_entry_data(char *fname, size_t fname_len, char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security) /* {{{ */
635+
phar_entry_data *phar_get_or_create_entry_data(char *fname, size_t fname_len, const char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security) /* {{{ */
636636
{
637637
phar_archive_data *phar;
638638
phar_entry_info *entry, etemp;
@@ -1234,7 +1234,7 @@ char * phar_decompress_filter(phar_entry_info * entry, int return_unknown) /* {{
12341234
/**
12351235
* retrieve information on a file contained within a phar, or null if it ain't there
12361236
*/
1237-
phar_entry_info *phar_get_entry_info(phar_archive_data *phar, char *path, size_t path_len, char **error, int security) /* {{{ */
1237+
phar_entry_info *phar_get_entry_info(phar_archive_data *phar, const char *path, size_t path_len, char **error, int security) /* {{{ */
12381238
{
12391239
return phar_get_entry_info_dir(phar, path, path_len, 0, error, security);
12401240
}
@@ -1245,7 +1245,7 @@ phar_entry_info *phar_get_entry_info(phar_archive_data *phar, char *path, size_t
12451245
* valid pre-existing empty directory entries
12461246
*/
12471247
// TODO: convert this to use zend_string too
1248-
phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, char *path, size_t path_len, char dir, char **error, int security) /* {{{ */
1248+
phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, const char *path, size_t path_len, char dir, char **error, int security) /* {{{ */
12491249
{
12501250
const char *pcr_error;
12511251
phar_entry_info *entry;
@@ -2006,7 +2006,7 @@ zend_result phar_create_signature(phar_archive_data *phar, php_stream *fp, char
20062006
/* }}} */
20072007

20082008
// TODO: convert this to zend_string too
2009-
void phar_add_virtual_dirs(phar_archive_data *phar, char *filename, size_t filename_len) /* {{{ */
2009+
void phar_add_virtual_dirs(phar_archive_data *phar, const char *filename, size_t filename_len) /* {{{ */
20102010
{
20112011
const char *s;
20122012
zend_string *str;

0 commit comments

Comments
 (0)