Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Hi Thomas, On Thu, Jan 29, 2015 at 4:55 PM, Thomas Bley ma...@thomasbley.de wrote: I think you mean: function myErrorHandler( int $errno , string $errstr [, string $errfile [, int $errline [, array $errcontext [, string $extra_errstr ) Yes. We have these empty-call-filled-on-return parameters in preg_match(...$match) but I am not sure if it makes things more complicated than they should be. For me, changing $errstr to $errstr is the simplest thing to do. I agree. I would like to have this feature. Better log is better management. I don't care much about complexity, but simpler is better. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Hi, the idea behind the rfc is not to change error messages, it's only to append useful information to them. Regards Thomas Andrea Faulds wrote on 28.01.2015 17:53: Hi Thomas, On 27 Jan 2015, at 02:45, Thomas Bley ma...@thomasbley.de wrote: Here is the rfc: https://wiki.php.net/rfc/error_handler_callback_parameters_passed_by_reference Thanks to reeze for coding and putting it on the wiki. Regards Thomas This feels rather weird and “hacky” to me. Error handlers really shouldn’t be able to modify messages, it’s bad enough that they can already ignore errors or do other weird things with them. So, I’m -1 on this. Thanks. -- Andrea Faulds http://ajf.me/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Hi Thomas, On 28 Jan 2015, at 17:36, Thomas Bley ma...@thomasbley.de wrote: the idea behind the rfc is not to change error messages, it's only to append useful information to them. That’s still changing the message. Error handlers are global anyway, so this is useless (or dangerous) for libraries. I don’t get it. -- Andrea Faulds http://ajf.me/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Error handlers are global anyway, so this is useless (or dangerous) for libraries. Yes that's true, maybe set_error_handler can be changed so it behaves like register_shutdown_function or spl_autoload_register when multiple callbacks are registered? Regards Thomas Andrea Faulds wrote on 28.01.2015 18:38: Hi Thomas, On 28 Jan 2015, at 17:36, Thomas Bley ma...@thomasbley.de wrote: the idea behind the rfc is not to change error messages, it's only to append useful information to them. That’s still changing the message. Error handlers are global anyway, so this is useless (or dangerous) for libraries. I don’t get it. -- Andrea Faulds http://ajf.me/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Hi Thomas, On Thu, Jan 29, 2015 at 11:51 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: If there are many people concerns to change error message, how about append user error message to original error message when it's changed? How about this signature. function myErrorHandler($errno, $errstr, $errfile, $errline, $extra_errstr) Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Hi Andrea, On Thu, Jan 29, 2015 at 2:38 AM, Andrea Faulds a...@ajf.me wrote: Error handlers are global anyway, so this is useless (or dangerous) for libraries. I suggest library/framework developers *not* to use user defined error handler. It's better to leave error handler for application developers. I strongly suggest application developers to use user defined error handler to catch unexpected problems. If app codes are written not to raise errors under normal operations (which I strongly recommend to do so), errors are sign of system problems. e.g. Subsystem is down, misconfiguration, crackers are attacking system, etc. In case of attack, admins would like to know which account is causing the problem. Error message like Warning: failed to write /etc/passwd is less useful than Warning: failed to write /etc/passwd. User: yohgaki. User defined error handler is for error handling customization, letting user customize message makes sense. Thomas, If there are many people concerns to change error message, how about append user error message to original error message when it's changed? It's a hacky behavior to me, though. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
I think you mean: function myErrorHandler( int $errno , string $errstr [, string $errfile [, int $errline [, array $errcontext [, string $extra_errstr ) We have these empty-call-filled-on-return parameters in preg_match(...$match) but I am not sure if it makes things more complicated than they should be. For me, changing $errstr to $errstr is the simplest thing to do. Regards Thomas Yasuo Ohgaki wrote on 29.01.2015 03:55: Hi Thomas, On Thu, Jan 29, 2015 at 11:51 AM, Yasuo Ohgaki yohg...@ohgaki.net mailto:yohg...@ohgaki.net wrote: If there are many people concerns to change error message, how about append user error message to original error message when it's changed? How about this signature. function myErrorHandler($errno, $errstr, $errfile, $errline, $extra_errstr) Regards, -- Yasuo Ohgaki yohg...@ohgaki.net mailto:yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
User defined error handler is for error handling customization, letting user customize message makes sense. This is exactly our case. Thomas, If there are many people concerns to change error message, how about append user error message to original error message when it's changed? It's a hacky behavior to me, though. I see no difference if the userland developer can extend (or even change) error messages if he can completely disable (using @, return true in the callback, etc.). Of course it is our task to inform about best practices in the php manual and warn about bad things. Regards Thomas Yasuo Ohgaki wrote on 29.01.2015 03:51: Hi Andrea, On Thu, Jan 29, 2015 at 2:38 AM, Andrea Faulds a...@ajf.me wrote: Error handlers are global anyway, so this is useless (or dangerous) for libraries. I suggest library/framework developers *not* to use user defined error handler. It's better to leave error handler for application developers. I strongly suggest application developers to use user defined error handler to catch unexpected problems. If app codes are written not to raise errors under normal operations (which I strongly recommend to do so), errors are sign of system problems. e.g. Subsystem is down, misconfiguration, crackers are attacking system, etc. In case of attack, admins would like to know which account is causing the problem. Error message like Warning: failed to write /etc/passwd is less useful than Warning: failed to write /etc/passwd. User: yohgaki. User defined error handler is for error handling customization, letting user customize message makes sense. Thomas, If there are many people concerns to change error message, how about append user error message to original error message when it's changed? It's a hacky behavior to me, though. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Error handlers are global anyway, so this is useless (or dangerous) for libraries. I'm not sure if there is a library that parses php errors and is not able to handle prefixes or suffixes. I'm using a similar workaround for many years now in code bases with 500k+ lines of code and it was never useless or dangerous. Regards Thomas Thomas Bley wrote on 28.01.2015 18:50: Error handlers are global anyway, so this is useless (or dangerous) for libraries. Yes that's true, maybe set_error_handler can be changed so it behaves like register_shutdown_function or spl_autoload_register when multiple callbacks are registered? Regards Thomas Andrea Faulds wrote on 28.01.2015 18:38: Hi Thomas, On 28 Jan 2015, at 17:36, Thomas Bley ma...@thomasbley.de wrote: the idea behind the rfc is not to change error messages, it's only to append useful information to them. That’s still changing the message. Error handlers are global anyway, so this is useless (or dangerous) for libraries. I don’t get it. -- Andrea Faulds http://ajf.me/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Hi Thomas, On 27 Jan 2015, at 02:45, Thomas Bley ma...@thomasbley.de wrote: Here is the rfc: https://wiki.php.net/rfc/error_handler_callback_parameters_passed_by_reference Thanks to reeze for coding and putting it on the wiki. Regards Thomas This feels rather weird and “hacky” to me. Error handlers really shouldn’t be able to modify messages, it’s bad enough that they can already ignore errors or do other weird things with them. So, I’m -1 on this. Thanks. -- Andrea Faulds http://ajf.me/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Hi all, On Wed, Jan 28, 2015 at 3:09 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: On Tue, Jan 27, 2015 at 9:26 PM, Xinchen Hui larue...@php.net wrote: if you want custom logs, maybe you should use custom logger. It requires to catch all errors, I suppose. If PHP allows it, either would work. I considered a little more. With error handler that catches E_ERROR cannot be complex handler as complex handler may use various resources that may cause all kinds of errors. Therefore, complex custom error handler logger may miss to log important errors. Best practice would be separating app log and system(PHP) error log. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Hi Xinhcen, On Tue, Jan 27, 2015 at 9:26 PM, Xinchen Hui larue...@php.net wrote: if you want custom logs, maybe you should use custom logger. It requires to catch all errors, I suppose. If PHP allows it, either would work. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Hi! I feel it's kindof wrong direction. if you want custom logs, maybe you should use custom logger. Same here, API basing on modifying something that is supposed to be internal engine things feels wrong, even if it does work. Changing error message is borderline, changing things like lines/filenames just feels wrong. Maybe better access to logging system for custom logging would be more efficient instead? -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Request-Uri: /custom/foobar Request-Params: {action:edit} actually, this should already be in access.log(the last 500 error one).. I don't see why you need it in error log Sorry, my example is not precise enough. Normally, $_POST, $_SESSION['username'] and php://input are not part of access.log. So having ajax requests with all kinds of inputs makes things more difficult. Parsing complete access logs can be quite slow and logging php://input for every request might not be possible in most cases. Regards Thomas Xinchen Hui wrote on 28.01.2015 07:15: On Wed, Jan 28, 2015 at 2:11 PM, Thomas Bley ma...@thomasbley.de wrote: There are some workarounds with register_shutdown_function to extend E_ERROR messages, but I think that's quite dirty in a system with many parallel requests. Here is an example: ?php ini_set('error_reporting', E_ALL); ini_set('display_errors', 0); ini_set('log_errors', 1); ini_set('error_log', 'php_error.log'); ini_set('date.timezone', 'Europe/Berlin'); ini_set('memory_limit', '16M'); $_SERVER['HTTP_HOST'] = 'mywebsite.com'; $_SERVER['REQUEST_URI'] = '/custom/foobar'; $_REQUEST = array('action' = 'edit'); function check_for_fatal() { $error = error_get_last(); if ($error['type'] == E_ERROR) { $errstr = ''; if (!empty($_SERVER['REQUEST_URI'])) { $errstr .= ' Request-Uri: '.$_SERVER['REQUEST_URI'].PHP_EOL; } $errstr .= ' Request-Params: '.json_encode($_REQUEST); file_put_contents('php_error.log', $errstr. PHP_EOL, FILE_APPEND); } } register_shutdown_function('check_for_fatal'); $str = str_repeat('##', 2*1024*1024); gives: [28-Jan-2015 07:00:53 Europe/Berlin] PHP Fatal error: Allowed memory size of 16777216 bytes exhausted (tried to allocate 20971521 bytes) in test.php on line 27 Request-Uri: /custom/foobar Request-Params: {action:edit} actually, this should already be in access.log(the last 500 error one).. I don't see why you need it in error log thanks Regards Thomas Xinchen Hui wrote on 27.01.2015 13:26: Hey: On Tue, Jan 27, 2015 at 11:08 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: Hi Thomas, On Tue, Jan 27, 2015 at 11:45 AM, Thomas Bley ma...@thomasbley.de wrote: Here is the rfc: https://wiki.php.net/rfc/error_handler_callback_parameters_passed_by_reference Thanks to reeze for coding and putting it on the wiki. It looks good to me. Future Scope set_error_handler() callback might be able to handle E_ERROR to be able to append additional information to memory_limit exhaustions (or others). I really want to catch E_RROR by user defined error handler. The only reason why this was disabled is abuse of error handler. We may document abuse of error handler with E_ERROR may result in undefined behavior including memory leak/crash and allow it. I feel it's kindof wrong direction. if you want custom logs, maybe you should use custom logger. thanks Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- Xinchen Hui @Laruence http://www.laruence.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
There are some workarounds with register_shutdown_function to extend E_ERROR messages, but I think that's quite dirty in a system with many parallel requests. Here is an example: ?php ini_set('error_reporting', E_ALL); ini_set('display_errors', 0); ini_set('log_errors', 1); ini_set('error_log', 'php_error.log'); ini_set('date.timezone', 'Europe/Berlin'); ini_set('memory_limit', '16M'); $_SERVER['HTTP_HOST'] = 'mywebsite.com'; $_SERVER['REQUEST_URI'] = '/custom/foobar'; $_REQUEST = array('action' = 'edit'); function check_for_fatal() { $error = error_get_last(); if ($error['type'] == E_ERROR) { $errstr = ''; if (!empty($_SERVER['REQUEST_URI'])) { $errstr .= ' Request-Uri: '.$_SERVER['REQUEST_URI'].PHP_EOL; } $errstr .= ' Request-Params: '.json_encode($_REQUEST); file_put_contents('php_error.log', $errstr. PHP_EOL, FILE_APPEND); } } register_shutdown_function('check_for_fatal'); $str = str_repeat('##', 2*1024*1024); gives: [28-Jan-2015 07:00:53 Europe/Berlin] PHP Fatal error: Allowed memory size of 16777216 bytes exhausted (tried to allocate 20971521 bytes) in test.php on line 27 Request-Uri: /custom/foobar Request-Params: {action:edit} Regards Thomas Xinchen Hui wrote on 27.01.2015 13:26: Hey: On Tue, Jan 27, 2015 at 11:08 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: Hi Thomas, On Tue, Jan 27, 2015 at 11:45 AM, Thomas Bley ma...@thomasbley.de wrote: Here is the rfc: https://wiki.php.net/rfc/error_handler_callback_parameters_passed_by_reference Thanks to reeze for coding and putting it on the wiki. It looks good to me. Future Scope set_error_handler() callback might be able to handle E_ERROR to be able to append additional information to memory_limit exhaustions (or others). I really want to catch E_RROR by user defined error handler. The only reason why this was disabled is abuse of error handler. We may document abuse of error handler with E_ERROR may result in undefined behavior including memory leak/crash and allow it. I feel it's kindof wrong direction. if you want custom logs, maybe you should use custom logger. thanks Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- Xinchen Hui @Laruence http://www.laruence.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Hi Xinchen, On Wed, Jan 28, 2015 at 3:15 PM, Xinchen Hui larue...@php.net wrote: actually, this should already be in access.log(the last 500 error one).. I don't see why you need it in error log PHP log would be more useful with user ID/name to identify who was trying to tamper with system, for example. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
On Wed, Jan 28, 2015 at 2:11 PM, Thomas Bley ma...@thomasbley.de wrote: There are some workarounds with register_shutdown_function to extend E_ERROR messages, but I think that's quite dirty in a system with many parallel requests. Here is an example: ?php ini_set('error_reporting', E_ALL); ini_set('display_errors', 0); ini_set('log_errors', 1); ini_set('error_log', 'php_error.log'); ini_set('date.timezone', 'Europe/Berlin'); ini_set('memory_limit', '16M'); $_SERVER['HTTP_HOST'] = 'mywebsite.com'; $_SERVER['REQUEST_URI'] = '/custom/foobar'; $_REQUEST = array('action' = 'edit'); function check_for_fatal() { $error = error_get_last(); if ($error['type'] == E_ERROR) { $errstr = ''; if (!empty($_SERVER['REQUEST_URI'])) { $errstr .= ' Request-Uri: '.$_SERVER['REQUEST_URI'].PHP_EOL; } $errstr .= ' Request-Params: '.json_encode($_REQUEST); file_put_contents('php_error.log', $errstr. PHP_EOL, FILE_APPEND); } } register_shutdown_function('check_for_fatal'); $str = str_repeat('##', 2*1024*1024); gives: [28-Jan-2015 07:00:53 Europe/Berlin] PHP Fatal error: Allowed memory size of 16777216 bytes exhausted (tried to allocate 20971521 bytes) in test.php on line 27 Request-Uri: /custom/foobar Request-Params: {action:edit} actually, this should already be in access.log(the last 500 error one).. I don't see why you need it in error log thanks Regards Thomas Xinchen Hui wrote on 27.01.2015 13:26: Hey: On Tue, Jan 27, 2015 at 11:08 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: Hi Thomas, On Tue, Jan 27, 2015 at 11:45 AM, Thomas Bley ma...@thomasbley.de wrote: Here is the rfc: https://wiki.php.net/rfc/error_handler_callback_parameters_passed_by_reference Thanks to reeze for coding and putting it on the wiki. It looks good to me. Future Scope set_error_handler() callback might be able to handle E_ERROR to be able to append additional information to memory_limit exhaustions (or others). I really want to catch E_RROR by user defined error handler. The only reason why this was disabled is abuse of error handler. We may document abuse of error handler with E_ERROR may result in undefined behavior including memory leak/crash and allow it. I feel it's kindof wrong direction. if you want custom logs, maybe you should use custom logger. thanks Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- Xinchen Hui @Laruence http://www.laruence.com/ -- Xinchen Hui @Laruence http://www.laruence.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Hi, the idea behind the rfc is not to change error messages, it's only to append useful information to them. Changing lines/filenames is surely something controversial and might be only useful when compiling configuration files or templates to php code. Therefore we have 3 options in the voting section. Maybe better access to logging system for custom logging would be more efficient instead? The logging system can be accessed with 1 line of code (trigger_error, throw new exception, etc), so from userland perspective I don't see any deficits here. In case someone needs multiple error handlers, it might be necessary to implement sth. like spl_set_error_handler (similar to spl_autoload_register), but that's not in the scope of this rfc. Regards Thomas Stanislav Malyshev wrote on 27.01.2015 20:43: Hi! I feel it's kindof wrong direction. if you want custom logs, maybe you should use custom logger. Same here, API basing on modifying something that is supposed to be internal engine things feels wrong, even if it does work. Changing error message is borderline, changing things like lines/filenames just feels wrong. Maybe better access to logging system for custom logging would be more efficient instead? -- Stas Malyshev smalys...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Hey: On Tue, Jan 27, 2015 at 11:08 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: Hi Thomas, On Tue, Jan 27, 2015 at 11:45 AM, Thomas Bley ma...@thomasbley.de wrote: Here is the rfc: https://wiki.php.net/rfc/error_handler_callback_parameters_passed_by_reference Thanks to reeze for coding and putting it on the wiki. It looks good to me. Future Scope set_error_handler() callback might be able to handle E_ERROR to be able to append additional information to memory_limit exhaustions (or others). I really want to catch E_RROR by user defined error handler. The only reason why this was disabled is abuse of error handler. We may document abuse of error handler with E_ERROR may result in undefined behavior including memory leak/crash and allow it. I feel it's kindof wrong direction. if you want custom logs, maybe you should use custom logger. thanks Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- Xinchen Hui @Laruence http://www.laruence.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Here is the rfc: https://wiki.php.net/rfc/error_handler_callback_parameters_passed_by_reference Thanks to reeze for coding and putting it on the wiki. Regards Thomas reeze wrote on 23.01.2015 10:59: Yeah, seem other want it it too, so I just updated the PR to allow the first four parameters been passed by reference. On 23 January 2015 at 16:23, Nicolas Grekas nicolas.grekas+...@gmail.com mailto:nicolas.grekas+...@gmail.com wrote: function myErrorHandler($errno, $errstr, $errfile, $errline) {...} It would be awesome to also pass $errfile and $errline by reference. This would allow mapping compiled source to real source code and have meaningful error file+line information. By compiled, I mean e.g. inlined classes (like the bootstrap.cache.php file in Symfony), or preprocessed sources, etc. Nicolas -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV][RFC] Enable error_handler callback parameters to be passed by reference
Hi Thomas, On Tue, Jan 27, 2015 at 11:45 AM, Thomas Bley ma...@thomasbley.de wrote: Here is the rfc: https://wiki.php.net/rfc/error_handler_callback_parameters_passed_by_reference Thanks to reeze for coding and putting it on the wiki. It looks good to me. Future Scope set_error_handler() callback might be able to handle E_ERROR to be able to append additional information to memory_limit exhaustions (or others). I really want to catch E_RROR by user defined error handler. The only reason why this was disabled is abuse of error handler. We may document abuse of error handler with E_ERROR may result in undefined behavior including memory leak/crash and allow it. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net