WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 6 months ago

#26969 reopened defect (bug)

Incorrect WP path set when creating a new site with wp in a subfolder

Reported by: Denis-de-Bernardy Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Upgrade/Install Keywords: needs-patch
Focuses: multisite Cc:
PR Number:

Description

If you create a site at / with wp located in /wp/, the installer creates the site with example.com/wp as the home url instead of the expected example.com.

Attachments (3)

26969.diff (1.0 KB) - added by Denis-de-Bernardy 6 years ago.
26969.2.diff (1.4 KB) - added by Denis-de-Bernardy 6 years ago.
Also fix the url for broken sites.
26969.3.diff (1.4 KB) - added by Denis-de-Bernardy 6 years ago.
*slaps head*

Download all attachments as: .zip

Change History (18)

#1 follow-up: @nacin
6 years ago

Hey Denis. Gonna need more info than that. :-) How are you creating such a site, given that home and siteurl are not configured until after the fact? If you are placing WordPress in /wp/ and an index.php file at the root including /wp/wp-blog-header.php, I'm not sure that'd be particularly fun for WordPress to pick up on. If you're gonna do an installation like this (which is quite a bit of manual work), it seems like changing the home URL after the fact is perfectly reasonable.

(Ideally, we eliminate home/siteurl from the UI. In 3.5, we almost had a multisite-specific constant we could have repurposed for this, but we ended up going a different direction. History is in #19796.)

#2 in reply to: ↑ 1 @Denis-de-Bernardy
6 years ago

Replying to nacin:

Hey Denis. Gonna need more info than that. :-) How are you creating such a site, given that home and siteurl are not configured until after the fact? If you are placing WordPress in /wp/ and an index.php file at the root including /wp/wp-blog-header.php, I'm not sure that'd be particularly fun for WordPress to pick up on.

The attached patch checks for wp-config.php in ABSPATH, and falls back to trying the same in the parent folder *and* an index.php file. If it finds both, it assumes WP resides in a subfolder and populates home_url accordingly.

I can make it actually read the file if you want, so as to make it check for the "/wp-blog-header.php" string, but I think it's a bit overkill.

If you're gonna do an installation like this (which is quite a bit of manual work), it seems like changing the home URL after the fact is perfectly reasonable.

It is; especially now with the new wp_install hook. But much like the MS installer problem I mentioned, it would be nicer to get it right to begin with. If you're not the slightest bit interested, just close the ticket, and I'll use existing hooks and a wp-content/install.php file to sort out all of these issues.

Btw, I replied to the url_to_postid ticket you pointed to — you can safely commit that patch.

@Denis-de-Bernardy
6 years ago

Also fix the url for broken sites.

@Denis-de-Bernardy
6 years ago

*slaps head*

#3 follow-up: @nacin
6 years ago

I don't think this is a safe enough assumption:

If no wp-config.php file exists in ABSPATH, but does in the parent folder
alongside an index.php file, assume WP is installed in that folder

It's entirely possible you are running a PHP site with an index.php front-end controller, and that the wp-config.php file is just one level up for /blog/ (which is otherwise a full copy of WordPress). It's also possible you have two WordPress installs — / and /blog/, possibly even with a shared wp-config.php file.

wp-load.php does check that one level up wp-settings.php does not also exist. This could help. But I feel this has the potential to cause more problems than it could solve. Short of parsing wp-blog-header.php, which sounds not very fun, I'm not really sure what we should do here. You cite the new wp_install hook — seems like it would make more sense for this check to remain plugin territory.

This ticket was mentioned in IRC in #wordpress-dev by ddebernardy. View the logs.


6 years ago

#5 in reply to: ↑ 3 ; follow-up: @Denis-de-Bernardy
6 years ago

Replying to nacin:

It's entirely possible you are running a PHP site with an index.php front-end controller, and that the wp-config.php file is just one level up for /blog/ (which is otherwise a full copy of WordPress). It's also possible you have two WordPress installs — / and /blog/, possibly even with a shared wp-config.php file.

In which cases WP currently fails anyway.

