WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#11771 closed defect (bug) (fixed)

stripslashes_from_options() prevents plugins from storing a site name with a slash?

Reported by: Denis-de-Bernardy Owned by:
Milestone: 3.0 Priority: normal
Severity: critical Version: 3.0
Component: Multisite Keywords: blocker
Focuses: Cc:

Description

stripslashes_from_options() looks extremely fishy.

doesn't it prevent a site from being called, say, "My dog's blog"?

Change History (15)

comment:2 @wpmuguru6 years ago

The stripslashes_from_options was added because saving "My dog's blog" as a site name resulted in a site name of "My dog\'s blog".

comment:3 @wpmuguru6 years ago

  • Resolution set to worksforme
  • Status changed from new to closed

comment:4 @Denis-de-Bernardy5 years ago

  • Keywords blocker added
  • Resolution worksforme deleted
  • Status changed from closed to reopened

In this case we should fix the origin of the symptom, rather than add band-aids.

comment:6 follow-up: @wpmuguru5 years ago

Here is the original MU ticket: http://trac.mu.wordpress.org/ticket/974. The patch noted on the ticket addressed the issue as far as ms-options.php & ms-edit.php is concerned. The stripslashes_from options function was added so that on upgrade any residual ones would be removed.

comment:7 in reply to: ↑ 6 @Denis-de-Bernardy5 years ago

Replying to wpmuguru:

Here is the original MU ticket: http://trac.mu.wordpress.org/ticket/974. The patch noted on the ticket addressed the issue as far as ms-options.php & ms-edit.php is concerned. The stripslashes_from options function was added so that on upgrade any residual ones would be removed.

As far as SVN blame is concerned, it got introduced here:

http://trac.mu.wordpress.org/changeset/2005

If you WPMU truly was inserting extra slashes at one point, either:

  • we decide it's old enough (I take it that 9 months is old, for a well-maintained WPMU site?), and the rare users who might have (unknowingly?) run into it have upgraded by now, so it's safe to remove it entirely; or
  • we want to keep it around for the same, rare, old-time WPMU users, in which case it should be conditionally applied -- if and only if db_version is below a certain number, etc.

Either way, this should not be left into WP as is.

comment:8 @Denis-de-Bernardy5 years ago

also, based on the ticket you point to, this only affected the screen that the site administrator can access. in other words, end-users would never be affected by this.

the code should be removed entirely, and seen as a bug that slipped into mu changeset 2005.

comment:9 @dd325 years ago

we want to keep it around for the same, rare, old-time WPMU users, in which case it should be conditionally applied -- if and only if db_version is below a certain number, etc.

We should be able to just add that to upgrade_28() if is_multisite() i would assume?

Code reference for stripslashes_from_options()

comment:10 follow-up: @Denis-de-Bernardy5 years ago

We should be able to just add that to upgrade_28() if is_multisite() i would assume?

Looks like it. But considering that this bug only ever affected site admins who would edit, say, a site's name in the sites management screen rather than under the individual site's settings / general screen, should we even bother doing so? I'd give this function much greater odds at breaking an end-user's strings than repairing them.

comment:11 follow-up: @dd325 years ago

should we even bother doing so?

I do tend of agree with you there, Anyone who was affected/is affected will probably have changed the name or would manually remove the \'s once they notice them.

comment:12 in reply to: ↑ 11 @Denis-de-Bernardy5 years ago

Replying to dd32:

should we even bother doing so?

I do tend of agree with you there, Anyone who was affected/is affected will probably have changed the name or would manually remove the \'s once they notice them.

Exactly. Let's remove this function entirely.

comment:13 in reply to: ↑ 10 @wpmuguru5 years ago

Replying to Denis-de-Bernardy:

We should be able to just add that to upgrade_28() if is_multisite() i would assume?

Looks like it. But considering that this bug only ever affected site admins who would edit, say, a site's name in the sites management screen rather than under the individual site's settings / general screen, should we even bother doing so?

It did affect other settings on the site/network options as well. Adding to one of the upgrade functions seems like an appropriate solution.

comment:14 @wpmuguru5 years ago

(In [13618]) move multisite upgrade functions to upgrade.php, see #11771

comment:15 @wpmuguru5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.