WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#21143 closed task (blessed) (fixed)

Add a hook for when no blog is found (in addition to NOBLOGREDIRECT)

Reported by: jamescollins Owned by: nacin
Milestone: 3.9 Priority: normal
Severity: normal Version:
Component: Bootstrap/Load Keywords: dev-feedback has-patch
Focuses: multisite Cc:

Description

(Related to #21142)

Background

On a multisite install, we allow users to change their subdomain name (or domain name).

We keep a list of their previously used subdomains/domains, and if there is a HTTP request for one of their old domains, we automatically perform a 301 redirect to the new domain name.

Problem

In order to perform a 301 redirect from and old domain to the new domain, we currently have to use wp-content/sunrise.php.

However we would like to take advantage of the logic at the top of wp-includes/ms-settings.php, rather than re-inventing that logic in our sunrise.php file.

The NOBLOGREDIRECT constant can be used for this, but it does not allow us to perform a 301 redirect.

Solution

Adding a simple hook to ms-settings.php would allow a sunrise.php file to determine what should happen if a HTTP request on a Multisite install does not correspond to one of the existing sites/blogs.
See attached patch.

Attachments (7)

21143_no_blog_redirect.diff (536 bytes) - added by jamescollins 7 years ago.
sunrise.php (694 bytes) - added by jamescollins 7 years ago.
Example wp-content/sunrise.php file that uses the new 'no_blog_redirect' hook
21143_blog_not_found.diff (534 bytes) - added by jamescollins 7 years ago.
New hook is now named blog_not_found
21143_2.diff (949 bytes) - added by jamescollins 7 years ago.
21143.diff (2.0 KB) - added by jeremyfelt 6 years ago.
21143.2.diff (1.6 KB) - added by jeremyfelt 6 years ago.
21143.3.diff (2.8 KB) - added by jeremyfelt 6 years ago.

Download all attachments as: .zip

Change History (29)

@jamescollins
7 years ago

Example wp-content/sunrise.php file that uses the new 'no_blog_redirect' hook

#1 @jamescollins
7 years ago

The new 'no_blog_redirect' hook would fire before mu-plugins are initialised, which means that a custom function would have to hook into 'no_blog_redirect' by using sunrise.php.

The attached sunrise.php file is a simple example that demonstrates how this new hook could be used.

The end result is a relatively simple way to add custom logic that only gets executed for requests that don't match an existing blog/site on a Multisite install.

#2 follow-up: @scribu
7 years ago

I don't think 'no_blog_redirect' is an appropriate name. Maybe I want to do something else when a blog is not found.

So, how about 'blog_not_found' instead?

@jamescollins
7 years ago

New hook is now named blog_not_found

#3 in reply to: ↑ 2 @jamescollins
7 years ago

Good suggestion scribu.

New patch adds a 'blog_not_found' hook instead.

#4 @scribu
7 years ago

  • Milestone changed from Awaiting Review to 3.5

#5 @nacin
7 years ago

  • Keywords needs-patch punt added; has-patch removed

The variables in that file are a bit confusing. $domain, $blogname, $current_site, $current_blog, $path

Looks like the most important ones there are $domain and $path, along with $current_site and $blogname (which is either a piece of the domain, or a piece or all of the path). $current_blog is going to be null, and if no blog was found, then passing it doesn't really make any sense.

If we're going to add an action here, we should probably add it to the next block too, which handles non-subdomain can't-find-the-blog situations.

I really want to just rewrite ms-settings.php one of these days. Since this can be handled with sunrise.php (though fairly annoying to do), I'm tempted to punt, unless we can come up a patch that consistently fires this hook for every time we can't find a site. (These are not the only two instances.)

@jamescollins
7 years ago

#6 @jamescollins
7 years ago

  • Keywords has-patch added; needs-patch removed

Thanks for your input Andrew. I agree that the code is in need of a rewrite!

21143_2.diff is another attempt at this.

Rather than duplicate the same do_action() code in multiple places, I've elected for a single call further up in the code.

The parameters for the hook have also been cleaned up, as per your suggestions.

It should now fire for both subdomain and subdirectory installs, and it'll also fire before NOBLOGREDIRECT comes into play.

#7 follow-up: @nacin
7 years ago

  • Milestone changed from 3.5 to Future Release

I want to revisit this in 3.6. I think I might finally rewrite ms-settings.php then, and I don't want an action to make it more difficult to basically start from scratch (because it is terrible). This can still be handled in some form in sunrise.php and through more hacky means, so I don't feel too bad about leaving it as is for now.

#8 @SergeyBiryukov
7 years ago

  • Keywords punt removed

#9 in reply to: ↑ 7 @jeremyfelt
6 years ago

  • Keywords dev-feedback added; has-patch removed
  • Milestone changed from Future Release to 3.7

Replying to nacin:

I want to revisit this in 3.6. I think I might finally rewrite ms-settings.php then, and I don't want an action to make it more difficult to basically start from scratch (because it is terrible). This can still be handled in some form in sunrise.php and through more hacky means, so I don't feel too bad about leaving it as is for now.

And forcing a revisit in 3.7. Would like to hear more about rewriting ms-settings.php, does that need its own ticket?

#10 @nacin
6 years ago

  • Milestone changed from 3.7 to Future Release

A bit late for a rewrite. 3.8 starts soon, can pick it up then.

#11 @Ipstenu
6 years ago

Related to #25802 perhaps?

#12 @jeremyfelt
6 years ago

  • Component changed from Multisite to Bootstrap/Load
  • Focuses multisite added

#13 @rmccue
6 years ago

Bringing forward to 3.9, lest we end up forgetting this again. :)

