4
\$\begingroup\$

I know this is a pretty simple class for loading a config file into an object, and then accessing it's properties. I think config options should be lightweight and a class like this seems to adds a lot of overhead IMO but I really like being able to access the config properties with $config->propertyName. So do you see any room for improvement? Mainly performance improvements?

<?php
// config.class.php
/*
example usage
$config = Config::getInstance(PATH TO FILE, FILE TYPE);
echo $config->ip;
echo $config->db['host'];


example array file
<?php
return array( 
    'db' => array(
        'host' => 'localhost',
        'user' => 'user1',
        'pass' => 'mypassword'),

    'ip' => '123456',

)

*/   

class Config
{
    private static $_instance = null;
    public $options = array();

    /**
     * Retrieves php array file, json file, or ini file and builds array
     * @param $filepath Full path to where the file is located
     * @param $type is the type of file.  can be "ARRAY" "JSON" "INI"
     */ 
    private function __construct($filepath, $type = 'ARRAY')
    {
        switch($type) {
            case 'ARRAY':
                $this->options = include $filepath;
                break;

            case 'INI';
                $this->options = parse_ini_file($filepath, true);
                break;

            case 'JSON':
                $this->options = json_decode(file_get_contents($filepath), true);
                break;  

            //TO-DO add Database option for settings. Table = id, property, value
            case 'DATABASE':
                $this->options = json_decode(file_get_contents($filepath), true);
                break;     
        }
    }

    private function __clone(){}

    public static function getInstance($filepath, $type = 'ARRAY')
    {
        if (!isset(self::$_instance)) {
            self::$_instance = new self($filepath, $type);
        }
        return self::$_instance;
    }

    /**
     * Retrieve value with constants being a higher priority
     * @param $key Array Key to get
     */
    public function __get($key)
    {
        if (isset($this->options[$key])) {
            return $this->options[$key];
        }else{
            trigger_error("Key $val does not exist", E_USER_NOTICE);
        }
    }

    /**
     * Set a new or update a key / value pair
     * @param $key Key to set
     * @param $value Value to set
     */
    public function __set($key, $value)
    {
        $this->options[$key] = $value;
    }

}
?>
\$\endgroup\$
3
\$\begingroup\$

Mainly performance improvements?

There doesn't seem to be much oppurtunity for performance improvements. This class basically consists of (1) loading and parsing the configuration and (2) accessing data. For loading, parse_ini_file and json_decode are builtin methods so unless you write your own parser from scratch, you won't make this any faster. And accessing data is basically a single condition checking and an array lookup, not much to improve there either.

However, here is a different kind of suggestion. You probably want to make sure that $filepath in the constructor is restricted to specific directories, for example your web root or better, a subfolder of that. That also means checking for attempts to escape from the restricted directory, e.g. $filepath does not have components like ../. I can easily imagine an attack where a malicious user gets to place a custom php script in /tmp and then (if you have some other vulnerability in your other code) manages to call new Config('/tmp/attack.php'). Then you have an arbitrary maclicous script executing with the priviliges of your website. Ouch.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.