-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Zend: Support constant expressions in error_reporting ini/env values #19943
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.
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
base: master
Are you sure you want to change the base?
Conversation
Previously, error_reporting could only be set to numeric values via ini or environment variables. This patch extends the OnUpdateErrorReporting handler to parse and evaluate constant expressions (e.g., 'E_ERROR | E_CORE_ERROR') using the AST machinery, matching PHP's runtime behavior. Invalid expressions fallback to E_ALL or 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please also add some failure test cases - if parsing fails, if parsing succeeds but the statements are empty, if the wrong type is found, etc.
echo "error_reporting: ", error_reporting(), "\n"; | ||
echo "E_ERROR | E_CORE_ERROR: ", (E_ERROR | E_CORE_ERROR), "\n"; | ||
--EXPECTF-- | ||
error_reporting: %d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should also test that those two values are the same, I suspect this test would pass on the current master too since it just requires that the results be numbers
@@ -0,0 +1,13 @@ | |||
--TEST-- | |||
error_reporting via INI: Konstanten-Ausdruck (E_ERROR | E_CORE_ERROR) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use English in the test names
zend_string *expr_code = zend_strpprintf(0, "%s;", ZSTR_VAL(new_value)); | ||
zend_ast *ast = NULL; | ||
zend_arena *ast_arena = NULL; | ||
int success = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variable appears unused
int success = 0; |
|
||
zval result; | ||
zend_string *expr_code = zend_strpprintf(0, "%s;", ZSTR_VAL(new_value)); | ||
zend_ast *ast = NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest declaring this variable as part of its definition lower down
zend_ast *ast = zend_compile_string_to_ast(expr_code, &ast_arena, ZSTR_EMPTY_ALLOC());
I don't think this is the right approach. If this feature is to be added, it should be solved more generally for all ini settings. I also don't think it's a good idea to use AST eval. This would enable a much wider range of syntax that isn't otherwise allowed in ini settings. |
|
Previously, error_reporting could only be set to numeric values via ini or environment variables. This patch extends the OnUpdateErrorReporting handler to parse and evaluate constant expressions (e.g., 'E_ERROR | E_CORE_ERROR') using the AST machinery, matching PHP's runtime behavior. Invalid expressions fallback to E_ALL or 0.
Issue: #19938