Last modified: 2010-05-15 15:59:44 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T14138, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 12138 - Improvement for function wfTempDir()
Improvement for function wfTempDir()
Status: RESOLVED INVALID
Product: MediaWiki
Classification: Unclassified
Uploading (Other open bugs)
1.11.x
All All
: Normal enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
: patch
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-11-28 09:08 UTC by Maxim
Modified: 2010-05-15 15:59 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
GlobalFunctions.php tmp directory patch (544 bytes, patch)
2007-11-28 09:08 UTC, Maxim
Details
GlobalFunctions.php tmp directory patch (429 bytes, patch)
2007-11-28 09:20 UTC, Maxim
Details
Another proposal to fix this bug (1.75 KB, application/x-httpd-php)
2008-03-16 00:38 UTC, Anders Häggström
Details
Another proposal to fix this bug (2.09 KB, application/x-httpd-php)
2008-03-16 01:20 UTC, Anders Häggström
Details

Description Maxim 2007-11-28 09:08:53 UTC
Created attachment 4387 [details]
GlobalFunctions.php tmp directory patch

In ./includes/GlobalFunctions.php

you have such code:
==========================================================================
/**
 * Tries to get the system directory for temporary files.
 * The TMPDIR, TMP, and TEMP environment variables are checked in sequence,
 * and if none are set /tmp is returned as the generic Unix default.
 *
 * NOTE: When possible, use the tempfile() function to create temporary
 * files to avoid race conditions on file creation, etc.
 *
 * @return string
 */
function wfTempDir() {
        foreach( array( 'TMPDIR', 'TMP', 'TEMP' ) as $var ) {
                $tmp = getenv( $var );
                if( $tmp && file_exists( $tmp ) && is_dir( $tmp ) && is_writable( $tmp ) ) {
                        return $tmp;
                }
        }
        # Hope this is Unix of some kind!
        return '/tmp';
}
==========================================================================

The improvement is fof this part:
==========================================================================
        # Hope this is Unix of some kind!
        return '/tmp';
==========================================================================

As you must know in php.ini we can set option 'upload_tmp_dir'

For example:
==========================================================================
; Temporary directory for HTTP uploaded files (will use system default if not
; specified).
upload_tmp_dir = /usr/local/www/tmp
==========================================================================

So when you will use in 'LocalSettings.php' option like
==========================================================================
$wgDiff3 = "/usr/local/www/wiki/diff3";
of
$wgDiff3 = "/usr/bin/diff3";
==========================================================================

You will get an error:
==========================================================================
Warning: tempnam() [function.tempnam]: open_basedir restriction in effect. File(/tmp) is not within the allowed path(s): (/usr/local/www/) in /usr/local/www/wiki/includes/GlobalFunctions.php on line 1085
==========================================================================

So we have to improve the code this way:
==========================================================================
        # Hope this is Unix of some kind!
        $ini_val = ( @phpversion() >= '4.0.0' ) ? 'ini_get' : 'get_cfg_var';
        $tmp_path = ( !$ini_val('safe_mode') ) ? ini_get('upload_tmp_dir') : '/tmp';
        return $tmp_path;
==========================================================================

In final we will have:
==========================================================================
/**
 * Tries to get the system directory for temporary files.
 * The TMPDIR, TMP, and TEMP environment variables are checked in sequence,
 * and if none are set /tmp is returned as the generic Unix default.
 *
 * NOTE: When possible, use the tempfile() function to create temporary
 * files to avoid race conditions on file creation, etc.
 *
 * @return string
 */
function wfTempDir() {
        foreach( array( 'TMPDIR', 'TMP', 'TEMP' ) as $var ) {
                $tmp = getenv( $var );
                if( $tmp && file_exists( $tmp ) && is_dir( $tmp ) && is_writable( $tmp ) ) {
                        return $tmp;
                }
        }
        # Hope this is Unix of some kind!
        $ini_val = ( @phpversion() >= '4.0.0' ) ? 'ini_get' : 'get_cfg_var';
        $tmp_path = ( !$ini_val('safe_mode') ) ? ini_get('upload_tmp_dir') : '/tmp';
        return $tmp_path;
}
==========================================================================

