Default Temporary Directory On Windows Selected Improperly

[dait]

Blesta 3.0.5 on Windows 2008 R2

 

The default temporary directory on Windows is selected by this function:

 

	private function tmpDir() {
		$dir = "/tmp/";		
		$os = $this->getOs();
		
		if ($os == "WIN")
			$dir = "C:\\Windows\\TEMP\\";
		
		// If PHP_VERSION > 5.2.1, use sys_get_temp_dir() function
		if (function_exists('sys_get_temp_dir'))
			$dir = sys_get_temp_dir() . DIRECTORY_SEPARATOR;
		
		return $dir;
	}

 

This has several major flaws.

 

1) C:\Windows\TEMP might not even exist and there is no check if it does.

2) sys_get_temp_dir is not a suitable pick either for Windows as described in this topic. Sending emails with attachment to Blesta ticket system causes the attachment to have invalid access rights because instead of using temporary directory for temporary files only, in this case it is being used for permanent files - the attachment files. The files are simply moved from the temporary directory into Blesta's upload directory, which does preserve rights of temporary files that are intended for temporary files only and not for permanent files.

 

Blesta's default value of Temp Directory seems right to the user but will not work as expected and it is a hard task to find the cause (unless user Google's out the original topic where I analyzed this).

 

Since http://docs.blesta.com/display/user/System+%3E+General is empty, the user has no information about Temp Directory setting and could not easily figure out that on Windows he should change the default value in order to make things working as expected. This is a problem because using the default settings and by following the official documentation the installation does not work properly. However, even if the user changes the value of Temp Directory and thus fixes the email attachment problem he newly has a Temp Directory setting pointed to a directory that should not be used for temporary files. If Blesta uses or will ever use this directory for temporary files (as its name suggests), the configuration will not be ideal.

 

 

 

I have revealed that Blesta 3.0.5 comes with this code update:

 

	public function getAttachments(MimeMailParser $email, $tmp_dir = null) {
		
		$files = array();
		
		if (!$tmp_dir) {
			$tmp_dir = ini_get("upload_tmp_dir");

			if (!$tmp_dir && function_exists("sys_get_temp_dir"))
				$tmp_dir = sys_get_temp_dir();
		}

 

However, it seems that by default this is always called with non null $tmp_dir argument (from processTicketFromEmail function):

 

		if (!$this->tmp_dir) {
			$tmp_dir = $this->Settings->getSetting("temp_dir");
			if ($tmp_dir)
				$this->tmp_dir = $tmp_dir->value;
		}
		
		...
		
		// Fetch the references to all files uploaded for this ticket
		$files = $this->EmailParser->getAttachments($email, $this->tmp_dir);

 

Not sure if interpreted correctly, but it seems to be that there is no how for the $tmp_dir to be empty unless a user intentionally sets the value to an empty string in the settings. Such a move is undocumented and it is unknown whether it could not cause problems in different part of Blesta.
 

 

QUESTION: In getAttachments code, in which scenario gets this line executed:

 

$tmp_dir = ini_get("upload_tmp_dir");

 

In our environment it does not get executed and by reading the code I could not see any suitable option. Did I miss something?

 

 

There are two possible solutions to this:

 

1) The ideal solution is that Blesta would introduce Upload Temp Directory setting - an analogy of PHP's ini_get("upload_tmp_dir") and set its default value to ini_get("upload_tmp_dir") and use this setting instead of Temp Directory setting. This would fix all the problems and it would be consistent with common practices of working with temporary directories.

 

2) The Temp Directory would be perfectly documented, informing Windows users about this problem.

 

QUICKFIX: Change line 152 in plugins\support_manager\components\ticket_manager\ticket_manager.php from this

		$files = $this->EmailParser->getAttachments($email, $this->tmp_dir);

 

to this

		$files = $this->EmailParser->getAttachments($email);

This will force Blesta to execute this line

 

$tmp_dir = ini_get("upload_tmp_dir");

in the getAttachments code and everything will work as expected if your PHP installation has upload_tmp_dir correctly set.

 

 

Now, I can not stress this out enough: PLEASE DO NOT ARGUMENT THAT THIS IS NOT A BLESTA BUG BECAUSE IT IS YOUR DECISION ON HOW YOUR DEFAULT TEMP FOLDER IN YOUR SYSTEM IS CONFIGURED.

 

Such an argument is not valid because:

• If you install your Windows system and leave it as is, you will run into these problems.
• If you install your Windows system by the book, using best practices and secure your box perfectly, you will run into these problems.
• No common application (unlike some critical, very specific, kernel or security related applications) should require tweaks and hacks in operating system.
• No common application should require changing a system setting that other applications may rely on. Changing the default rights for Windows temporary folder just because of Blesta is crazy and should not be done.
• An easy-to-implement solution is available and is perfectly consistent with how PHP deals with a similar issue.

 

[Cody]

Install::tmpDir() method was updated in CORE-814 for version 3.1.0.

 

For those that are curious, here's the new method:

 

 

    /**
     * Determine the location of the temp directory on this system
     */
    private function tmpDir() {
        $dir = ini_get("upload_tmp_dir");

        if (!$dir && function_exists("sys_get_temp_dir"))
            $dir = sys_get_temp_dir();
            
        if (!$dir) {
            $dir = "/tmp/";        
            if ($this->getOs() == "WIN")
                $dir = "C:\\Windows\\TEMP\\";
        }
        
        $dir = rtrim($dir, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR;
        
        return $dir;
    }

 

The solution to the issue you are experiencing is to set your temp directory to the value you get from ini_get("upload_tmp_dir").