Bug #43177: If an eval() has a parse error, the overall exit status and return code should not be affected.

Without this fix, a webpage using eval() may return code 500.  That might display
fine and the 500 go unnoticed, but using AJAX or wget, the 500 will cause problems.

Author: smalyshev

Zend/zend.c

@@ -1238,7 +1238,13 @@ 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) &&
+			EG(current_execute_data)->opline &&
+			EG(current_execute_data)->opline->opcode == ZEND_INCLUDE_OR_EVAL &&
+			EG(current_execute_data)->opline->extended_value == ZEND_EVAL)) {
+			EG(exit_status) = 255;
+		}
 		zend_init_compiler_data_structures(TSRMLS_C);
 	}
 }

main/main.c

@@ -257,7 +257,7 @@ static void php_disable_classes(TSRMLS_D)
 
 /* {{{ php_binary_init
  */
-static void php_binary_init(TSRMLS_D) 
+static void php_binary_init(TSRMLS_D)
 {
 	char *binary_location;
 #ifdef PHP_WIN32
@@ -845,7 +845,7 @@ PHPAPI void php_verror(const char *docref, const char *params, int type, const c
 		efree(docref_buf);
 	}
 
-	if (PG(track_errors) && module_initialized && 
+	if (PG(track_errors) && module_initialized &&
 			(!EG(user_error_handler) || !(EG(user_error_handler_error_reporting) & type))) {
 		if (!EG(active_symbol_table)) {
 			zend_rebuild_symbol_table(TSRMLS_C);
@@ -962,7 +962,7 @@ static void php_error_cb(int type, const char *error_filename, const uint error_
 	/* store the error if it has changed */
 	if (display) {
 #ifdef ZEND_SIGNALS
-		HANDLE_BLOCK_INTERRUPTIONS();	
+		HANDLE_BLOCK_INTERRUPTIONS();
 #endif
 		if (PG(last_error_message)) {
 			free(PG(last_error_message));
@@ -1133,11 +1133,20 @@ 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;
+		{ /* new block to allow variable definition */
+			/* eval() errors do not affect exit_status or response code */
+			zend_bool during_eval = (type == E_PARSE) && (EG(current_execute_data) &&
+						EG(current_execute_data)->opline &&
+						EG(current_execute_data)->opline->opcode == ZEND_INCLUDE_OR_EVAL &&
+						EG(current_execute_data)->opline->extended_value == ZEND_EVAL);
+			if (!during_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 &&
+				    !during_eval
 				) {
 					sapi_header_line ctr = {0};
 
@@ -1158,6 +1167,7 @@ static void php_error_cb(int type, const char *error_filename, const uint error_
 				}
 			}
 			break;
+		}
 	}
 
 	/* Log if necessary */
@@ -1211,7 +1221,7 @@ PHPAPI char *php_get_current_user(TSRMLS_D)
 		name[len] = '\0';
 		SG(request_info).current_user_length = len;
 		SG(request_info).current_user = estrndup(name, len);
-		return SG(request_info).current_user;		
+		return SG(request_info).current_user;
 #else
 		struct passwd *pwd;
 #if defined(ZTS) && defined(HAVE_GETPWUID_R) && defined(_SC_GETPW_R_SIZE_MAX)
@@ -1239,9 +1249,9 @@ PHPAPI char *php_get_current_user(TSRMLS_D)
 #if defined(ZTS) && defined(HAVE_GETPWUID_R) && defined(_SC_GETPW_R_SIZE_MAX)
 		efree(pwbuf);
 #endif
-		return SG(request_info).current_user;		
+		return SG(request_info).current_user;
 #endif
-	}	
+	}
 }
 /* }}} */
 
@@ -1256,7 +1266,7 @@ PHP_FUNCTION(set_time_limit)
 	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l", &new_timeout) == FAILURE) {
 		return;
 	}
-	
+
 	new_timeout_strlen = zend_spprintf(&new_timeout_str, 0, "%ld", new_timeout);
 
 	if (zend_alter_ini_entry_ex("max_execution_time", sizeof("max_execution_time"), new_timeout_str, new_timeout_strlen, PHP_INI_USER, PHP_INI_STAGE_RUNTIME, 0 TSRMLS_CC) == SUCCESS) {
@@ -1890,7 +1900,7 @@ static void core_globals_dtor(php_core_globals *core_globals TSRMLS_DC)
 PHP_MINFO_FUNCTION(php_core) { /* {{{ */
 	php_info_print_table_start();
 	php_info_print_table_row(2, "PHP Version", PHP_VERSION);
-	php_info_print_table_end(); 
+	php_info_print_table_end();
 	DISPLAY_INI_ENTRIES();
 }
 /* }}} */
@@ -2166,7 +2176,7 @@ int php_module_startup(sapi_module_struct *sf, zend_module_entry *additional_mod
 		return FAILURE;
 	}
 
-	/* initialize registry for images to be used in phpinfo() 
+	/* initialize registry for images to be used in phpinfo()
 	   (this uses configuration parameters from php.ini)
 	 */
 	if (php_init_info_logos() == FAILURE) {
@@ -2212,7 +2222,7 @@ int php_module_startup(sapi_module_struct *sf, zend_module_entry *additional_mod
 			EG(current_module) = NULL;
 		}
 	}
-	
+
 	/* disable certain classes and functions as requested by php.ini */
 	php_disable_functions(TSRMLS_C);
 	php_disable_classes(TSRMLS_C);
@@ -2247,38 +2257,38 @@ int php_module_startup(sapi_module_struct *sf, zend_module_entry *additional_mod
 			const char *directives[16]; /* Remember to change this if the number of directives change */
 		} directives[2] = {
 			{
-				E_DEPRECATED, 
-				"Directive '%s' is deprecated in PHP 5.3 and greater", 
+				E_DEPRECATED,
+				"Directive '%s' is deprecated in PHP 5.3 and greater",
 				{
 					NULL
 				}
-			}, 
+			},
 			{
-				E_CORE_ERROR, 
-				"Directive '%s' is no longer available in PHP", 
+				E_CORE_ERROR,
+				"Directive '%s' is no longer available in PHP",
 				{
 					"allow_call_time_pass_reference",
-					"define_syslog_variables", 
-					"highlight.bg", 
-					"magic_quotes_gpc", 
-					"magic_quotes_runtime", 
-					"magic_quotes_sybase", 
-					"register_globals", 
-					"register_long_arrays", 
-					"safe_mode", 
-					"safe_mode_gid", 
-					"safe_mode_include_dir", 
-					"safe_mode_exec_dir", 
-					"safe_mode_allowed_env_vars", 
-					"safe_mode_protected_env_vars", 
-					"zend.ze1_compatibility_mode", 
+					"define_syslog_variables",
+					"highlight.bg",
+					"magic_quotes_gpc",
+					"magic_quotes_runtime",
+					"magic_quotes_sybase",
+					"register_globals",
+					"register_long_arrays",
+					"safe_mode",
+					"safe_mode_gid",
+					"safe_mode_include_dir",
+					"safe_mode_exec_dir",
+					"safe_mode_allowed_env_vars",
+					"safe_mode_protected_env_vars",
+					"zend.ze1_compatibility_mode",
 					NULL
 				}
 			}
 		};
 
 		unsigned int i;
-		
+
 		zend_try {
 			/* 2 = Count of deprecation structs */
 			for (i = 0; i < 2; i++) {
@@ -2298,7 +2308,7 @@ int php_module_startup(sapi_module_struct *sf, zend_module_entry *additional_mod
 			retval = FAILURE;
 		} zend_end_try();
 	}
-	
+
 	sapi_deactivate(TSRMLS_C);
 	module_startup = 0;
 
@@ -2353,7 +2363,7 @@ void php_module_shutdown(TSRMLS_D)
 	sapi_flush(TSRMLS_C);
 
 	zend_shutdown(TSRMLS_C);
-	
+
 	/* Destroys filter & transport registries too */
 	php_shutdown_stream_wrappers(module_number TSRMLS_CC);
 
@@ -2396,7 +2406,7 @@ PHPAPI int php_execute_script(zend_file_handle *primary_file TSRMLS_DC)
 {
 	zend_file_handle *prepend_file_p, *append_file_p;
 	zend_file_handle prepend_file = {0}, append_file = {0};
-#if HAVE_BROKEN_GETCWD 
+#if HAVE_BROKEN_GETCWD
 	int old_cwd_fd = -1;
 #else
 	char *old_cwd;

sapi/cli/tests/bug43177.phpt

@@ -0,0 +1,82 @@
+--TEST--
+Bug #61977 Test exit code for various errors
+--SKIPIF--
+<?php
+include "skipif.inc"; 
+?>
+--FILE--
+<?php
+include "php_cli_server.inc";
+php_cli_server_start(<<<'SCRIPT'
+	ini_set('display_errors', 0);
+	switch($_SERVER["REQUEST_URI"]) {
+	        case "/parse":
+	                eval("this is a parse error");
+					echo "OK\n";
+	                break;
+	        case "/fatal":
+	                eval("foo();");
+					echo "OK\n";
+	                break;
+			case "/compile":
+					eval("class foo { final private final function bar() {} }");
+					echo "OK\n";
+					break;
+			case "/fatal2":
+					foo();
+					echo "OK\n";
+					break;
+	        default:
+	                return false;
+	}
+SCRIPT
+);
+
+list($host, $port) = explode(':', PHP_CLI_SERVER_ADDRESS);
+$port = intval($port)?:80;
+
+foreach(array("parse", "fatal", "fatal2", "compile") as $url) {
+	$fp = fsockopen($host, $port, $errno, $errstr, 0.5);
+	if (!$fp) {
+  		die("connect failed");
+	}
+
+	if(fwrite($fp, <<<HEADER
+GET /$url HTTP/1.1
+Host: {$host}
+
+
+HEADER
+)) {
+        while (!feof($fp)) {
+                echo fgets($fp);
+        }
+	}
+}
+
+?>
+--EXPECTF--
+HTTP/1.1 200 OK
+Host: localhost
+Connection: close
+X-Powered-By: %s
+Content-type: text/html
+
+OK
+HTTP/1.0 500 Internal Server Error
+Host: localhost
+Connection: close
+X-Powered-By: %s
+Content-type: text/html
+
+HTTP/1.0 500 Internal Server Error
+Host: localhost
+Connection: close
+X-Powered-By: %s
+Content-type: text/html
+
+HTTP/1.0 500 Internal Server Error
+Host: localhost
+Connection: close
+X-Powered-By: %s
+Content-type: text/html