Skip to content

Commit 561401c

Browse files
committed
Transform phar_entry_info filename to zend_string
The contents of the string are copied many times, especially in hash tables. Avoid all this work by using zend_string in the first place.
1 parent 725ec83 commit 561401c

File tree

8 files changed

+305
-326
lines changed

8 files changed

+305
-326
lines changed

ext/phar/dirstream.c

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ int phar_wrapper_mkdir(php_stream_wrapper *wrapper, const char *url_from, int mo
398398
if ((e = phar_get_entry_info_dir(phar, ZSTR_VAL(resource->path) + 1, ZSTR_LEN(resource->path) - 1, 2, &error, 1))) {
399399
/* directory exists, or is a subdirectory of an existing file */
400400
if (e->is_temp_dir) {
401-
efree(e->filename);
401+
zend_string_efree(e->filename);
402402
efree(e);
403403
}
404404
php_stream_wrapper_log_error(wrapper, options, "phar error: cannot create directory \"%s\" in phar \"%s\", directory already exists", ZSTR_VAL(resource->path)+1, ZSTR_VAL(resource->host));
@@ -434,14 +434,13 @@ int phar_wrapper_mkdir(php_stream_wrapper *wrapper, const char *url_from, int mo
434434
entry.is_zip = 1;
435435
}
436436

437-
entry.filename = estrdup(ZSTR_VAL(resource->path) + 1);
437+
entry.filename = zend_string_init(ZSTR_VAL(resource->path) + 1, ZSTR_LEN(resource->path) - 1, false);
438438

439439
if (phar->is_tar) {
440440
entry.is_tar = 1;
441441
entry.tar_type = TAR_DIR;
442442
}
443443

444-
entry.filename_len = ZSTR_LEN(resource->path) - 1;
445444
php_url_free(resource);
446445
entry.is_dir = 1;
447446
entry.phar = phar;
@@ -450,23 +449,23 @@ int phar_wrapper_mkdir(php_stream_wrapper *wrapper, const char *url_from, int mo
450449
entry.flags = PHAR_ENT_PERM_DEF_DIR;
451450
entry.old_flags = PHAR_ENT_PERM_DEF_DIR;
452451

453-
if (NULL == zend_hash_str_add_mem(&phar->manifest, entry.filename, entry.filename_len, (void*)&entry, sizeof(phar_entry_info))) {
454-
php_stream_wrapper_log_error(wrapper, options, "phar error: cannot create directory \"%s\" in phar \"%s\", adding to manifest failed", entry.filename, phar->fname);
452+
if (NULL == zend_hash_add_mem(&phar->manifest, entry.filename, &entry, sizeof(phar_entry_info))) {
453+
php_stream_wrapper_log_error(wrapper, options, "phar error: cannot create directory \"%s\" in phar \"%s\", adding to manifest failed", ZSTR_VAL(entry.filename), phar->fname);
455454
efree(error);
456-
efree(entry.filename);
455+
zend_string_efree(entry.filename);
457456
return 0;
458457
}
459458

460459
phar_flush(phar, &error);
461460

462461
if (error) {
463-
php_stream_wrapper_log_error(wrapper, options, "phar error: cannot create directory \"%s\" in phar \"%s\", %s", entry.filename, phar->fname, error);
464-
zend_hash_str_del(&phar->manifest, entry.filename, entry.filename_len);
462+
php_stream_wrapper_log_error(wrapper, options, "phar error: cannot create directory \"%s\" in phar \"%s\", %s", ZSTR_VAL(entry.filename), phar->fname, error);
463+
zend_hash_del(&phar->manifest, entry.filename);
465464
efree(error);
466465
return 0;
467466
}
468467

469-
phar_add_virtual_dirs(phar, entry.filename, entry.filename_len);
468+
phar_add_virtual_dirs(phar, ZSTR_VAL(entry.filename), ZSTR_LEN(entry.filename));
470469
return 1;
471470
}
472471
/* }}} */
@@ -547,7 +546,7 @@ int phar_wrapper_rmdir(php_stream_wrapper *wrapper, const char *url, int options
547546
) {
548547
php_stream_wrapper_log_error(wrapper, options, "phar error: Directory not empty");
549548
if (entry->is_temp_dir) {
550-
efree(entry->filename);
549+
zend_string_efree(entry->filename);
551550
efree(entry);
552551
}
553552
php_url_free(resource);
@@ -563,7 +562,7 @@ int phar_wrapper_rmdir(php_stream_wrapper *wrapper, const char *url, int options
563562
) {
564563
php_stream_wrapper_log_error(wrapper, options, "phar error: Directory not empty");
565564
if (entry->is_temp_dir) {
566-
efree(entry->filename);
565+
zend_string_efree(entry->filename);
567566
efree(entry);
568567
}
569568
php_url_free(resource);
@@ -574,15 +573,15 @@ int phar_wrapper_rmdir(php_stream_wrapper *wrapper, const char *url, int options
574573

575574
if (entry->is_temp_dir) {
576575
zend_hash_str_del(&phar->virtual_dirs, ZSTR_VAL(resource->path)+1, path_len);
577-
efree(entry->filename);
576+
zend_string_efree(entry->filename);
578577
efree(entry);
579578
} else {
580579
entry->is_deleted = 1;
581580
entry->is_modified = 1;
582581
phar_flush(phar, &error);
583582

584583
if (error) {
585-
php_stream_wrapper_log_error(wrapper, options, "phar error: cannot remove directory \"%s\" in phar \"%s\", %s", entry->filename, phar->fname, error);
584+
php_stream_wrapper_log_error(wrapper, options, "phar error: cannot remove directory \"%s\" in phar \"%s\", %s", ZSTR_VAL(entry->filename), phar->fname, error);
586585
php_url_free(resource);
587586
efree(error);
588587
return 0;

ext/phar/phar.c

Lines changed: 42 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ void destroy_phar_manifest_entry_int(phar_entry_info *entry) /* {{{ */
367367

368368
phar_metadata_tracker_free(&entry->metadata_tracker, entry->is_persistent);
369369

370-
pefree(entry->filename, entry->is_persistent);
370+
zend_string_release_ex(entry->filename, entry->is_persistent);
371371

372372
if (entry->link) {
373373
pefree(entry->link, entry->is_persistent);
@@ -424,7 +424,7 @@ void phar_entry_remove(phar_entry_data *idata, char **error) /* {{{ */
424424
if (idata->fp && idata->fp != idata->phar->fp && idata->fp != idata->phar->ufp && idata->fp != idata->internal_file->fp) {
425425
php_stream_close(idata->fp);
426426
}
427-
zend_hash_str_del(&idata->phar->manifest, idata->internal_file->filename, idata->internal_file->filename_len);
427+
zend_hash_del(&idata->phar->manifest, idata->internal_file->filename);
428428
idata->phar->refcount--;
429429
efree(idata);
430430
} else {
@@ -1134,29 +1134,30 @@ static zend_result phar_parse_pharfile(php_stream *fp, char *fname, size_t fname
11341134
MAPPHAR_FAIL("internal corruption of phar \"%s\" (truncated manifest entry)")
11351135
}
11361136

1137-
PHAR_GET_32(buffer, entry.filename_len);
1137+
uint32_t filename_len;
1138+
PHAR_GET_32(buffer, filename_len);
11381139

1139-
if (entry.filename_len == 0) {
1140+
if (filename_len == 0) {
11401141
MAPPHAR_FAIL("zero-length filename encountered in phar \"%s\"");
11411142
}
11421143

11431144
if (entry.is_persistent) {
11441145
entry.manifest_pos = manifest_index;
11451146
}
11461147

1147-
if (entry.filename_len > (size_t)(endbuffer - buffer - 24)) {
1148+
if (filename_len > (size_t)(endbuffer - buffer - 24)) {
11481149
MAPPHAR_FAIL("internal corruption of phar \"%s\" (truncated manifest entry)");
11491150
}
11501151

1151-
if ((manifest_ver & PHAR_API_VER_MASK) >= PHAR_API_MIN_DIR && buffer[entry.filename_len - 1] == '/') {
1152+
if ((manifest_ver & PHAR_API_VER_MASK) >= PHAR_API_MIN_DIR && buffer[filename_len - 1] == '/') {
11521153
entry.is_dir = 1;
11531154
} else {
11541155
entry.is_dir = 0;
11551156
}
11561157

1157-
phar_add_virtual_dirs(mydata, buffer, entry.filename_len);
1158-
entry.filename = pestrndup(buffer, entry.filename_len, entry.is_persistent);
1159-
buffer += entry.filename_len;
1158+
phar_add_virtual_dirs(mydata, buffer, filename_len);
1159+
const char *filename_raw = buffer;
1160+
buffer += filename_len;
11601161
PHAR_GET_32(buffer, entry.uncompressed_filesize);
11611162
PHAR_GET_32(buffer, entry.timestamp);
11621163

@@ -1176,13 +1177,15 @@ static zend_result phar_parse_pharfile(php_stream *fp, char *fname, size_t fname
11761177
PHAR_GET_32(buffer, entry.flags);
11771178

11781179
if (entry.is_dir) {
1179-
entry.filename_len--;
1180+
filename_len--;
11801181
entry.flags |= PHAR_ENT_PERM_DEF_DIR;
11811182
}
11821183

1184+
entry.filename = zend_string_init(filename_raw, filename_len, entry.is_persistent);
1185+
11831186
PHAR_GET_32(buffer, len);
11841187
if (len > (size_t)(endbuffer - buffer)) {
1185-
pefree(entry.filename, entry.is_persistent);
1188+
zend_string_free(entry.filename);
11861189
MAPPHAR_FAIL("internal corruption of phar \"%s\" (truncated manifest entry)");
11871190
}
11881191
/* Don't implicitly call unserialize() on potentially untrusted input unless getMetadata() is called directly. */
@@ -1199,21 +1202,21 @@ static zend_result phar_parse_pharfile(php_stream *fp, char *fname, size_t fname
11991202
case PHAR_ENT_COMPRESSED_GZ:
12001203
if (!PHAR_G(has_zlib)) {
12011204
phar_metadata_tracker_free(&entry.metadata_tracker, entry.is_persistent);
1202-
pefree(entry.filename, entry.is_persistent);
1205+
zend_string_free(entry.filename);
12031206
MAPPHAR_FAIL("zlib extension is required for gz compressed .phar file \"%s\"");
12041207
}
12051208
break;
12061209
case PHAR_ENT_COMPRESSED_BZ2:
12071210
if (!PHAR_G(has_bz2)) {
12081211
phar_metadata_tracker_free(&entry.metadata_tracker, entry.is_persistent);
1209-
pefree(entry.filename, entry.is_persistent);
1212+
zend_string_free(entry.filename);
12101213
MAPPHAR_FAIL("bz2 extension is required for bzip2 compressed .phar file \"%s\"");
12111214
}
12121215
break;
12131216
default:
12141217
if (entry.uncompressed_filesize != entry.compressed_filesize) {
12151218
phar_metadata_tracker_free(&entry.metadata_tracker, entry.is_persistent);
1216-
pefree(entry.filename, entry.is_persistent);
1219+
zend_string_free(entry.filename);
12171220
MAPPHAR_FAIL("internal corruption of phar \"%s\" (compressed and uncompressed size does not match for uncompressed entry)");
12181221
}
12191222
break;
@@ -1223,10 +1226,11 @@ static zend_result phar_parse_pharfile(php_stream *fp, char *fname, size_t fname
12231226
/* if signature matched, no need to check CRC32 for each file */
12241227
entry.is_crc_checked = (manifest_flags & PHAR_HDR_SIGNATURE ? 1 : 0);
12251228
phar_set_inode(&entry);
1229+
// TODO: avoid copy
12261230
if (mydata->is_persistent) {
1227-
str = zend_string_init_interned(entry.filename, entry.filename_len, 1);
1231+
str = zend_string_init_interned(ZSTR_VAL(entry.filename), ZSTR_LEN(entry.filename), 1);
12281232
} else {
1229-
str = zend_string_init(entry.filename, entry.filename_len, 0);
1233+
str = zend_string_init(ZSTR_VAL(entry.filename), ZSTR_LEN(entry.filename), 0);
12301234
}
12311235
zend_hash_add_mem(&mydata->manifest, str, (void*)&entry, sizeof(phar_entry_info));
12321236
zend_string_release(str);
@@ -2390,14 +2394,14 @@ zend_result phar_postprocess_file(phar_entry_data *idata, uint32_t crc32, char *
23902394
phar_zip_data_desc desc;
23912395

23922396
if (SUCCESS != phar_open_archive_fp(idata->phar)) {
2393-
spprintf(error, 0, "phar error: unable to open zip-based phar archive \"%s\" to verify local file header for file \"%s\"", idata->phar->fname, entry->filename);
2397+
spprintf(error, 0, "phar error: unable to open zip-based phar archive \"%s\" to verify local file header for file \"%s\"", idata->phar->fname, ZSTR_VAL(entry->filename));
23942398
return FAILURE;
23952399
}
23962400
php_stream_seek(phar_get_entrypfp(idata->internal_file), entry->header_offset, SEEK_SET);
23972401

23982402
if (sizeof(local) != php_stream_read(phar_get_entrypfp(idata->internal_file), (char *) &local, sizeof(local))) {
23992403

2400-
spprintf(error, 0, "phar error: internal corruption of zip-based phar \"%s\" (cannot read local file header for file \"%s\")", idata->phar->fname, entry->filename);
2404+
spprintf(error, 0, "phar error: internal corruption of zip-based phar \"%s\" (cannot read local file header for file \"%s\")", idata->phar->fname, ZSTR_VAL(entry->filename));
24012405
return FAILURE;
24022406
}
24032407

@@ -2410,7 +2414,7 @@ zend_result phar_postprocess_file(phar_entry_data *idata, uint32_t crc32, char *
24102414
entry->compressed_filesize, SEEK_SET);
24112415
if (sizeof(desc) != php_stream_read(phar_get_entrypfp(idata->internal_file),
24122416
(char *) &desc, sizeof(desc))) {
2413-
spprintf(error, 0, "phar error: internal corruption of zip-based phar \"%s\" (cannot read local data descriptor for file \"%s\")", idata->phar->fname, entry->filename);
2417+
spprintf(error, 0, "phar error: internal corruption of zip-based phar \"%s\" (cannot read local data descriptor for file \"%s\")", idata->phar->fname, ZSTR_VAL(entry->filename));
24142418
return FAILURE;
24152419
}
24162420
if (desc.signature[0] == 'P' && desc.signature[1] == 'K') {
@@ -2421,8 +2425,8 @@ zend_result phar_postprocess_file(phar_entry_data *idata, uint32_t crc32, char *
24212425
}
24222426
}
24232427
/* verify local header */
2424-
if (entry->filename_len != PHAR_ZIP_16(local.filename_len) || entry->crc32 != PHAR_ZIP_32(local.crc32) || entry->uncompressed_filesize != PHAR_ZIP_32(local.uncompsize) || entry->compressed_filesize != PHAR_ZIP_32(local.compsize)) {
2425-
spprintf(error, 0, "phar error: internal corruption of zip-based phar \"%s\" (local header of file \"%s\" does not match central directory)", idata->phar->fname, entry->filename);
2428+
if (ZSTR_LEN(entry->filename) != PHAR_ZIP_16(local.filename_len) || entry->crc32 != PHAR_ZIP_32(local.crc32) || entry->uncompressed_filesize != PHAR_ZIP_32(local.uncompsize) || entry->compressed_filesize != PHAR_ZIP_32(local.compsize)) {
2429+
spprintf(error, 0, "phar error: internal corruption of zip-based phar \"%s\" (local header of file \"%s\" does not match central directory)", idata->phar->fname, ZSTR_VAL(entry->filename));
24262430
return FAILURE;
24272431
}
24282432

@@ -2450,7 +2454,7 @@ zend_result phar_postprocess_file(phar_entry_data *idata, uint32_t crc32, char *
24502454
entry->is_crc_checked = 1;
24512455
return SUCCESS;
24522456
} else {
2453-
spprintf(error, 0, "phar error: internal corruption of phar \"%s\" (crc32 mismatch on file \"%s\")", idata->phar->fname, entry->filename);
2457+
spprintf(error, 0, "phar error: internal corruption of phar \"%s\" (crc32 mismatch on file \"%s\")", idata->phar->fname, ZSTR_VAL(entry->filename));
24542458
return FAILURE;
24552459
}
24562460
}
@@ -2712,7 +2716,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa
27122716
}
27132717
/* after excluding deleted files, calculate manifest size in bytes and number of entries */
27142718
++new_manifest_count;
2715-
phar_add_virtual_dirs(phar, entry->filename, entry->filename_len);
2719+
phar_add_virtual_dirs(phar, ZSTR_VAL(entry->filename), ZSTR_LEN(entry->filename));
27162720

27172721
if (entry->is_dir) {
27182722
/* we use this to calculate API version, 1.1.1 is used for phars with directories */
@@ -2729,7 +2733,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa
27292733
}
27302734

27312735
/* 32 bits for filename length, length of filename, manifest + metadata, and add 1 for trailing / if a directory */
2732-
offset += 4 + entry->filename_len + sizeof(entry_buffer) + (entry->metadata_tracker.str ? ZSTR_LEN(entry->metadata_tracker.str) : 0) + (entry->is_dir ? 1 : 0);
2736+
offset += 4 + ZSTR_LEN(entry->filename) + sizeof(entry_buffer) + (entry->metadata_tracker.str ? ZSTR_LEN(entry->metadata_tracker.str) : 0) + (entry->is_dir ? 1 : 0);
27332737

27342738
/* compress and rehash as necessary */
27352739
if ((oldfile && !entry->is_modified) || entry->is_dir) {
@@ -2757,7 +2761,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa
27572761
}
27582762
php_stream_close(newfile);
27592763
if (error) {
2760-
spprintf(error, 0, "unable to seek to start of file \"%s\" while creating new phar \"%s\"", entry->filename, phar->fname);
2764+
spprintf(error, 0, "unable to seek to start of file \"%s\" while creating new phar \"%s\"", ZSTR_VAL(entry->filename), phar->fname);
27612765
}
27622766
return;
27632767
}
@@ -2778,11 +2782,11 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa
27782782
php_stream_close(newfile);
27792783
if (entry->flags & PHAR_ENT_COMPRESSED_GZ) {
27802784
if (error) {
2781-
spprintf(error, 0, "unable to gzip compress file \"%s\" to new phar \"%s\"", entry->filename, phar->fname);
2785+
spprintf(error, 0, "unable to gzip compress file \"%s\" to new phar \"%s\"", ZSTR_VAL(entry->filename), phar->fname);
27822786
}
27832787
} else {
27842788
if (error) {
2785-
spprintf(error, 0, "unable to bzip2 compress file \"%s\" to new phar \"%s\"", entry->filename, phar->fname);
2789+
spprintf(error, 0, "unable to bzip2 compress file \"%s\" to new phar \"%s\"", ZSTR_VAL(entry->filename), phar->fname);
27862790
}
27872791
}
27882792
return;
@@ -2815,7 +2819,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa
28152819
}
28162820
php_stream_close(newfile);
28172821
if (error) {
2818-
spprintf(error, 0, "unable to seek to start of file \"%s\" while creating new phar \"%s\"", entry->filename, phar->fname);
2822+
spprintf(error, 0, "unable to seek to start of file \"%s\" while creating new phar \"%s\"", ZSTR_VAL(entry->filename), phar->fname);
28192823
}
28202824
goto cleanup;
28212825
}
@@ -2826,7 +2830,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa
28262830
}
28272831
php_stream_close(newfile);
28282832
if (error) {
2829-
spprintf(error, 0, "unable to copy compressed file contents of file \"%s\" while creating new phar \"%s\"", entry->filename, phar->fname);
2833+
spprintf(error, 0, "unable to copy compressed file contents of file \"%s\" while creating new phar \"%s\"", ZSTR_VAL(entry->filename), phar->fname);
28302834
}
28312835
goto cleanup;
28322836
}
@@ -2929,23 +2933,23 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa
29292933

29302934
if (entry->is_dir) {
29312935
/* add 1 for trailing slash */
2932-
phar_set_32(entry_buffer, entry->filename_len + 1);
2936+
phar_set_32(entry_buffer, ZSTR_LEN(entry->filename) + 1);
29332937
} else {
2934-
phar_set_32(entry_buffer, entry->filename_len);
2938+
phar_set_32(entry_buffer, ZSTR_LEN(entry->filename));
29352939
}
29362940

29372941
if (4 != php_stream_write(newfile, entry_buffer, 4)
2938-
|| entry->filename_len != php_stream_write(newfile, entry->filename, entry->filename_len)
2942+
|| ZSTR_LEN(entry->filename) != php_stream_write(newfile, ZSTR_VAL(entry->filename), ZSTR_LEN(entry->filename))
29392943
|| (entry->is_dir && 1 != php_stream_write(newfile, "/", 1))) {
29402944
if (must_close_old_file) {
29412945
php_stream_close(oldfile);
29422946
}
29432947
php_stream_close(newfile);
29442948
if (error) {
29452949
if (entry->is_dir) {
2946-
spprintf(error, 0, "unable to write filename of directory \"%s\" to manifest of new phar \"%s\"", entry->filename, phar->fname);
2950+
spprintf(error, 0, "unable to write filename of directory \"%s\" to manifest of new phar \"%s\"", ZSTR_VAL(entry->filename), phar->fname);
29472951
} else {
2948-
spprintf(error, 0, "unable to write filename of file \"%s\" to manifest of new phar \"%s\"", entry->filename, phar->fname);
2952+
spprintf(error, 0, "unable to write filename of file \"%s\" to manifest of new phar \"%s\"", ZSTR_VAL(entry->filename), phar->fname);
29492953
}
29502954
}
29512955
goto cleanup;
@@ -2979,7 +2983,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa
29792983
php_stream_close(newfile);
29802984

29812985
if (error) {
2982-
spprintf(error, 0, "unable to write temporary manifest of file \"%s\" to manifest of new phar \"%s\"", entry->filename, phar->fname);
2986+
spprintf(error, 0, "unable to write temporary manifest of file \"%s\" to manifest of new phar \"%s\"", ZSTR_VAL(entry->filename), phar->fname);
29832987
}
29842988

29852989
goto cleanup;
@@ -3020,7 +3024,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa
30203024
}
30213025
php_stream_close(newfile);
30223026
if (error) {
3023-
spprintf(error, 0, "unable to seek to start of file \"%s\" while creating new phar \"%s\"", entry->filename, phar->fname);
3027+
spprintf(error, 0, "unable to seek to start of file \"%s\" while creating new phar \"%s\"", ZSTR_VAL(entry->filename), phar->fname);
30243028
}
30253029
goto cleanup;
30263030
}
@@ -3032,7 +3036,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa
30323036
}
30333037
php_stream_close(newfile);
30343038
if (error) {
3035-
spprintf(error, 0, "unable to seek to start of file \"%s\" while creating new phar \"%s\"", entry->filename, phar->fname);
3039+
spprintf(error, 0, "unable to seek to start of file \"%s\" while creating new phar \"%s\"", ZSTR_VAL(entry->filename), phar->fname);
30363040
}
30373041
goto cleanup;
30383042
}
@@ -3048,7 +3052,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa
30483052
php_stream_close(newfile);
30493053

30503054
if (error) {
3051-
spprintf(error, 0, "unable to write contents of file \"%s\" to new phar \"%s\"", entry->filename, phar->fname);
3055+
spprintf(error, 0, "unable to write contents of file \"%s\" to new phar \"%s\"", ZSTR_VAL(entry->filename), phar->fname);
30523056
}
30533057

30543058
goto cleanup;

0 commit comments

Comments
 (0)