Skip to content

Commit fbac7a3

Browse files
authored
phar: Get rid of bailouts in Phar::webPhar() (php#20190)
Bailouts are bad because they stop the GC etc. They also hide leaks. This makes the behaviour equivalent to exit(), as it's meant to stop the request (which is why bailout was used). We also have to fix some leaks that pop up then.
1 parent 984416d commit fbac7a3

File tree

1 file changed

+29
-16
lines changed

1 file changed

+29
-16
lines changed

ext/phar/phar_object.c

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,7 @@ PHP_METHOD(Phar, webPhar)
699699
zval params, retval;
700700

701701
ZVAL_STRINGL(&params, entry, entry_len);
702+
efree(entry);
702703

703704
rewrite_fci.param_count = 1;
704705
rewrite_fci.params = &params;
@@ -716,9 +717,10 @@ PHP_METHOD(Phar, webPhar)
716717

717718
switch (Z_TYPE(retval)) {
718719
case IS_STRING:
719-
efree(entry);
720+
/* TODO: avoid relocation??? */
720721
entry = estrndup(Z_STRVAL_P(rewrite_fci.retval), Z_STRLEN_P(rewrite_fci.retval));
721722
entry_len = Z_STRLEN_P(rewrite_fci.retval);
723+
zval_ptr_dtor_str(&retval);
722724
break;
723725
case IS_TRUE:
724726
case IS_FALSE:
@@ -729,15 +731,16 @@ PHP_METHOD(Phar, webPhar)
729731
}
730732
efree(pt);
731733

732-
zend_bailout();
734+
zend_throw_unwind_exit();
735+
return;
733736
default:
737+
zval_ptr_dtor(&retval);
734738
zend_throw_exception_ex(phar_ce_PharException, 0, "phar error: rewrite callback must return a string or false");
735739

736740
cleanup_fail:
737741
if (free_pathinfo) {
738742
efree(path_info);
739743
}
740-
efree(entry);
741744
efree(pt);
742745
#ifdef PHP_WIN32
743746
efree(fname);
@@ -752,29 +755,28 @@ PHP_METHOD(Phar, webPhar)
752755

753756
if (!entry_len || (entry_len == 1 && entry[0] == '/')) {
754757
efree(entry);
758+
efree(pt);
759+
760+
bool is_entry_allocated = false;
761+
755762
/* direct request */
756763
if (index_php_len) {
757764
entry = index_php;
758765
entry_len = index_php_len;
759766
if (entry[0] != '/') {
760767
spprintf(&entry, 0, "/%s", index_php);
761768
++entry_len;
769+
is_entry_allocated = true;
762770
}
763771
} else {
764772
/* assume "index.php" is starting point */
765-
entry = estrndup("/index.php", sizeof("/index.php"));
773+
entry = "/index.php";
766774
entry_len = sizeof("/index.php")-1;
767775
}
768776

769777
if (FAILURE == phar_get_archive(&phar, fname, fname_len, NULL, 0, NULL) ||
770778
(info = phar_get_entry_info(phar, entry, entry_len, NULL, false)) == NULL) {
771779
phar_do_404(phar, fname, fname_len, f404);
772-
773-
if (free_pathinfo) {
774-
efree(path_info);
775-
}
776-
777-
zend_bailout();
778780
} else {
779781
char *tmp = NULL, sa = '\0';
780782
sapi_header_line ctr = {0};
@@ -801,21 +803,32 @@ PHP_METHOD(Phar, webPhar)
801803
*tmp = sa;
802804
}
803805

804-
if (free_pathinfo) {
805-
efree(path_info);
806-
}
807-
808806
sapi_header_op(SAPI_HEADER_REPLACE, &ctr);
809807
sapi_send_headers();
810808
efree((void *) ctr.line);
811-
zend_bailout();
812809
}
810+
811+
if (is_entry_allocated) {
812+
efree(entry);
813+
}
814+
if (free_pathinfo) {
815+
efree(path_info);
816+
}
817+
818+
zend_throw_unwind_exit();
819+
return;
813820
}
814821

815822
if (FAILURE == phar_get_archive(&phar, fname, fname_len, NULL, 0, NULL) ||
816823
(info = phar_get_entry_info(phar, entry, entry_len, NULL, false)) == NULL) {
824+
efree(entry);
825+
efree(pt);
826+
if (free_pathinfo) {
827+
efree(path_info);
828+
}
817829
phar_do_404(phar, fname, fname_len, f404);
818-
zend_bailout();
830+
zend_throw_unwind_exit();
831+
return;
819832
}
820833

821834
if (mimeoverride && zend_hash_num_elements(Z_ARRVAL_P(mimeoverride))) {

0 commit comments

Comments
 (0)