Check 'variables_order' php setting for incompatible settings

[Sebastian Mordziol]

Hello,

In some cases, the $currentDB variable may not be set (I could not easily determine why) - I noticed this mostly when trying to export databases, which would throw an error.

As it stands, if the $tdata variable is not set, the $currentDB variable is never initialized. An easy fix is to select a default database from the available list at this location in the script (around line 860 in the compiled script):

sort($databases);

// Select a default database, that way it is initialized in case $tdata is not set
if(!empty($databases)) {
    $currentDB = $databases[0];
}

if(isset($tdata))
{
    foreach($databases as $db_id => $database)
    {
        if($database['path'] === $tdata['path'])
        {
            $currentDB = $database;
            $params->database = $database['path'];
            break;
        }
    }
}

Cheers,

Sebastian.

[phpLiteAdmin]

Thanks for reporting. Normally, the database is now passed in a GET-Parameter called database . Before, it was passed as a Cookie. This change was made in order to make it possible to work with multiple databases in the same browser in different tabs/windows.

‌

When exactly does this happen and which error do you get? Is it somehow reproducible? Can you check whether the database Parameter is in the URL when the error happens?

Just using the default database does not really solve it because it would create the new issue, that PLA may randomly switch between DBs.

[phpLiteAdmin]

The point of code that you mention is making sure a database that was just created is selected. If no database has been created, it is normal that $tdata is not set.

$currentDB is normally set in lines 900-908 (of the one-file phpliteadmin.php 1.9.8.2).

[Sebastian Mordziol]

Hello,

I have a single database in my install. I can reproduce the problem by doing the following:

1. Click on the database name to select it
2. Go to the export tab
3. Leave everything on default settings
4. Click export

The request parameters look like this:

/?database=.%5Cbiographies.sqlite&fulltexts=0&numRows=30&view=export

The export does not show up, and enabling debug mode shows the error message that the $currentDB is not set, in this line:

$db = new Database($currentDB); // $currentDB is not set
echo $db->export_sql($tables, $drop, $structure, $data, $transaction, $comments);

Does that help?

[phpLiteAdmin]

It would also be interesting what the path to your database looks like, i.e. what the value of the database Parameter is in your URLs. Maybe your path contains some special characters that sometimes gets passed along incorrectly resulting in a path that does not point to a managed DB…

Edit: You already posted in between, thanks. I will investigate.

[Sebastian Mordziol]

Found the problem. On our server, there is a security related extension that clears the $GLOBALS array when the script starts. This made the isManagedDB function fail, because it uses the global $databases array to check for valid databases.

Sorry for the confusion, and thanks for the quick help!

Regards,

Sebastian.

[phpLiteAdmin]

Is this some standard security extension? What is the name of this? Maybe other users use it too.

[Sebastian Mordziol]

Hello,

FYI, this is on an intranet server. I checked with our sysadmin, he said it’s not an extension, but a PHP ini setting. Apparently, if you set this in php.ini:

variables_order = ""

Then the superglobal variables do not get populated. I did not know about this myself.

It’s documented in the PHP docs: https://www.php.net/manual/en/ini.core.php#ini.variables-order

We do have strict policies not to use globals, but it’s certainly an edge case.

[phpLiteAdmin]

Thanks for letting me know. From what I understand, I would expect that this setting breaks phpLiteAdmin entirely. But I will check in detail and experiment with it.

Maybe we can check this setting and if it is set like this, display a warning that this configuration may break phpLiteAdmin.

Thanks again for reporting and replying fast.

[Sebastian Mordziol]

Personally, I believe converting phpLiteAdmin to be independent of global variables would be great for the future, if not critical.

Beyond avoiding this issue, a big plus would be that it could then be encapsulated to be integrated as a view into an existing application. Currently, you cannot take it out of its global context.

Cheers,

Sebastian.

[phpLiteAdmin]

Well, it is definitely true that phpLiteAdmin needs a lot of refactoring, which also includes use of global variables.

However, variables_order = "" affectes superglobals , .i.e stuff like $_GET or $_POST. I don’t see how we could avoid using these completely.

Regarding embedding into another app: Yeah, this would of course be nice, but unfortunately, our codebase is far from a state where this seems feasible.

Problem is basically that I currently don’t have time for a big refactoring or rewrite. Thus, what we do is refactoring in small steps.

Greetings,
Christopher

[phpLiteAdmin]

In php 7.2.24 I configured variables_order = "" and this resulted in the default of EGPCS. Then I tried variables_order = " " (with a space), which resulted in no superglobals at all. Then, however, phpLiteAdmin is completely broken (login does not work as $_POST, $_GET etc. is missing).

Anyway, with https://bitbucket.org/phpliteadmin/public/commits/f2971be4c5c4/ I added a check that makes sure variables_order contains everything needed.

‌

‌

[Sebastian Mordziol]

Thanks!