Make WordPress Core

Opened 7 weeks ago

Last modified 6 weeks ago

#43208 new enhancement

Separate setting validation from sanitization

Reported by: flixos90 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Options, Meta APIs Keywords: 2nd-opinion has-patch needs-unit-tests
Focuses: Cc:


As widely known, validation is different from sanitization. A value should first be validated and then be sanitized. Historically, WordPress has been mixing these two responsibilities in the sanitize_option() function, however it is easily possible to add an extra layer on top of that which maintains full backward-compatibility.

Newer parts of core, such as the Customizer and the REST API, have been dealing with this in a better way, keeping the two separate. We can achieve the same for options themselves too.

I suggest introducing a validate_option_{$option} filter that works somewhat similar like the customize_validate_{$setting_id} filter used in the Customizer. It passes an empty WP_Error object that can be added to. In addition to allow separate validation from sanitization, it also makes handling of validation easier, since it can then automatically set the value to the previous value and call add_settings_error(), passing any error messages set, which matches current core behavior.

Attachments (4)

43208.diff (3.8 KB) - added by flixos90 7 weeks ago.
43208.2.diff (3.8 KB) - added by flixos90 6 weeks ago.
43208.3.diff (5.8 KB) - added by flixos90 6 weeks ago.
43208.4.diff (7.3 KB) - added by wakkermedia 6 weeks ago.

Download all attachments as: .zip

Change History (9)

7 weeks ago

#1 @flixos90
7 weeks ago

  • Keywords has-patch added; needs-patch removed

43208.diff introduces the validate_option_{$option} filter as a possible implementation. Note that it is only fired if previous core validation didn't already produce an error. This allows for full backward-compatibility, allows performing additional validation even on core values, while preventing plugins from removing errors detected by core.

For easy handling of setting validation, a $validate_callback argument has been introduced for register_setting() which is then hooked into the filter.

#2 @flixos90
7 weeks ago

An alternative approach: Introduce an entirely new function validate_option() and do all validation including the filter in there. This function would then be called inside of add_option() and update_option() (before sanitize_option()), and if an error is returned, make the function not change the value set and return false. This actually seems like a cleaner approach to me, but also like a bigger change. Let's discuss further.

6 weeks ago

6 weeks ago

#3 @flixos90
6 weeks ago

  • Keywords needs-unit-tests added

43208.2.diff is a cleaner approach to separating validation from sanitization. It introduces a function validate_option( $option, $value ), that consists of a filter to perform setting validation, which was inspired by the existing Customizer logic following a similar purpose. A validate callback can be provided intuitively via register_setting(). Benefits:

  • Validation is not tied into updating the option. This allows for flexible handling: First, check if the value is valid. Then, if invalid, cancel the process, or if valid, proceed with updating the option.
  • No backward-compatibility issues, as it's a new function. Validation of core options that were previously handled in sanitize_option() should remain that way.

Of course, just having the function there won't actually provide a huge benefit. Therefore 43208.3.diff contains implementation of the integration points, which are:

  • Updating an option via a settings page submission (src/wp-admin/options.php).
  • Updating an option from the Customizer (src/wp-includes/class-wp-customize-setting.php).
  • Updating an option via the REST API (src/wp-includes/rest-api/endpoints/class-wp-rest-settings-controller.php).

This makes handling feedback about validation errors simple and straightforward. The REST API previously didn't handle this at all, for settings page submissions it was achieved in a somewhat hacky way by using sanitize_option() and calling add_settings_error() from there. The Customizer was the only location already handling validation properly, however even here there's now the benefit of having a uniform way to register validation for settings. Validation callbacks specific to the Customizer will still fire, so it's fully backward-compatible.

@joehoyle @johnjamesjacoby @SergeyBiryukov Would like to have your feedback on this approach. Any objections/considerations? I'll work on unit tests for this too, if it's a viable approach.

This ticket was mentioned in Slack in #core by flixos90. View the logs.

6 weeks ago

6 weeks ago

#5 @WakkerMedia
6 weeks ago

in 43208.4.diff Added validate_option functionality to class-wp-customize-manager.php on line: 2303 - 2311 Because the customize validator is also used in that file.

Note: See TracTickets for help on using tickets.