domain_exists should add trailing slash to path

[ejdanderson]

insert_blog runs trailingslashit on the path, however, domain_exists uses whatever is passed in. If WP is enforcing the trailing slash on blog insertion, it really should when checking existence. It should probably also cast the site_id as an int per insert_blog.

[jamescollins]

When working on #21142, I encountered $path being empty when creating a site via wp-signup.php, so I agree that we should be running trailingslashit on the path.

The $site_id parameter does not need to be cast to an integer though, because the $wpdb->prepare() takes care of that.

[jeremyfelt]

In both places that insert_blog() is used in core, domain_exists() is used immediately before. If we continue to enforce / on sites as they are inserted, we should use the same checks in both functions.

20589.diff​ is a refresh of the original patch that applies to trunk. Unit tests would need to be refreshed as we don't expect a trailing slash at this point.

Another answer may be to deprecate domain_exists() in favor of something more comprehensive. site_id is currently checked along with domain and path on every request. Using this with another site_id would not identify possible conflicts across multiple networks. This may be a bigger workflow to figure out in another ticket.

Moving to 3.9 for discussion. This could likely apply to an overall domain/path strategy as well.

[ericmann]

Adds a test to verify trailingslashit behavior.

[ericmann]

Contributed patch during WordCamp Phoenix!!!

[jeremyfelt]

We might want to hold on any changes to domain_exists() until get_site_by_path() is sorted via #27003. There's a chance that we could deprecate or change the internals so that the trailing slash is not necessary.

[jeremyfelt]

We can't use get_site_by_path() as a direct replacement in current trunk as it will always search for / as a path in addition to the passed $path argument.

The current patch does add tests and trailing slashes to domain_exists() so that current behavior gets a bit better. It would be nice to drop the site ID check at some point in the future, but this can probably go in to solve the immediate issue.

[nacin]

Replying to jeremyfelt:

We can't use get_site_by_path() as a direct replacement in current trunk as it will always search for / as a path in addition to the passed $path argument.

I wonder if that is an indication that get_site_by_path() is not quite flexible enough and/or isn't ideally named. Let's discuss that further.

[nacin]

Fixed by [27717], accidentally posted to #18209. Tests were accidentally omitted, adding those now.

[nacin]

In 27718:

Ensure the $path is trailing-slashed in domain_exists().

Adds tests for [27717].

props ejdanderson, ericmann.
fixes #20589.