WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#20449 closed defect (bug) (fixed)

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

Reported by: iturgeon Owned by: 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 6 years ago.
20449.2.patch (748 bytes) - added by WraithKenny 5 years ago.
20449-ut.diff (1.3 KB) - added by ryan 5 years ago.
18768-UT.patch (825 bytes) - added by WraithKenny 5 years ago.
adds an assertion for 18768

Download all attachments as: .zip

Change History (18)

#1 @knutsp
6 years ago

  • Cc knut@… added

#2 @SergeyBiryukov
6 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
5 years ago

  • Milestone changed from Awaiting Review to 3.5

#4 @dd32
5 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
5 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
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#7 @WraithKenny
5 years ago

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

#8 @WraithKenny
5 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.

@WraithKenny
5 years ago

#9 @WraithKenny
5 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
5 years ago

#10 @SergeyBiryukov
5 years ago

Tested 20449.2.patch, looks good to me.

#11 @nacin
5 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
5 years ago

adds an assertion for 18768

#12 @dd32
5 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
5 years ago

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

#14 @WraithKenny
5 years ago

No Prob :-)

Note: See TracTickets for help on using tickets.