r65060 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r65059‎ | r65060 | r65061 >
Date:13:01, 15 April 2010
Author:brion
Status:ok (Comments)
Tags:
Comment:
new-installer branch: don't freak out and abort at 'unwritable' session save dir when it's left blank; in this case it defaults to some system temp dir which we aren't sure of...
Modified paths:
  • /branches/new-installer/phase3/includes/installer/WebInstaller.php (modified) (history)

Diff [purge]

Index: branches/new-installer/phase3/includes/installer/WebInstaller.php
——@@ -205,9 +205,14 @@
206206 */
207207 function startSession() {
208208 $sessPath = $this->getSessionSavePath();
209 - if( !is_dir( $sessPath ) || !is_writeable( $sessPath ) ) {
210 - $this->showError( 'config-session-path-bad', $sessPath );
211 - return false;
 209+ if( $sessPath != '' ) {
 210+ if( !is_dir( $sessPath ) || !is_writeable( $sessPath ) ) {
 211+ $this->showError( 'config-session-path-bad', $sessPath );
 212+ return false;
 213+ }
 214+ } else {
 215+ // If the path is unset it'll default to some system bit, which *probably* is ok...
 216+ // not sure how to actually get what will be used.
212217 }
213218 if( wfIniGetBool( 'session.auto_start' ) || session_id() ) {
214219 // Done already

Comments

#Comment by Tim Starling (talk | contribs)   01:57, 30 June 2010 
// If the path is unset it'll default to some system bit, which *probably* is ok...
// not sure how to actually get what will be used.

Well, the relevant code is in mod_files.c: 

	if (*save_path == '\0') {
		/* if save path is an empty string, determine the temporary dir */
		save_path = php_get_temporary_directory();

		if (PG(safe_mode) && (!php_checkuid(save_path, NULL, CHECKUID_CHECK_FILE_AND_DIR))) {
			return FAILURE;
		}
		if (php_check_open_basedir(save_path TSRMLS_CC)) {
			return FAILURE;
		}
	}

It probably makes sense to simulate those two failure cases. The PHP function sys_get_temp_dir() is a wrapper around php_get_temporary_directory(). It only exists since PHP 5.2.1, but it's probably the thing to use, it just needs a function_exists() check.

Status & tagging log