WordPress.org

Make WordPress Core

Opened 18 months ago

Closed 18 months ago

Last modified 18 months 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 18 months ago.

Download all attachments as: .zip

Change History (16)

comment:1 scribu18 months ago

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

comment:2 wpmuguru18 months ago

  • Keywords needs-patch added

comment:3 scribu18 months 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.

comment:4 scribu18 months 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.

scribu18 months ago

comment:5 SergeyBiryukov18 months ago

Possibly related: #18242

comment:6 nacin18 months ago

  • Priority changed from normal to high

comment:7 wpmuguru18 months ago

Why

echo $protocol; echo esc_html( $site_url_no_http );

instead of

echo $protocol . esc_html( $site_url_no_http );

comment:8 scribu18 months ago

No reason; that's how it was originally.

comment:9 scribu18 months 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

comment:10 scribu18 months ago

  • Keywords has-patch removed

comment:11 nacin18 months ago

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

comment:12 follow-up: scribu18 months ago

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

comment:13 in reply to: ↑ 12 ; follow-up: nacin18 months 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.

comment:14 scribu18 months 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.

comment:15 in reply to: ↑ 13 wpmuguru18 months 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.