Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#3009 closed Bug (fixed)

display_errors is always set to On. It should honor the php.ini setting instead

Reported by: gggeek Owned by: vipsoft
Priority: normal Milestone: 1.12.x - Piwik 1.12.x
Component: Core Keywords:
Cc: Sensitive: no

Description

As put in evidence by the "security" plugin, display_errors is always on by default.

The line:

ini_set('display_errors', (!defined('PIWIK_DISPLAY_ERRORS')
PIWIK_DISPLAY_ERRORS) ? 1 : 0);

should be:
ini_set('display_errors', (defined('PIWIK_DISPLAY_ERRORS') && PIWIK_DISPLAY_ERRORS) ? 1 : 0);

Change History (8)

comment:1 Changed 2 years ago by vipsoft (robocoder)

  • Milestone set to 1.7.x - Piwik 1.7.2
  • Owner set to vipsoft
  • Status changed from new to assigned
  • Summary changed from Error in setting display_errors to Don't try to set display_errors

It's a bit more complicated than that. In newer php versions, that line actually has no effect (ie can only be set in php.ini). And depending on the php version, it can be a non-boolean value (which PhpSecInfo doesn't recognize).

The fix is to remove the line, update the bootstrap.php docs, and fix PhpSecInfo detection.

comment:2 Changed 2 years ago by vipsoft (robocoder)

From http://ca2.php.net/manual/en/errorfunc.configuration.php#ini.display-errors

Although display_errors may be set at runtime (with ini_set()), it won't have any affect if the script has fatal errors. This is because the desired runtime action does not get executed.

comment:3 Changed 2 years ago by vipsoft (robocoder)

(In [5973]) refs #3009 - don't set display_errors in vain; updated online docs (re: bootstrap.php)

comment:4 Changed 2 years ago by vipsoft (robocoder)

Haha... it's a bug in the php documentation.

I just tested:

<?php ini_set('display_errors', 1); require_once('nonexistent.php');

vs

<?php ini_set('display_errors', 0); require_once('nonexistent.php');

And the latter DID suppress the fatal error.

comment:5 Changed 2 years ago by vipsoft (robocoder)

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [5974]) fixes #3009 - display_errors now defaults to disabled; leaving PhpSecInfo "as is" because ini_set("display_errors", "stderr") is still leaky

comment:6 Changed 2 years ago by vipsoft (robocoder)

  • Summary changed from Don't try to set display_errors to Disable display_errors (default: production use)

comment:7 Changed 2 years ago by gggeek

About the line:

<?php ini_set('display_errors', 0); require_once('nonexistent.php');

What you are probably seeing is a blank page. Which is neither a bug in the php doc nor in the php execution:
. the script is halted with a fatal error - you will find it in the error log
. no error message is printed to screen

More meaningful tests are:

<?php ini_set('display_errors', 0); require_once('nonexistent.php'); echo 'hi';

or

<?php require_once('nonexistent.php'); ini_set('display_errors', 0);

comment:8 Changed 2 years ago by matt (mattab)

  • Summary changed from Disable display_errors (default: production use) to display_errors is always set to On. It should honor the php.ini setting instead
Note: See TracTickets for help on using tickets.