•  » Requests
  •  » Php error not displayed by default

#1 2011-01-26 12:18:01

Griou
Guest

Php error not displayed by default

Hi,

I was quite surprised to see that piwigo displays error by default. I think should be disable by default, it'll avoid unexperimented/careless user to have a live piwigo installation that could potentially show error and dangerous informations.

Cheers,

Gaetan

 

#2 2011-01-27 09:19:21

plg
Piwigo Team
Nantes, France, Europe
2002-04-05
13861

Re: Php error not displayed by default

Hi Griou,

Yes, you may be right. I admit it helps a lot when a user encounter an error, he can show us the error.

By  default:

Code:

$conf['show_php_errors'] = E_ALL;

and maybe we should decrease this level.

Offline

 

#3 2011-01-27 10:28:38

rvelices
Former Piwigo Team
2005-12-29
1960

Re: Php error not displayed by default

I would say we put E_ALL for RC versions, but not for final releases ?
Anyway I'm fine with any decision.

Offline

 

#4 2011-01-27 10:53:49

plg
Piwigo Team
Nantes, France, Europe
2002-04-05
13861

Re: Php error not displayed by default

What does my php.ini file says:

; error_reporting
;   Default Value: E_ALL & ~E_NOTICE
;   Development Value: E_ALL | E_STRICT
;   Production Value: E_ALL & ~E_DEPRECATED

Offline

 

#5 2011-01-31 11:47:04

griou
Guest

Re: Php error not displayed by default

Hi guys,

I think the level is not really the problem here , it's mostly the fact that errors are displayed. I understand it makes it easy for you guys to fix some user's problem .

For me errors shouldn't be displayed but they still have to be reported and logged. I don't know exactly how piwigo works, but if one uses the following config :

Code:

$conf['show_php_errors'] = "";

Does it means that error are not reported at all ?

Cheers,

Gaetan

 

#6 2011-02-03 15:07:25

plg
Piwigo Team
Nantes, France, Europe
2002-04-05
13861

Re: Php error not displayed by default

You're right griou.

Currently, Piwigo does:

Code:

if(isset($conf['show_php_errors']) && !empty($conf['show_php_errors']))
{
  @ini_set('error_reporting', $conf['show_php_errors']);
  @ini_set('display_errors', true);
}

For a production website, I think we should do (without the "if" condition):

Code:

@ini_set('error_reporting', $conf['php_error_reporting']);
@ini_set('display_errors', false);
@ini_set('log_errors', true);

that means:

1) replace $conf['show_php_errors'] by $conf['php_error_reporting'] (because "show_php_errors" is confusing)

2) add $conf['php_display_errors']

3) add $conf['php_log_errors'] (errors are logged into Apache error.log)

and have the following code in include/common.inc.php:

Code:

@ini_set('error_reporting', $conf['php_error_reporting']);
@ini_set('display_errors', $conf['php_display_errors']);
@ini_set('log_errors', $conf['php_log_errors']);

and in include/config_default.inc.php:

Code:

$conf['php_error_reporting'] = E_ALL & ~E_DEPRECATED;
$conf['php_display_errors'] = false;
$conf['php_log_errors'] = true;

Offline

 

#7 2011-02-04 10:33:06

rvelices
Former Piwigo Team
2005-12-29
1960

Re: Php error not displayed by default

I'm OK with the code, but I have a doubt about releasing with display_errors=false.
User lambda will not have the reflex nor the knowledge to look into the error log (supposing he knows how to find it). So he will not come here and let us know about the problem.
Also in case something is wrong, he will come here for help and in addition to usual questions (what plugins do you have?, what versions, etc...) we will have to ask for the error log.

Offline

 

#8 2011-02-04 10:34:39

rvelices
Former Piwigo Team
2005-12-29
1960

Re: Php error not displayed by default

Also note that fatal errors will not be displayed anymore either. So we'll end up with many 'blank pages'

Offline

 

#9 2011-02-04 11:11:34

plg
Piwigo Team
Nantes, France, Europe
2002-04-05
13861

Re: Php error not displayed by default

rvelices, in your opinion, would it make sense to have:

Code:

if (is_admin())
{
  @ini_set('display_errors', true);
}
else
{
  @ini_set('display_errors', false);
}

(or written more compact)

Code:

@ini_set('display_errors', is_admin());

Offline

 

#10 2011-02-04 11:30:29

plg
Piwigo Team
Nantes, France, Europe
2002-04-05
13861

Re: Php error not displayed by default

rvelices wrote:

Also note that fatal errors will not be displayed anymore either. So we'll end up with many 'blank pages'

I 100% agree that a blank page is worse displaying an error.

At visitor level, it would be better to have a nice "An error has occured" page (like the "Page not found" we have).

Offline

 
  •  » Requests
  •  » Php error not displayed by default

Board footer

Powered by FluxBB

github twitter newsletter Donate Piwigo.org © 2002-2024 · Contact