Variable variables and superglobals

Question

I'm transitioning a huge PHP software from PHP4 to PHP5 and among the many (many) problems I'm facing, the biggest one so far seems to be that the previous programmer just banqueted over the register_globals feature, throwing every now and then some variable without specifying the source and shamelessely hiding warnings and notices under the carpet.

I tried to resolve this issue by creating a function that iterates over an array (passed as an argument) and creates global variables via the "variable variables" feature, and then calling it in every page for $_POST, $_GET and $_SESSION. This is the code:

function fix_global_array($array) {
  foreach($array as $key => $value){
    if(!isset($$key)) {
      global $$key;
      $$key = $value;
    }
  }
}

The problem with this function is that the condition isset($$key) is never true, thus the code inside the brackets is always executed and overrides the previous declarations.

Is there any explanation for this behaviour? I read the PHP documentation where it states that

Please note that variable variables cannot be used with PHP's Superglobal arrays within functions or class methods.

but I don't understand if it's related to my problem or not (to tell the truth, I don't understand what it means either, I couldn't find any example).

PS: please, don't bother telling me that using global variables and/or variable variables is bad programming, I know this only too well by myself, but the other option would be modifying about 2.700 files with a thousand lines of code each line per line, and I'm the only programmer here... But if you know a better solution to get rid of all those "undefined variable" warnings, you can make my day.

PPS: and be patient with my english too ^_^

Answer 1

The reason that isset($$key) is never true, in your given code, is because you call global $$key after the condition-check; the variable isn't in scope until it's been registered with global. To fix this, simply move the line to above the if-statement, so you function will look like:

function fix_global_array($array) {
    foreach($array as $key => $value){
        global $$key;
        if(!isset($$key)) {
            $$key = $value;
        }
    }
}

This will work fine when passed an array, even if said array is $_POST or $_GET. The order you pass in the arrays will matter though. If an index/key is defined in $_POST and also in $_GET, and you pass $_POST to the function first - the value from $_GET will not be stored into the variable.

Alternatively, if you want to shy-away from using variable-variables, either for readability issues or simple preference, you can use the $GLOBALS superglobal in the same way:

function fix_global_array($array) {
    foreach($array as $key => $value){
        if(!isset($GLOBALS[$key])) {
            $GLOBALS[$key] = $value;
        }
    }
}

With this method, the variables are still accessible as if they were defined normally. For example:

$data = array('first' => 'one', 'second' => 'two');
fix_global_array($data);
echo $first;    // outputs: one
echo $second;   // outputs: two

This example is suitable for both code-samples above.

Extra-alternatively, you can use PHP's extract() function. It's purpose is to do exactly what you're fix_global_array() method does - and even has a flag to overwrite existing variable values. Example usage:

extract($data);
echo $first; // outputs: one

A warning regarding extract(), which directly applies to this situation, comes from the PHP website:

Do not use extract() on untrusted data, like user input (i.e. $_GET, $_FILES, etc.). If you do, for example if you want to run old code that relies on register_globals temporarily, make sure you use one of the non-overwriting extract_type values such as EXTR_SKIP and be aware that you should extract in the same order that's defined in variables_order within the php.ini.

Answer 2

But if you know a better solution to get rid of all those "undefined variable" warnings, you can make my day.

There is. Fix the problem of not having superglobals used. Now, naturally, I'm not saying you should manually change every flipping variable call by yourself, but I imagine this is something you could automate, maybe. See if you can follow my brainwave.

First, you would have to get a listing of all "undefined variable" notices. This is as simple as registering an error handler, checking for E_NOTICE calls and check if it's an undefined variable call. I have taken the liberty of writing up a little piece of code that does precisely that.

<?php

/**
 * GlobalsLog is a class which can be used to set an error handler which will 
 * check for undefined variables and checks whether they exist in superglobals.
 * 
 * @author Berry Langerak
 */
class GlobalsLog {
    /**
     * Contains an array of all undefined variables which *are* present in one of the superglobals.
     * 
     * @var array 
     */
    protected $globals;

    /**
     * This contains the order in which to test for presence in the superglobals.
     * 
     * @var array 
     */
    protected $order = array( 'SERVER', 'COOKIE', 'POST', 'GET', 'ENV' );

    /**
     * This is where the undefined variables should be stored in, so we can replace them later.
     * 
     * @var string 
     */
    protected $logfile;

    /**
     * Construct the logger. All undefined variables which are present in one of the superglobals will be stored in $logfile.
     * 
     * @param string $logfile 
     */
    public function __construct( $logfile ) {
        $this->logfile = $logfile;

        set_error_handler( array( $this, 'errorHandler' ), E_NOTICE );
    }

    /**
     * The error handler.
     * 
     * @param int $errno
     * @param string $errstr
     * @param string $errfile
     * @param int $errline
     * @return boolean
     */
    public function errorHandler( $errno, $errstr, $errfile, $errline ) {
        $matches = array( );
        if( preg_match( '~^Undefined variable: (.+)$~', $errstr, $matches ) !== 0 ) {
            foreach( $this->order as $superglobal ) {
                if( $this->hasSuperglobal( $superglobal, $matches[1] ) ) {
                    $this->globals[$errfile][] = array( $matches[1], $superglobal, $errline );
                    return true;
                }
            }
        }
    }

    /**
     * Called upon destruction of the object, and writes the undefined variables to the logfile.
     */
    public function __destruct( ) {
        $globals = array_merge( $this->globals, $this->existing( ) );

        file_put_contents( 
            $this->logfile,
            sprintf( "<?php\nreturn %s;\n", var_export( $globals, true ) )
        );
    }

