WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#11730 closed feature request (fixed)

Should display a warning when processing a setting which is not registered

Reported by: westi Owned by: westi
Milestone: 3.0 Priority: normal
Severity: normal Version: 3.0
Component: Plugins Keywords: has-patch
Focuses: multisite Cc:

Description

The settings api has a whitelist mechanism which is enforced in MU but not in plain WordPress.

As the two are converging we need a easy way of warning plugin developers that they are not using the whitelist.

If would be good to have something like the _deprecated* functionality when WP_DEBUG is enabled for this.

Attachments (1)

11730.diff (1.5 KB) - added by nacin 4 years ago.
Simplified unregistered setting warning.

Download all attachments as: .zip

Change History (18)

comment:1 nacin4 years ago

Jan 05 21:36:07 2010
westi: I'll throw a ticket in trac.. might even get someone like nacin writing the patch for it :-)

Arm sufficiently twisted, but I can't seem to find where it is enforced in MU trunk.

comment:2 nacin4 years ago

  • Keywords multisite added

comment:3 wpmuguru4 years ago

[13646] Display a warning when processing an unregistered setting.

comment:5 nacin4 years ago

(In [13721]) Fix branching, and correct deprecated version numbers. see #11730

nacin4 years ago

Simplified unregistered setting warning.

comment:6 nacin4 years ago

  • Keywords has-patch added

I've looked over this some more and came up with some improvements in 11730.diff. Posting as a patch first in case I missed something.

comment:7 follow-up: wpmuguru4 years ago

The whitelisting was introduced in [8802] which was 2.6.

comment:8 in reply to: ↑ 7 nacin4 years ago

Replying to wpmuguru:

The whitelisting was introduced in [8802] which was 2.6.

2.7 per milestone in #7277, the inline docs, and http://core.trac.wordpress.org/browser/trunk/wp-includes/version.php?rev=8802 :-)

comment:9 nacin4 years ago

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

(In [13785]) Simplify the deprecated notice for unregistered settings. fixes #11730

comment:10 wpmuguru4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

The only unregistered settings that are processed in single WP or in MS by a site admin are ones where the $option_page is 'options' AND the setting is not whitelisted. If the setting is whitelisted, then it has been registered.

The last patch removes the whitelisting check.

comment:11 nacin4 years ago

(In [13789]) Move unregistered check to proper branch of code. prevents options.php panel from throwing errors. see #11730

comment:12 nacin4 years ago

I've spent a while going over the original changesets, MU code, and other documentation on this. Unless I am misreading this, an option cannot be whitelisted while being on an 'options' option_page, unless the option was manually whitelisted without being properly registered.

There are technically other ways to "whitelist" options, such as via the filter, but MU 2.7+ only checks for 'options' == $option_page. Donncha wrote register_setting() and also the MU 2.7+ whitelist enforcement check, so I used his narrow implementation when I wrote [13785].

The $unregistered check was added to the same branch of code as the original MU check, but I realized that was preventing the hidden options.php form from functioning. So I've moved it to a better location in [13789].

comment:13 wpmuguru4 years ago

See the whitelist_options filter on line 73. After a svn up this morning, this does not throw a warning on an unregistered setting.

See also http://wpmututorials.com/how-to/writing-plugins/.

comment:14 nacin4 years ago

add_option_whitelist() is used by option_update_filter() which is designed to append to the whitelist array any settings that have been registered.

The settings appended are registered via register_setting() and its alias add_option_update_handler(), or unregistered via unregister_setting() or its alias remove_option_update_handler().

Best I can tell, doing a just-in-time registration by hooking into the whitelisting filter is not the proper way to register, and that add_option_whitelist() is a function for internal use. I have tracked the code down to http://trac.mu.wordpress.org/changeset/1188 in trying to determine the intent here.

In order to clean this up, we should make most of these private access, and deprecate the *_option_update_handler() functions in favor of register_setting() and unregister_setting().

comment:15 wpmuguru4 years ago

I'm not going to have time to look at this for a few days. It's ok (imo) for the beta to go out as is and we can discuss sometime in the next week or so. I'm not opposed to tightening things down some.

comment:16 wpmuguru4 years ago

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

No one has reported anything with this. I'm closing pending someone reporting an issue.

comment:17 nacin4 years ago

I just spent 20 minutes looking for http://mu.wordpress.org/forums/topic/7415, so I'm pasting that link here for future reference.

Note: See TracTickets for help on using tickets.