Make WordPress Core

Opened 4 years ago

Last modified 14 months ago

#52575 new defect (bug)

get_home_path() returns "/" instead of path to WordPress directory

Reported by: pixellogik's profile pixellogik Owned by:
Milestone: Awaiting Review Priority: normal
Severity: major Version: 5.6.1
Component: Filesystem API Keywords: reporter-feedback dev-feedback
Focuses: Cc:

Description

Wrong return value from get_home_path() in /wp-admin/includes/file.php
https://developer.wordpress.org/reference/functions/get_home_path/

Expected result: Absolute filesystem path to the root of the WordPress installation
Result in situation below: /

Context
A WordPress in its own directory installed according method II mentioned here
https://wordpress.org/support/article/giving-wordpress-its-own-directory/

Settings (Example)
Wordpress: https://pixellogik.de/wp
Website: https://pixellogik.de

Plugin WP-SCSS installed

Reproduce
Open https://pixellogik.de in a browser

The plugin calls get_home_path() in enqueue_files() in line 213
/www/htdocs/w012345/pixellogik.de/wp/wp-content/plugins/wp-scss/class/class-wp-scss.php

Due to the unexpected value "/" of get_home_path() the URL of the generated CSS file does not point to a file.
The CSS won't be loaded, the site looks scrambled.

What went wrong?
If SCRIPT_FILENAME is outside the installation directory, the directory can't be found. This case is not handled, hence value "/" is returned

Possible fix:
return ABSPATH in that case

Attachments (1)

52575.diff (996 bytes) - added by peterwilsoncc 15 months ago.

Download all attachments as: .zip

Change History (8)

#1 @costdev
3 years ago

  • Keywords reporter-feedback dev-feedback close added

I just followed Method II here as the ticket mentions. My custom scss is compiled and correctly loaded into the theme.

Notes:

  1. I used pretty permalinks i.e. /%postname%/, which meant copying the .htaccess file from wp/ back to the root directory as stated in the guide linked above. I also had to refresh permalinks in the admin.
  2. Once this is done, I had to go to the WP SCSS settings and click Save Settings, which correctly pointed the plugin to the new location for the files.

There don't seem to be any commits to this function since the ticket was raised. It's possible that one of the steps above or in the guide were missed.

Pending reporter feedback, or dev feedback regarding whether or not there is a sufficient number of edge cases to suggest that get_home_path() should handle when SCRIPT_FILENAME is outside the installation directory, I think this ticket is ready to close.

#2 @lllor
2 years ago

I confirm the issue.

In short, when accessing the website using the index.php in the root folder (it happens only when visiting the homepage) the

$_SERVER['SCRIPT_FILENAME']

is loaded with the root folderpath and the subsequent string processing performed by get_home_page is not executed correctly. As a workaround, the .htaccess should redirect even the / url to the subdirectory/index.php

#3 @arnolp
15 months ago

  • Keywords close removed

On my quite standard installation (wp in subdirectory,nginx) : this bug occurs on every page.

$home : https://<mysite.com>
$siteurl : https://<mysite.com>/wp
$wp_path_rel_to_home : /wp
$_SERVER['SCRIPT_FILENAME'] : /www/<myusername>/public/index.php


:
so on line 113,

 $pos = strripos( str_replace( '\\', '/', $_SERVER['SCRIPT_FILENAME'] ), trailingslashit( $wp_path_rel_to_home ) );

will always return 0, relative path is NOT in server script filename

--> home_path will always return "/"

--> generating tons of open_basedir restriction in effect, as wordpress tries to read the root of the server on includes/misc.php on lines 278/279 !

I had to manually patch wp core to fix it , which will break on every update ..

#4 @peterwilsoncc
15 months ago

Reviewing the unit tests for the function, I think get_home_path() is behaving as intended for WordPress running in a sub-directory, that is a configuration such as:

  • home page: example.com/
  • WP install: example.com/wp

The correct method for getting the path to the WP install is to use the ABSPATH constant, get_home_path() looks intended to match the directory for the URL returned by home_url().