    /**
     * Gets the undefined variables that were previously discovered, if any.
     * 
     * @return array
     */
    protected function existing( ) {
        if( file_exists( $this->logfile ) ) {
            $globals = require $this->logfile;
            return $globals;
        }
        return array( );
    }

    /**
     * Checks to see if the variable $index exists in the superglobal $superglobal.
     * 
     * @param string $superglobal
     * @param string $index
     * @return bool
     */
    protected function hasSuperglobal( $superglobal, $index ) {
        return array_key_exists( $index, $this->getSuperglobal( $superglobal ) );
    }

    /**
     * Returns the value of the superglobal. This has to be done on each undefined variable, because
     * the session superglobal maybe created *after* GlobalsLogger has been created.
     * 
     * @param string $superglobal
     * @return array
     */
    protected function getSuperglobal( $superglobal ) {
        $globals = array(
            'SERVER' => $_SERVER,
            'COOKIE' => $_COOKIE,
            'POST' => $_POST,
            'GET' => $_GET,
            'ENV' => $_ENV
        );
        return isset( $globals[$superglobal] ) ? $globals[$superglobal] : array( );
    }
}

/**
 * Lastly, instantiate the object, and store all undefined variables that exist
 * in one of the superglobals in a file called "undefined.php", in the same 
 * directory as this file.
 */
$globalslog = new GlobalsLog( __DIR__ . '/undefined.php' );

If you were to include this file in each page that is requested (optionally using php_prepend_file), you'll end up with all your undefined variables in 'undefined.php', after clicking through your entire application.

This is rather interesting information, as you now know which undefined variable is located in which file, on which line, and in which superglobal it actually exists. When determining the superglobal, I've kept the order of Environment, Get, Post, Cookie, and Server in mind, for deciding which takes precedence.

For the next part of our neat little trick, we'll have to loop through all the files where a undefined variable notice was found, and try to replace the undefined variable with its superglobal counterpart. This is actually pretty easy as well, and again, I've created a script to do so:

#!/usr/bin/php
<?php
/**
 * A simple script to replace non globals with their globals counterpart.
 */
$script = array_shift( $argv );

$logfile = array_shift( $argv );

$backup = array_shift( $argv ) === '--backup';

if( $logfile === false || !is_file( $logfile ) || !is_readable( $logfile ) ) {
    print "Usage: php $script <logfile> [--backup].\n";
    exit;
}

$globals = require $logfile;

if( !is_array( $globals ) || count( $globals ) === 0 ) {
    print "No superglobals missing found, nothing to do here.\n";
    exit;
}

$replaced = 0;

/**
 * So we have the files where superglobals are missing, but shouldn't be.
 * Loop through the files.
 */
foreach( $globals as $filename => $variables ) {
    if( !is_file( $filename ) || !is_writable( $filename ) ) {
        print "Can't write to file $filename.\n";
        exit;
    }

    foreach( $variables as $variable ) {
        $lines[$variable[2]] = $variable;
    }

    /**
     * We can write to the file. Read it in, line by line,
     * and see if there's anything to do on that line.
     */
    $fp = fopen( $filename, 'rw+' );
    $i = 0;
    $buffer = '';
    while( $line = fgets( $fp, 1000 ) ) {
        ++$i;
        if( array_key_exists( $i, $lines ) ) {
            $search = sprintf( '$%s', $lines[$i][0] );
            $replace = sprintf( "\$_%s['%s']", $lines[$i][1], $lines[$i][0] );
            $line = str_replace( $search, $replace, $line );
            $replaced ++;
        }
        $buffer .= $line;
    }

    if( $backup ) {
        $backupfile = $filename . '.bck';
        file_put_contents( $backupfile, file_get_contents( $filename ) );
    }

    file_put_contents( $filename, $buffer );
}

echo "Executed $replaced replacements.\n";
unlink( $logfile );

Now, there's simply the matter of calling this script. I've tested this, and this is is the file I've tested it with:

<?php

require 'logger.php';

$_GET['foo'] = 'This is a value';
$_POST['foo'] = 'This is a value';

$_GET['bar'] = 'test';

function foo( ) {
    echo $foo;
}

foo( );

echo $bar;

There are two undefined variables ($foo and $bar), both of which exist in one (or more) of the superglobals. After visiting the page in my browser, there were two entries in my logfile undefined.php; namely foo and bar. Then, I ran the command php globalsfix.php undefined.php --backup which gave me the following output:

berry@berry-pc:/www/public/globalfix% php globalsfix.php undefined.php --backup
Executed 2 replacements.

Curious as to what the results were? Well, so was I. Here it is:

<?php

require 'logger.php';

$_GET['foo'] = 'This is a value';
$_POST['foo'] = 'This is a value';

$_GET['bar'] = 'test';

function foo( ) {
    echo $_POST['foo'];
}

foo( );

echo $_GET['bar'];

Hurrah! No more undefined variables, those are being read from the correct superglobals as of now. Big fat disclaimer: create a backup first. Also, this will not solve all of your problems right away. If you have an if( $foo ) statement, the undefined variable will make sure the corresponding block is never executed, meaning it's very likely not all undefined variables will be caught in one go (but it will solve that issue on the second or third go with this script). Nevertheless; it is a good place to start "cleaning up" your code base.

Also, congrats on reading my entire answer. :)