WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#22235 closed defect (bug) (maybelater)

Blog path included in base

Reported by: scribu Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.5
Component: Multisite Keywords: close
Focuses: Cc:

Description

Previously: #19796

My config:

define('MULTISITE', true);
define('SUBDOMAIN_INSTALL', false);
$base = '/core/';
define('DOMAIN_CURRENT_SITE', 'wp.dev');
define('PATH_CURRENT_SITE', '/core/');
define('SITE_ID_CURRENT_SITE', 1);
define('BLOG_ID_CURRENT_SITE', 1);

When I go to Network Admin -> Sites, the list of blogs looks like this:

/core/
/core/foo/
/core/bar/

and then, if I go to edit /core/bar/, I can change /core/ to something else, making that particular blog inaccessible.

Attachments (1)

22235.diff (3.2 KB) - added by scribu 6 years ago.

Download all attachments as: .zip

Change History (16)

#1 @scribu
6 years ago

  • Owner set to MarkJaquith
  • Status changed from new to assigned

#2 @wpmuguru
6 years ago

  • Keywords needs-patch added

#3 @scribu
6 years ago

I think the most elegant way to go about this would be to combine the Path and Domain input fields into a single URL input and then use parse_url() to update the relevant bits.

And, if the new path doesn't start with $base, we can show an error message and refuse to save it.

#4 @scribu
6 years ago

  • Owner MarkJaquith deleted

22235.diff is the start of a patch. It doesn't validate the base yet.

Looking at the additional logic in get_blogaddress_by_domain(), I get the impression that my patch will break something, but I'm not sure what.

@scribu
6 years ago

#5 @SergeyBiryukov
6 years ago

Possibly related: #18242

#6 @nacin
6 years ago

  • Priority changed from normal to high

#7 @wpmuguru
6 years ago

Why

echo $protocol; echo esc_html( $site_url_no_http );

instead of

echo $protocol . esc_html( $site_url_no_http );

#8 @scribu
6 years ago

No reason; that's how it was originally.

#9 @scribu
6 years ago

  • Keywords has-patch close added; needs-patch removed
  • Priority changed from high to normal

I'm not sure this is such a big deal anymore. I mean, you could just change the domain completely and MS won't stop you, even if you don't have domain mapping enabled.

Created a new ticket for combining the fields: #22383

#10 @scribu
6 years ago

  • Keywords has-patch removed

#11 @nacin
6 years ago

Was this a regression? I was under the opinion that it was.

#12 follow-up: @scribu
6 years ago

It would be a regression, if we did any validation before, like checking that the user didn't change the domain.

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

  • Milestone changed from 3.5 to Future Release

Replying to scribu:

It would be a regression, if we did any validation before, like checking that the user didn't change the domain.

But we didn't... So this is not a regression and has been around since the merge, yes? If so, punting. Feel free to close as a duplicate of #22383 if you think they can merge.

#14 @scribu
6 years ago

  • Milestone Future Release deleted
  • Resolution set to maybelater
  • Status changed from assigned to closed

Correct. Going to close as maybelater, in case we ever want to start adding comprehensive validation.

#15 in reply to: ↑ 13 @wpmuguru
6 years ago

Replying to nacin:

Replying to scribu:

It would be a regression, if we did any validation before, like checking that the user didn't change the domain.

But we didn't... So this is not a regression and has been around since the merge, yes?

Yes.

Note: See TracTickets for help on using tickets.