Skip to content

Issue 43177 #98

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

Closed
wants to merge 6 commits into from
Closed

Issue 43177 #98

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a65037e
If an eval() has a parse error, the overall exit status and return
truthptn Jun 1, 2012
87b5158
Merge remote branch 'upstream/master' into issue-43177
Jun 7, 2012
03f4804
Making sure opline exists and opcode is ZEND_INCLUDE_OR_EVAL before
Jun 8, 2012
79ebc24
Also need to confirm current_execute_data is non-null
Jun 8, 2012
5cdb988
Moving variable definition to start of code block.
Aug 21, 2012
511cb36
Merge remote branch 'upstream/master' into issue-43177
Aug 21, 2012
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
5 changes: 4 additions & 1 deletion Zend/zend.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1218,7 +1218,10 @@ ZEND_API void zend_error(int type, const char *format, ...) /* {{{ */
va_end(args);

if (type == E_PARSE) {
EG(exit_status) = 255;
/* eval() errors do not affect exit_status */
if (EG(current_execute_data)->opline->extended_value != ZEND_EVAL) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure this is safe - what guarantees that EG(current_execute_data)->opline is valid for every call to zend_error?

For example, this segfaults: php -r '0+'

Copy link
Member

Choose a reason for hiding this comment

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

Also I'm not sure whether just checking the extended value is safe. ZEND_EVAL is 1 and I imagine that this extended value is also used for other opcodes in other circumstances. So you should probably additionally check that the opcode is ZEND_INCLUDE_OR_EVAL.

Copy link
Author

Choose a reason for hiding this comment

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

Glad someone who knows how internals works looked at this! I've updated the code to check for the existence of everything being used before using it. In main.c, I introduced a local boolean. I guessed the type should be "zend_bool", but that was just a guess.

php -r '0+' no longer segfaults. Bad evals are harmless. Other parse errors are fatal (but don't segfault).

EG(exit_status) = 255;
}
zend_init_compiler_data_structures(TSRMLS_C);
}
}
Expand Down
9 changes: 7 additions & 2 deletions main/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1124,11 +1124,16 @@ static void php_error_cb(int type, const char *error_filename, const uint error_
case E_PARSE:
case E_COMPILE_ERROR:
case E_USER_ERROR:
EG(exit_status) = 255;
/* eval() errors do not affect exit_status */
if (EG(current_execute_data)->opline->extended_value != ZEND_EVAL) {
EG(exit_status) = 255;
}
if (module_initialized) {
if (!PG(display_errors) &&
!SG(headers_sent) &&
SG(sapi_headers).http_response_code == 200
SG(sapi_headers).http_response_code == 200 &&
/* eval() errors do not affect response code */
EG(current_execute_data)->opline->extended_value != ZEND_EVAL
) {
sapi_header_line ctr = {0};

Expand Down