IMO, we should hit the low-hanging fruit of adding the filter, and rewriting ms-settings.php can happen independently in another ticket.

#14 @jeremyfelt
6 years ago

  • Milestone changed from Future Release to 3.9

Related: #27003. As that goes in, it's a prime time for adding a hook for no site found.

@jeremyfelt
6 years ago

#15 @jeremyfelt
6 years ago

  • Keywords has-patch added

21143.diff adds a new action for site_not_found.

I've moved the defined( 'WP_INSTALLING' ) check up as this seems to be a very independent scenario. The remainder of the $destination capture ends with the hook before redirecting, allowing developers to change the outcome.

#16 follow-up: @nacin
6 years ago

I imagine the hook should receive, in addition to $domain and $path, the $current_site variable, as we would have already declared ms_not_installed() earlier if we couldn't find the network.

The branching here is not entirely obvious; I actually wanted to move all of this code into ms_not_installed() entirely but then I realized its name wasn't a very good fit. Looking at 21143.diff, what about the case when ms_not_installed() is to be called? I'd imagine the hook should be fired then, as well? The situation is where a site cannot be found for $domain and it also doesn't match the found network.

@jeremyfelt
6 years ago

#17 in reply to: ↑ 16 @jeremyfelt
6 years ago

Replying to nacin:

I imagine the hook should receive, in addition to $domain and $path, the $current_site variable, as we would have already declared ms_not_installed() earlier if we couldn't find the network.

...what about the case when ms_not_installed() is to be called?

Good points.

In 21143.2.diff, 2 hooks are added - network_not_found to cover the first ms_not_installed() instance and site_not_found to cover the entire no blog block. $current_site is now passed to site_not_found as well.

I wonder how necessary that signup redirect is. If that was taken out or checked separately, the redirect decision would become cleaner.

@jeremyfelt
6 years ago

#18 @jeremyfelt
6 years ago

Replying to jeremyfelt:

I wonder how necessary that signup redirect is. If that was taken out or checked separately, the redirect decision would become cleaner.

I attempted to clean this up a bit in 21143.3.diff.

  1. The network_not_found action fires before ms_not_installed() if a network cannot be found.
  2. If we find a network but no site and WP_INSTALLING is set, set a blog ID of 1 to continue loading WordPress.
  3. If we're not installing and we haven't found anything, fire site_not_found before...
    1. If it's a subdomain install, redirect to one of NOBLOGREDIRECT if defined, the network's signup URL if registration is set to anything other than none, or the main network domain/path.
    2. If it's a subdirectory install and the requested domain doesn't match the network's domain, redirect to the network's domain and path.
    3. Bail completely with ms_not_installed().

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


6 years ago

#20 @jeremyfelt
6 years ago

21143.3.diff here is somewhat outdated by 27003.22.diff in #27003. Parts can go over though if that one is merged.

#21 @nacin
6 years ago

  • Type changed from enhancement to task (blessed)

Setting this to a task as it goes hand-in-hand with working out the kinks from #27003.

#22 @nacin
6 years ago

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

In 27663:

Introduce a ms_site_not_found filter to replace NOBLOGREDIRECT.

Move some processing down below the point where we bail if there's no site. Add more documentation.

props jeremyfelt.
fixes #21143, see #27003.

Note: See TracTickets for help on using tickets.