Skip to content

Commit

Permalink
Fixed bug #62294
Browse files Browse the repository at this point in the history 
The primary issue was already resolved in 7c3e487,
but the particular example used in this bug report ran into an
additional issue on PHP 8, because I forgot to drop a number of
zend_bailout calls when switch require failure to throw.
  • Loading branch information
@nikic
nikic committed Aug 10, 2020
1 parent 7c3e487 commit c439f1f
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 6 deletions.
2 changes: 2 additions & 0 deletions NEWS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ PHP NEWS
(Nikita)
. Fixed bug #65275 (Calling exit() in a shutdown function does not change the
exit value in CLI). (Nikita)
. Fixed bug #62294 (register_shutdown_function() does not correctly handle
exit code). (Nikita)

- Date:
. Fixed bug #60302 (DateTime::createFromFormat should new static(), not new
Expand Down
1 change: 0 additions & 1 deletion Zend/zend_language_scanner.l
View file
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,6 @@ ZEND_API zend_op_array *compile_file(zend_file_handle *file_handle, int type)
if (!EG(exception)) {
if (type==ZEND_REQUIRE) {
zend_message_dispatcher(ZMSG_FAILED_REQUIRE_FOPEN, file_handle->filename);
zend_bailout();
} else {
zend_message_dispatcher(ZMSG_FAILED_INCLUDE_FOPEN, file_handle->filename);
}
Expand Down
4 changes: 0 additions & 4 deletions ext/opcache/ZendAccelerator.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1716,7 +1716,6 @@ static zend_persistent_script *opcache_compile_file(zend_file_handle *file_handl
if (!EG(exception)) {
if (type == ZEND_REQUIRE) {
zend_message_dispatcher(ZMSG_FAILED_REQUIRE_FOPEN, file_handle->filename);
zend_bailout();
} else {
zend_message_dispatcher(ZMSG_FAILED_INCLUDE_FOPEN, file_handle->filename);
}
Expand Down Expand Up @@ -1875,7 +1874,6 @@ zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int type)
if (!EG(exception)) {
if (type == ZEND_REQUIRE) {
zend_message_dispatcher(ZMSG_FAILED_REQUIRE_FOPEN, file_handle->filename);
zend_bailout();
} else {
zend_message_dispatcher(ZMSG_FAILED_INCLUDE_FOPEN, file_handle->filename);
}
Expand Down Expand Up @@ -2032,7 +2030,6 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
if (!EG(exception)) {
if (type == ZEND_REQUIRE) {
zend_message_dispatcher(ZMSG_FAILED_REQUIRE_FOPEN, file_handle->filename);
zend_bailout();
} else {
zend_message_dispatcher(ZMSG_FAILED_INCLUDE_FOPEN, file_handle->filename);
}
Expand Down Expand Up @@ -2090,7 +2087,6 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
if (!EG(exception)) {
if (type == ZEND_REQUIRE) {
zend_message_dispatcher(ZMSG_FAILED_REQUIRE_FOPEN, file_handle->filename);
zend_bailout();
} else {
zend_message_dispatcher(ZMSG_FAILED_INCLUDE_FOPEN, file_handle->filename);
}
Expand Down
5 changes: 5 additions & 0 deletions sapi/cli/tests/bug62294.inc
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<?php

register_shutdown_function(function() {
require 'path/to/an/unknown/file';
});
12 changes: 12 additions & 0 deletions sapi/cli/tests/bug62294.phpt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
Bug #62294: register_shutdown_function() does not handle exit code correctly
--FILE--
<?php

$php = getenv('TEST_PHP_EXECUTABLE');
exec($php . ' ' . __DIR__ . '/bug62294.inc', $output, $exit_status);
var_dump($exit_status);

?>
--EXPECT--
int(255)
1 change: 0 additions & 1 deletion sapi/phpdbg/phpdbg_list.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ zend_op_array *phpdbg_compile_file(zend_file_handle *file, int type) {
if (zend_stream_fixup(file, &bufptr, &len) == FAILURE) {
if (type == ZEND_REQUIRE) {
zend_message_dispatcher(ZMSG_FAILED_REQUIRE_FOPEN, file->filename);
zend_bailout();
} else {
zend_message_dispatcher(ZMSG_FAILED_INCLUDE_FOPEN, file->filename);
}
Expand Down

5 comments on commit c439f1f

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

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

@nikic looks like this test is failing

TEST FAILURE: ../sapi/cli/tests/bug62294.diff --

001+ PHP Warning:  require(path/to/an/unknown/file): Failed to open stream: No such file or directory in /builddir/build/BUILD/php-8.0.0beta2/sapi/cli/tests/bug62294.inc on line 4
002+ PHP Fatal error:  Uncaught Error: Failed opening required 'path/to/an/unknown/file' (include_path='.:/opt/remi/php80/root/usr/share/pear:/opt/remi/php80/root/usr/share/php:/usr/share/pear:/usr/share/php') in /builddir/build/BUILD/php-8.0.0beta2/sapi/cli/tests/bug62294.inc:4
003+ Stack trace:
004+ #0 [internal function]: {closure}()
005+ #1 {main}
006+   thrown in /builddir/build/BUILD/php-8.0.0beta2/sapi/cli/tests/bug62294.inc on line 4
     int(255)

@carusogabriel
Copy link
Contributor

Choose a reason for hiding this comment

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

@remicollet What environment did it failed for you?

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

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

Linux (Fedora 31 to 33, RHEL 7 and 8)

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

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

Strangely, the exception is properly raised in all case.

Using a local build, the test passes:

$ TEST_PHP_EXECUTABLE=$PWD/sapi/cli/php sapi/cli/php -n sapi/cli/tests/bug62294.phpt
--TEST--
Bug #62294: register_shutdown_function() does not handle exit code correctly
--FILE--
int(255)
--EXPECT--
int(255)

While the RPM build see the exception

$ TEST_PHP_EXECUTABLE=$(which php80) php80 -n sapi/cli/tests/bug62294.phpt
--TEST--
Bug #62294: register_shutdown_function() does not handle exit code correctly
--FILE--
PHP Warning:  require(path/to/an/unknown/file): Failed to open stream: No such file or directory in /work/build/phpmaster/sapi/cli/tests/bug62294.inc on line 4
PHP Fatal error:  Uncaught Error: Failed opening required 'path/to/an/unknown/file' (include_path='.:/opt/remi/php80/root/usr/share/pear:/opt/remi/php80/root/usr/share/php:/usr/share/pear:/usr/share/php') in /work/build/phpmaster/sapi/cli/tests/bug62294.inc:4
Stack trace:
#0 [internal function]: {closure}()
#1 {main}
  thrown in /work/build/phpmaster/sapi/cli/tests/bug62294.inc on line 4
int(255)
--EXPECT--
int(255)

But I can't see any explanation for this behavior...

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed by 8c57474

Please sign in to comment.