Fixes #12514 - Uses sys_get_temp_dir in place of empty string for temporary file creation

[sudheesh001]

Fix for #12514

[codecov-io]

Current coverage is 54.33% (diff: 100%)

No coverage report found for master at f4d3704.

Powered by Codecov. Last update f4d3704...da60d4f

[nijel]

I think the code should be rather changed to share logic with File class, which does handle various openbasedir restrictions just fine, see https://github.com/phpmyadmin/phpmyadmin/blob/master/libraries/File.php#L465

[sudheesh001]

@nijel So should this be rewritten as follows ?

$tmpfname = null;
if (! empty($GLOBALS['cfg']['TempDir']) && @is_writable($GLOBALS['cfg']['TempDir'])) {
        $tmp_subdir = $GLOBALS['cfg']['TempDir'];
} else {
        $tmp_subdir = ini_get('upload_tmp_dir');
        if (empty($tmp_subdir)) {
            $tmp_subdir = sys_get_temp_dir();
        }
        $tmp_subdir = rtrim($tmp_subdir, DIRECTORY_SEPARATOR);
}
$tmpfname = tempnam($tmp_subdir, $enc);

[nijel]

Rather File class should export this code and this method should should be used by both File class and here in the Kanji encoding code. We don't want similar code copied over to multiple places...

[sudheesh001]

@nijel I've been looking through the code and realized that there's a todo that's put above the checkUploadedFile() function which says
@todo move check of $cfg['TempDir'] into Config?

So should this part be moved into a function inside Config.php as follows ?

public function getDefaultTempDirectory() {
    $tmp_subdir = null;
    if (! empty($GLOBALS['cfg']['TempDir']) && @is_writable($GLOBALS['cfg']['TempDir'])) {
        $tmp_subdir = $GLOBALS['cfg']['TempDir'];
    } else {
        $tmp_subdir = ini_get('upload_tmp_dir');
        if (empty($tmp_subdir)) {
            $tmp_subdir = sys_get_temp_dir();
        }
        $tmp_subdir = rtrim($tmp_subdir, DIRECTORY_SEPARATOR);
    }
    return $tmp_subdir;
}

And then call this by doing $this->getDefaultTempDirectory() in the Encoding.php and File.php functions ?

Is that the right way to do this ?

[nijel]

Yes, @sudheesh001 , that sounds like a good approach.

[nijel]

@sudheesh001 will you get anytime soon to implementing described change?

[nijel]

Closing due to no response, somebody else started to work on #12514 as well.