Make WordPress Core

Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#20449 closed defect (bug) (fixed)

get_home_path() error on windows with different home and site_url values

Reported by: iturgeon's profile iturgeon Owned by: dd32's profile dd32
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.3.1
Component: Permalinks Keywords: has-patch commit
Focuses: Cc:

Description

get_home_path() was always returning a root directory reference '/'

Environment:

IIS 7.5, FCGI, PHP 5.3.10, WP 3.3.1

Walking through

I'll just run through what's happening to the values below:

// for ref, script_filename is
//$_SERVER["SCRIPT_FILENAME"] = 'D:\root\vhosts\site\httpdocs\wp\wp-admin\options-permalink.php'	
	
function get_home_path() {
	$home = get_option( 'home' );
	// $home='http://site.com'
	$siteurl = get_option( 'siteurl' );
	// $siteurl = 'http://site.com/wp'
	if ( $home != '' && $home != $siteurl ) {  
		$wp_path_rel_to_home = str_replace($home, '', $siteurl);
		// $wp_path_rel_to_home = '/wp'
		$pos = strpos($_SERVER["SCRIPT_FILENAME"], $wp_path_rel_to_home);
		// $pos = FALSE
		$home_path = substr($_SERVER["SCRIPT_FILENAME"], 0, $pos);
		// $home_path = ''
		$home_path = trailingslashit( $home_path );
		// $home_path = '/'
	} else {
		$home_path = ABSPATH;
	}
	return $home_path;
	//  returns '/'
}

Suggest adding the following line

  $wp_path_rel_to_home = str_replace('/', DIRECTORY_SEPARATOR, $wp_path_rel_to_home); 


Diff:

--- a/wp/wp-admin/includes/file.php
+++ b/wp/wp-admin/includes/file.php
@@ -81,6 +81,7 @@ function get_home_path() {
 	$siteurl = get_option( 'siteurl' );
 	if ( $home != '' && $home != $siteurl ) {
 		$wp_path_rel_to_home = str_replace($home, '', $siteurl); /* $siteurl - $home */
+		$wp_path_rel_to_home = str_replace('/', DIRECTORY_SEPARATOR, $wp_path_rel_to_home); // replaces url slashs with backslashes when needed
 		$pos = strpos($_SERVER["SCRIPT_FILENAME"], $wp_path_rel_to_home);
 		$home_path = substr($_SERVER["SCRIPT_FILENAME"], 0, $pos);
 		$home_path = trailingslashit( $home_path );
-- 

Attachments (4)

20449.patch (880 bytes) - added by SergeyBiryukov 12 years ago.
20449.2.patch (748 bytes) - added by WraithKenny 11 years ago.
20449-ut.diff (1.3 KB) - added by ryan 11 years ago.
18768-UT.patch (825 bytes) - added by WraithKenny 11 years ago.
adds an assertion for 18768

Download all attachments as: .zip

Change History (18)

#1 @knutsp
12 years ago

  • Cc knut@… added

#2 @SergeyBiryukov
12 years ago

  • Keywords has-patch added

For some reason, $_SERVER['SCRIPT_FILENAME'] contains forward slashes on my Windows install:

S:/home/wordpress/trunk/wp/wp-admin/options-permalink.php

So the bug didn't reproduce as is. ABSPATH contains backslashes though:

S:\home\wordpress\trunk\wp/

The patch also handles #10447.

#3 @dd32
12 years ago

  • Milestone changed from Awaiting Review to 3.5

#4 @dd32
12 years ago

  • Owner set to dd32
  • Resolution set to fixed
  • Status changed from new to closed

In [21224]:

Make get_home_path() work in more cases by being case insensitive and sanitzing Windows paths. In some cases (such as differing case of hostnames or paths in the site/home options, or when SCRIPT_FILENAME contains forward slashes) the function was failing to return the correct path, and would instead return /. Props to SergeyBiryukov for the initial patch. Fixes #20449 Fixes #10447

#5 @WraithKenny
11 years ago

If on D:\root\vhosts\site\httpdocs\wp\wp-admin\options-permalink.php with a subfolder install ($wp_path_rel_to_home = '/wp') the desired result is D:\root\vhosts\site\httpdocs\ but we get D:\root\vhosts\site\httpdocs\wp\

This causes the .htaccess file to be written to the WordPress Address rather then the Site Address.

The strripos hits the wp in wp-admin so in the special case that the subfolder is wp (like Mark's skeleton) this patch doesn't work well.

Should this be reopened, or a new ticket?

#6 @nacin
11 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#7 @WraithKenny
11 years ago

$pos = stripos( ... fixes in case of wp folder install.

#8 @WraithKenny
11 years ago

#18768 is the reason for the strripos instead of stripos. We probably need something more complicated then just the first or last occurrence.

#9 @WraithKenny
11 years ago

20449.2.patch uses trailingslashit on $wp_path_rel_to_home (to search against a directory), which prevents /wp/ from matching /wp.dev or /wp-admin etc.

@ryan
11 years ago

#10 @SergeyBiryukov
11 years ago

Tested 20449.2.patch, looks good to me.

#11 @nacin
11 years ago

  • Keywords commit added

Added ryan's tests in [1146].

The tests fail without, and pass with, 20449.2.patch.

dd32 has been traveling over the last few days, so would like to get him to look at this. But happy to have this committed without that review for now.

@WraithKenny
11 years ago

adds an assertion for 18768

#12 @dd32
11 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 22800:

Correct get_home_path() for cases where WordPress is installed in a subdirectory called /wp/, previously it would match on /wp-admin instead of /wp causing an incorrect return path. Props SergeyBiryukov. Fixes #20449

#13 @dd32
11 years ago

'doh, copied the wrong patch submitter! Sorry WraithKenny!

#14 @WraithKenny
11 years ago

No Prob :-)

Note: See TracTickets for help on using tickets.