It looks like a documentation issue, the function's current docs are certainly ambiguous as to whether it returns the path for the home page or WP installation.

In 52575.diff I've rewritten the documentation in an effort to make it clearer that the returned path matches the path to home_url() rather than to site_url(). Suggestions for edits are most welcome.

#5 @arnolp
14 months ago

@peterwilsoncc;
Sorry to say I think you've overlooked the issue :
The linked junit tests only test the page "<server path>/wp/wp-admin/options-permalink.php", where it works fine.

However on front-end pages, $_SERVERSCRIPT_FILENAME? will be <server path>/index.php , so will never contains relative path to wp install (/wp in your example), so get_home_path will always be "/",
which triggers a lot of warning by incorrectly reading the root of server

Here is the get_home_path wpcore php code:

<?php
function get_home_path() {
        $home    = set_url_scheme( get_option( 'home' ), 'http' ); // ---> example.com
        $siteurl = set_url_scheme( get_option( 'siteurl' ), 'http' ); // ---> example.com/wp

        if ( ! empty( $home ) && 0 !== strcasecmp( $home, $siteurl ) ) { // ---> true, always, when home and siteurl are different

                $wp_path_rel_to_home = str_ireplace( $home, '', $siteurl ); // ---> '/wp'
                $pos                 = strripos( str_replace( '\\', '/', $_SERVER['SCRIPT_FILENAME'] ), trailingslashit( $wp_path_rel_to_home ) ); 
                //-- >will always return 0, relative path '/wp' is NOT in server script filename '/www/<myusername>/public/index.php'


                $home_path           = substr( $_SERVER['SCRIPT_FILENAME'], 0, $pos ); // ---> '/'
                $home_path           = trailingslashit( $home_path ); // ---> '/'
        } else {
                $home_path = ABSPATH;
        }

        return str_replace( '\\', '/', $home_path );
}

I don't get why this code would ever work apart from admin pages context;

#6 follow-up: @peterwilsoncc
14 months ago

@arnolp Gotcha, thanks for your patience.

I think we're all in agreement on the ticket that it's returning the correct value within the admin for WP in it's own directory:

Home page: example.com
WP Install: example.com/wp
Path to WP: /path/to/site/wp
Expected get_home_path() result: /path/to/site

I don't get why this code would ever work apart from admin pages context;

As the function is located within wp-admin/includes/file.php, I think it was only ever designed to work within the admin. To modify it to work on the front-end as well would be considered an enhancement.

That may be possible using the magic constant __DIR__ if there is the desire to make it a front-end friendly function.

#7 in reply to: ↑ 6 @arnolp
14 months ago

@peterwilsoncc thank you very much for your explanation, indeed.

I was confused by a (bad) plugin, which was flushing rules on front end pages.
Thus, calling :

WP_Rewrite->flush_rules(true)
--> /public/wp/wp-includes/class-wp-rewrite.php(1875): save_mod_rewrite_rules()
--> /public/wp/wp-admin/includes/misc.php(278): file_exists('/.htaccess') <--- uses get_home_path() from front-end

(Well I could argue, if I was really in bad faith, that class-wp-rewrite is not in /wp-admin, thus "allowing" front-end use, but I really can't see a usecase of flushing rewrite rules on front end pages , seems to be a bad idea !)

--> All good on my side. Not sure about the op's usecase, but that's a different one.

Replying to peterwilsoncc:

@arnolp Gotcha, thanks for your patience.

I think we're all in agreement on the ticket that it's returning the correct value within the admin for WP in it's own directory:

Home page: example.com
WP Install: example.com/wp
Path to WP: /path/to/site/wp
Expected get_home_path() result: /path/to/site

I don't get why this code would ever work apart from admin pages context;

As the function is located within wp-admin/includes/file.php, I think it was only ever designed to work within the admin. To modify it to work on the front-end as well would be considered an enhancement.

That may be possible using the magic constant __DIR__ if there is the desire to make it a front-end friendly function.

Note: See TracTickets for help on using tickets.