wp-load.php does check that one level up wp-settings.php does not also exist. This could help. But I feel this has the potential to cause more problems than it could solve. Short of parsing wp-blog-header.php, which sounds not very fun, I'm not really sure what we should do here. You cite the new wp_install hook — seems like it would make more sense for this check to remain plugin territory.

how about using debug_backtrace() to check where WP was included from exactly?

#6 in reply to: ↑ 5 @Denis-de-Bernardy
6 years ago

  • Status changed from new to closed

Replying to Denis-de-Bernardy:

Replying to nacin:

wp-load.php does check that one level up wp-settings.php does not also exist. This could help. But I feel this has the potential to cause more problems than it could solve. Short of parsing wp-blog-header.php, which sounds not very fun, I'm not really sure what we should do here. You cite the new wp_install hook — seems like it would make more sense for this check to remain plugin territory.

how about using debug_backtrace() to check where WP was included from exactly?

Better yet, what about checking the referral that sends the user to the install url?

#7 follow-up: @nacin
6 years ago

  • Status changed from closed to reopened

get_included_files() would probably be more efficient/sane than a backtrace. If we wanted to go that route.

#8 in reply to: ↑ 7 @Denis-de-Bernardy
6 years ago

Replying to nacin:

get_included_files() would probably be more efficient/sane than a backtrace. If we wanted to go that route.

The following, from my bug fixes plugin, actually *tries* the subfolder:

    /**
     * Sets the home url when WP is installed in its own folder
     **/
    protected function fixHomeUrl()
    {
        $home_url = get_option('home');
        $site_url = get_option('siteurl');
        $home_dir = basename($home_url);
        $site_dir = basename($site_url);
        $phys_dir = basename(ABSPATH);
        
        # Bail if $phys_dir is not in any url
        if ($home_dir != $phys_dir || $site_dir != $phys_dir) return;
        
        # Bail if the parent url isn't loading this WP install
        $maybe_home_url = dirname($home_url);
        $response = wp_remote_head($maybe_home_url, array('timeout' => 5, 'httpversion' => '1.1'));
        if (is_wp_error($response) || 200 != wp_remote_retrieve_response_code($response)) return;
        $pingback_url = wp_remote_retrieve_header($response, 'x-pingback');
        if (!$pingback_url || $pingback_url != get_bloginfo('pingback_url')) return;
        
        # Override the home url found by WP
        update_option('home', $maybe_home_url);
    }

#9 follow-up: @SergeyBiryukov
6 years ago

Related: #24480

#10 in reply to: ↑ 9 @Denis-de-Bernardy
6 years ago

Replying to SergeyBiryukov:

Related: #24480

I *think* that was a different issue. But my WP is somewhat rusty, so take it with a grain of salt.

#11 @SergeyBiryukov
6 years ago

Both tickets apparently have to do with wp_guess_url() and working in a subdirectory, so I've just added the link for additional context.

#12 follow-up: @jeremyfelt
6 years ago

  • Focuses multisite added
  • Milestone changed from Awaiting Review to Future Release
  • Priority changed from normal to low
  • Severity changed from normal to minor
  • Version changed from trunk to 3.0

If we can find a clean way to do this, I'd like it. Should be handled around any other Multisite installation stuff.

#13 in reply to: ↑ 12 ; follow-up: @Denis-de-Bernardy
6 years ago

Replying to jeremyfelt:

If we can find a clean way to do this, I'd like it. Should be handled around any other Multisite installation stuff.

What's not clean about what I suggested in comment:8 further up?

#14 in reply to: ↑ 13 @jeremyfelt
6 years ago

Replying to Denis-de-Bernardy:

Replying to jeremyfelt:

If we can find a clean way to do this, I'd like it. Should be handled around any other Multisite installation stuff.

What's not clean about what I suggested in comment:8 further up?

Not sure, I haven't tested anything yet. Just gardening. :)

The self correction via wp_remote_head() does stick out as something out of the ordinary. It would (I think) be our first use of it in core.

#15 @chriscct7
4 years ago

  • Keywords needs-patch added
  • Priority changed from low to normal
  • Severity changed from minor to normal
Note: See TracTickets for help on using tickets.