get_blog_details is sensitive to path's trailing slash

[mitchoyoshitaka]

STR:

1. Create a site, then edit it so that the domain is the main domain and the path is /sub. Note that the admin doesn't turn this into /sub/, nor give an error.
2. Go to /sub/wp-admin. You'll notice it doesn't redirect, but it just gives you the main site's wp-admin. Going to /sub also just gets interpreted as a URL for the main site.

Potential solutions:

1. Make the admin automatically turn the path of /sub into /sub/.
2. Make get_blog_details match modulo trailing slash.
3. (Document carefully in Codex and hope people read it.)

[jeremyfelt]

A couple weird things can happen when updating a site address via site-info.php if you don't watch yourself. See ​http://cl.ly/image/413y2X3j3V1Q for slashes galore.

We could probably do a bandaid fix here, but I'm not sure if it's completely critical. While random numbers of slashes can be entered on the page, it can be edited fairly easily through the same screen once you see it doesn't work.

I kind of want to say maybelater, with the grand hopes that some of this gets a refresh one day. This ticket has been hanging out for 2 years without a lot of noise.

[jeremyfelt]

#17376 is slightly related to step 2 in the original ticket description

[jeremyfelt]

18117.diff​ corrects the out of control slashes on site-info.php

1. Moves the capture of $_POST['blog'] data via wp_unslash() further up the file
2. Removes leading and trailing forward slashes from domain
3. Ensures leading and trailing forward slashes on path
4. Allows for clean display of URL and URL parts at site-info.php, clean URLs stored in siteurl and home, and clean DB entries in the blogs table.

[nacin]

Per conversation with jeremyfelt, moving to future.

[simonwheatley]

Patch refreshed, and unit tests from #23865 added

[simonwheatley]

The patch by @jeremyfelt still appears good, after a very slight refresh for offsets. I've added the unit tests I originally wrote for #23865, which pass with the patch.

[jeremyfelt]

Rather than attempting to resolve this in one specific place - site-info.php - it makes sense to go to the root. 18117.3.diff adds some sanitization to update_blog_details() for path.

• Explode into an array on /, so that multiple subdirectories can be specified.
• Strip all empty array elements, so that /// doesn't cause issues.
• Implode with / and add leading and trailing slashes.

I actually think the above could be used for subdirectory validation at some point, but only when we're ready.

The tests @simonwheatley added in 18117.2.diff will work, though we'll need to refresh them and add to the tests/multisite/site.php file. We could also think about breaking them into one test per assertion. They definitely should not be added to the massive list of current update_blog_details() tests.

This would fix #23865 as well.

[earnjam]

I tried 18117.3.diff but it failed a couple of tests and kicked a ton of errors. Upon further investigation, I realized it was setting the path to // for the main site and causing lots of problems. This was due to it forcing two slashes to be added here:
$details[ $field ] = '/' . implode( '/', $details[ $field ] ) . '/';

In 18117.4.diff I changed that line to:
$details[ $field ] = trailingslashit( '/' . implode( '/', $details[ $field ] ) );
That fixed the problem and now it's passing all the tests.

I also added the tests @simonwheatley included in 18117.2.diff and added a few more to cover the multiple subdirectories use case that @jeremyfelt mentioned.

I also had to fix some tests in test_update_blog_details(). They were attempting to update the path without a leading slash (my-path/) and asserting against that. Since we'd now be sanitizing the slashes properly in update_blog_details() that test would no longer pass because it checks for the wrong thing.

So that last portion actually points out another issue. When a super admin updates the path of a site from site-info.php, it updates the siteurl and home url prior to running through update_blog_details() where we would now be checking the slashes. If they leave off the leading slash, then you end up with http://domain.compath. This needs to be fixed too, but it should probably be its own ticket.

[jeremyfelt]

Nice work @earnjam, good catch on the double trailing slashes. I'll run this through its paces in my configuration as well.

I'm wondering if we can tackle #23865 at the same time as it's very much related to your last comment.

[earnjam]

#23865 seems to be resolved with this patch.

I also added #30417 which is related to my last comment above about the siteurl and home values getting broken. I've added a patch for that piggybacks on this and uses the changes in update_blog_details() to make sure those values are correct too.

[jeremyfelt]

This is working great, @earnjam! I messed with things for a while and it all seems in place. The tests in 18117.5.diff have been updated to match some of the work from #30080.

I'm going to hold for now and bring this up during the multisite chat tomorrow. I'm confident that you've nailed it here, but I'm wary of dropping a change to update_blog_details() right before a release candidate. I would also very much like to include the fix for #30417 at the same time.

[jeremyfelt]

This is ready to go in early in 4.2, along with #30417 - this should all fix #23865 as well.

[jeremyfelt]

In 31155:

Enforce leading and trailing slashes on paths updated with update_blog_details()

In multisite, core expects the stored value for a site's path to have leading and trailing slashes. When these slashes are missing, it becomes impossible to visit the site.

This enforces proper /path/ creation in update_blog_details(), most likely used when updating an existing site through site-info.php.

Props earnjam, simonwheatley.

Fixes #18117. Fixes #23865.

[jeremyfelt]

After a ​conversation and some thought, I was stretching a bit in an earlier patch to "validate" the number of slashes in all segments of a saved path. This isn't necessarily the right place to do that, especially because array_filter() will strip out some valid paths - try /my-site/0/.

By using $details[ $field ] = trailingslashit( '/' . trim( $details[ $field ], '/' ) );, we can make sure any number of slashes on the front and back are stripped before adding ours back in.

[jeremyfelt]

In 31158:

Use a less complex approach for enforcing path slashes in update_blog_details()

Ensure leading and traling slashes are in place and don't touch anything in the middle. Validating with array_filter() would have missed a possible valid falsy path - /my-path/0/.

Props nacin.

Fixes #18117.