Make WordPress Core

Opened 9 years ago

Last modified 5 years ago

#30233 new enhancement

Replace or rewrite domain_exists() for more accurate usage

Reported by: jeremyfelt's profile jeremyfelt Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Networks and Sites Keywords: dev-feedback
Focuses: multisite Cc:

Description

domain_exists() was added in MU:543 in almost its current form. The enforcement of trailing slashes on paths was added in #20589 and a filter was added in #21442.

A few notes:

  • The lookup for a domain and path combination is restricted to one network. This allows the same domain and path combination to be used on multiple networks, which should not be default behavior.
  • The name, domain_exists(), implies a check for domain. It is really checking for a full site URL.
  • While it is entirely possible to ignore the result by providing your own in the filter, it would be nice to not always require this for multi-network configurations.

My guess is that the original intent was to ensure a subdomain or path was not present when creating a site on an open network.

In thinking of how to address this, these two possibilities came to mind.

  • Deprecate domain_exists() and wrap a new function that does a larger check. wp_get_site() could work alongside wp_get_sites() and support domain/path lookup.
  • Allow the current $site_id argument to be null (for all), or an array (for many), in addition to the current int expectation.

Attachments (1)

30233.diff (3.5 KB) - added by earnjam 9 years ago.

Download all attachments as: .zip

Change History (7)

#1 @earnjam
9 years ago

I'm having a hard time recreating the specified situation where the same domain/path combination are used on multiple networks...at least in the context of places where domain_exists() is used. Do you have some steps I can follow to replicate?

I think I have a patch ready that is a combination of the two possibilities you have for solutions here, but wanted to really test it out to make sure it addressed the issues you outlined above.

I'll go ahead and post it for educational purposes. It might not be the proper way to handle this, but at least a starting point for talking about this further.

@earnjam
9 years ago

This ticket was mentioned in Slack in #core-multisite by earnjam. View the logs.


9 years ago

#3 @earnjam
9 years ago

Related #31148

The work being done there would allow us to deprecate this.

#4 @jeremyfelt
9 years ago

  • Milestone changed from Awaiting Review to Future Release

#5 @thomaswm
8 years ago

The internals of domain_exists() have been rewritten in [37620]. Instead of hard-coded DB queries, it now uses get_sites(). That should make it pretty easy to implement this:

Allow the current $site_id argument to be null (for all), or an array (for many), in addition to the current int expectation.

This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.


8 years ago

Note: See TracTickets for help on using tickets.