WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#19566 closed defect (bug) (fixed)

Multisite: No Database Prefix causes strpos(): Empty delimiter

Reported by: Ipstenu Owned by: nacin
Milestone: 3.3.1 Priority: normal
Severity: minor Version: 3.3
Component: Multisite Keywords:
Focuses: Cc:

Description

From posts

http://wordpress.org/support/topic/strpos-empty-delimiter?replies=12
and
http://wordpress.org/support/topic/wp-33-upgrade-error-functionstrpos-empty-delimiter

NOW, as Nacin pointed out "An empty database prefix is not supported."

This is true, however if it's not supported, perhaps we need to go that extra mommy step and stop people from being able to make a site without a prefix. Clearly people are doin_it_wrong, but if that's going to break things, we may need to police it.

Change History (13)

comment:1 nacin2 years ago

  • Milestone changed from Awaiting Review to 3.3.1
  • Owner set to nacin
  • Status changed from new to reviewing

comment:2 jk3us2 years ago

  • Cc jk3us added

For the record, here is a previous ticket where support for empty prefixes was worked on (and added, but may not have been complete).

Then, this ticket was closed as invalid instead of making changes to prevent someone from making this mistake.

Version 0, edited 2 years ago by jk3us (next)

comment:3 nacin2 years ago

Yeah, I'm going to bring back full support for empty prefixes. It's easier for us to support them, than mark them as bad and force people to migrate to adding prefixes to their tables. (Messy.)

comment:5 Ipstenu2 years ago

Maybe ... both?

That is, make sure WP will work without them, since clearly people missed the boat, but then on the setup pages, don't allow someone to complete installing WP if they don't have one.

comment:6 knutsp2 years ago

@Ipstenu: If full support for empty prefixes are achieved, even in multisite, why should a non-empty prefix be enforced?

From #16229 you will see that the setup, as it is now, ignores a user entered empty prefix and silently inserts the default one. I would like to reopen it, but not if convinced that empty prefixes are potentially bad.

comment:7 Ipstenu2 years ago

@knutsp - My feeling is that it's problematic. While having no db prefix works in MOST situations, it doesn't work in all. And while we have full support in core, that doesn't mean a plugin (who could logically assume that prefixes exist) will always work. Database prefixing is standard 'best practice' according to every DB guru I've worked with. We should encourage this.

Basically I'm of the mind that we should continue to support it for the people who did it anyway, but going forward we should try to minimize the damage and not let new people do it. And it's not like that's going to prevent someone from making their own wp-config.php and skipping over the installer (which I do all the time). Just a double rainbow CYA.

comment:8 nacin2 years ago

By full support, I just mean it not breaking, but no DB prefix in multisite is a huge problem.

We store site-specific usermeta keyed by DB prefix. For example, 'wp_capabilities'.

The same carries over to multisite. For example, 'wp_4_capabilities', if the prefix is 'wp_' and the blog ID is 4.

In a network, the initial blog does not have a blog ID. That makes it 'capabilities' instead of '1_capabilities'.

We have a user option API that checks for a site-specific usermeta key first, then falls back to a global key. That means if 4_capabilities is not found, then capabilities is not used.

Now, 'capabilities' is a straw man here, as we strictly use user_meta rather than user_option for capabilities. But we do use user_option elsewhere, and we do *not* want it to fall back to the root site. While it may not have serious effects for a particular key in core (I haven't checked), it could potentially do damage to a lot of plugins.

As we can't die() on existing sites, we should block empty prefixes on single-site install, as well as on network.php to create a network. (We already block it for setup-config.php.) And I would go so far as to issue a visual warning for networks that have empty prefixes that they are doing it wrong, a _doing_it_wrong() message to make it into the logs (again only multisite), and a code comment in wp-config-sample.php.

comment:9 knutsp2 years ago

Thank you both for these excellent explanations.

comment:10 nacin2 years ago

In [19641]:

An empty database prefix is not supported for multisite. <small>Add a sanity check anyway.</small> see #19566.

comment:11 nacin2 years ago

We can start with this for now to suppress the E_WARNING on these installs.

Unfortunately, changing a table prefix requires a bit of work -- all tables need to be renamed, and a number of user meta keys will need to be updated. Not having a prefix to go on makes it really difficult to figure out which user keys need something prepended. It's a mess. So I don't feel too comfortable simply adding a note to network.php without coming up with some way to resolve it or document what to do.

comment:12 ryan2 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In [19643]:

An empty database prefix is not supported for multisite. <small>Add a sanity check anyway.</small> fixes #19566 for 3.3

comment:13 rmarks2 years ago

  • Cc rmarks@… added

I tried to search for this before opening creating #19970. It's my first patch, but I got burned by this one and wanted to help keep others from doing the same thing.

Note: See TracTickets for help on using tickets.