Tested and working.
Comment 1 Maxim 2007-11-28 09:20:20 UTC
Created attachment 4388 [details]
GlobalFunctions.php tmp directory patch
Comment 2 Anders Häggström 2008-03-15 21:29:21 UTC
Hello,
I also found this bug. I'm using MW-1.12.0rc1 and I use PHP Safe_mode. I think that the above solution can be improved a bit.

1) ini_get() returns the runtime-value of a setting and get_cfg_var() returns the native php.ini-setting.
2) We do not need to check for safe_mode. I think it is better to check for writeabillity in the given folder.
3) phpversion() only returns a value, it is better to use the function version_compare(). And I think there is a lot of other functions in MediaWiki that require PHP5 or higher (ini_get() requires PHP4 and above) so the version-check should be done during installation of MediaWiki not inside this function.
4) In PHP 5.2.1 and above there is a function to find the temp-dir.
5) Doesn't this whole function belong in the installation-process? And during runtime MW should only use the tmp-dir specified in LocalSettings.php ?
6) The @-signs must be there, otherwise PHP gives an open_basedir-warning if we test a directory outside the open_basedir-path.

My proposal is this:

/**
 * Tries to get the system directory for temporary files.
 * The TMPDIR, TMP, and TEMP environment variables are checked in sequence,
 * and if none are set PHP's "upload_temp_dir" is tested.
 * If all fails /tmp is returned as the generic Unix default.
 *
 * NOTE: When possible, use the tempfile() function to create temporary
 * files to avoid race conditions on file creation, etc.
 *
 * @return string
 */
function wfTempDir() {
	if(version_compare(phpversion(), '5.2.1') >= 0) {
		# We are using PHP 5.2.1 or greater and have the privilege to use sys_get_temp_dir().
		# We need to trim the output! The function is not consistent about the trialing slash.
		$tmp = @realpath( sys_get_temp_dir() );
		if( $tmp && @is_dir( $tmp ) && @is_writable( $tmp ) ) {
			return $tmp;
		}
	}

	# We test with php.ini-setting if the above directoy is not writeable
	# or if we use an old PHP version.
	$tmp = @realpath( ini_get( 'upload_tmp_dir' ) );
	if( $tmp && @is_dir( $tmp ) && @is_writable( $tmp ) ) {
		return $tmp;
	}

	# We fallback to the old method if the above directories are not writeable.
	foreach( array( 'TMPDIR', 'TMP', 'TEMP' ) as $var ) {
		$tmp = @realpath( getenv( $var ) );
		if( $tmp && @is_dir( $tmp ) && @is_writable( $tmp ) ) {
			return $tmp;
		}
	}

	# If we fail to find the system's global tempdirectory we try to make our own.
	global $IP;
	$tmp = $IP . '/tmp';
	if( @is_dir( $tmp ) && @is_writable( $tmp ) ) {
		return $tmp;
	}
	# PHP/4.2.0: The mode parameter became optional.
	# That means this function will fail due to wrong parametercount if we use that old PHP-version.
	if (!file_exists($tmp)) {
		@mkdir( $tmp, 0777 );
		if( @is_dir( $tmp ) && @is_writable( $tmp ) ) {
			return $tmp;
		}
	}

	# Hope this is Unix of some kind!
	return '/tmp';
}
Comment 3 Anders Häggström 2008-03-16 00:38:30 UTC
Created attachment 4724 [details]
Another proposal to fix this bug

This version of the function fixes some problems that, in my opinion, the other fix did not take caere of.
Comment 4 Anders Häggström 2008-03-16 01:20:37 UTC
Created attachment 4725 [details]
Another proposal to fix this bug

This improvment does two things extra.

1) If the PHP-version is very old ( <4.2.0 ) it tries to make a TempDir without the mode-parameter. This new directory might not be writable in some installations but it is better to try than just give up (if it is functional to run MW on such old PHP-verson at all?).

2) This version of the function return the boolean "false" if no writable TempDir is found. This might help the caller of this function to handle the error.
Comment 5 Robert Leverington 2008-03-16 10:11:16 UTC
(In reply to comment #4)
> 1) If the PHP-version is very old ( <4.2.0 ) it tries to make a TempDir without
> the mode-parameter. This new directory might not be writable in some
> installations but it is better to try than just give up (if it is functional to
> run MW on such old PHP-verson at all?).

There is no chance whatsoever that MediaWiki will run on any version of PHP less than 5 as it uses a variety of PHP 5 only OOP constructs, therefore this check is redundant and should not be included.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links