Opened 4 years ago

Closed 4 years ago

#1415 closed Bug (fixed)

Handle misconfigured config directory and file permissions

Reported by: fmaz Owned by:
Priority: normal Milestone: Piwik 0.6.3
Component: Core Keywords:
Cc: Sensitive: no

Description

Hi,

this time I have a solution. So, in the Piwik_Config::init() method, the is_readable($this->pathIniFileUserConfig) is made after the attempt to read the config file. I think the code should be swapped to look like this:

if(!is_readable($this->pathIniFileUserConfig))
		{
			die("The configuration file {$this->pathIniFileUserConfig} has not been found.");
		}
		$this->defaultConfig = new Piwik_Config_Ini($this->pathIniFileDefaultConfig, null, true);
		

Also note that I've placed a die instead of throwing an exception. The exception was silent, and no error where displayed about the config not found.

And then, many critical error were occuring because the config was missing. So a die() statement looked like useful at this place as we don't want the excecution to continue.

Change History (7)

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

  • Milestone set to 0 - Piwik 0.6.3

Sorry, what problem is this a solution for?

The exception is used to launch the installer. (See core/FrontController.php)

comment:2 Changed 4 years ago by fmaz

the is_readable() is made after the attempt to read the config file.

... the file is read before knowing if you can read it. In my case, you couldn't (file doesn't exist), so an uncatched exception was thrown.

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

Can you post a stacktrace for this code path?

Normally, this is caught by core/FrontController, which then launches the installer.

comment:4 Changed 4 years ago by fmaz

No, I cannot really do it, as I've fixed the code on my installation & I'm configured & I've desactivated XDebug as the site is now in production.

But just try to remove your config file, you should reproduce the bug.

And even without reproducing, just look at the "logic" behind the code:
Read a file, and then check if it's readable... what's the point ?

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

I see.

The patch is wrong, but I'm testing a fix now. Thanks.

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

  • Summary changed from config.ini not found silent exception to Handle misconfigured config directory and file permissions

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

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

(In [2290]) fixes #1415 - prettier error messages for misconfigurations

tested all possible combinations:

  • config/ directory not readable
  • config/global.ini.php not readable (but exists)
  • config/global.ini.php doesn't exist
  • config/global.ini.php empty
  • config/global.ini.php exists and:
    • config/config.ini.php not readable (but exists)
    • config/config.ini.php doesn't exist; triggers installer
    • config/config.ini.php empty
  • config/global.ini.php and config/config.ini.php both exist; normal
Note: See TracTickets for help on using tickets.