Separate setting validation from sanitization

[flixos90]

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.

[flixos90]

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.

[flixos90]

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.

[flixos90]

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.

[WakkerMedia]

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.

[flixos90]

43208.5.diff​ makes the following changes:

• Fix minor bug in register_setting() where the count of arguments passed to the filter was wrong prior.
• Only call validate_option() in Customizer if the customize setting is an actual option, NOT part of a multidimensional array option (a new utility method is_multidimensional() has been introduced for that).
• Call validate_option() before calling update_option() in the Customizer, and do not save if errors occur.

The latter two changes above ensure that options in the Customizer are validated as expected. We could go even further if we wanted to and even validate multidimensional option settings. That would require us to call validate_option() with the whole root value, with the actual value being part of the array (which could happen with the multidimensional_replace() method). However, since all of this logic needs to happen in the WP_Customize_Manager as well, we'd need to make the get_root_value() and multidimensional_replace() methods public, which would be ugly. On the other hand, the requirement to have duplicate logic here is ugly already anyway. :/

Pinging @johnjamesjacoby for general feedback and @westonruter for the Customizer part specifically.

[flixos90]

43208.6.diff​ ensures the patch applies cleanly again. It also changes the validate_option_{$option} filter into an action, in accordance which how a similar new hook was handled in #40364. An action is the proper way to amend a passed-in object. They are passed by reference, so filtering them through can only have negative side-effects with invalid filter values.

I also wrote a small test plugin for that, which registers a few different options with validation adds an interface to them via an options page, a Customizer section, and the REST API settings controller. You can easily test the new functionality using it.

<?php
/*
Plugin Name: Option Validation Tests
Plugin URI: https://make.wordpress.org/
Description: Tests for option validation.
Version: 1.0
Author: WordPress Core Contributors
Author URI: https://make.wordpress.org/
*/

function test_register_settings() {
        register_setting(
                'option_validation_tests',
                'ovt_integer',
                array(
                        'type'              => 'integer',
                        'description'       => __( 'An integer option.' ),
                        'sanitize_callback' => 'absint',
                        'validate_callback' => function( $errors, $value ) {
                                test_validate_setting( $errors, $value, array( 'min' => 3 ) );
                        },
                        'show_in_rest'      => true,
                )
        );

        register_setting(
                'option_validation_tests',
                'ovt_email',
                array(
                        'type'              => 'string',
                        'description'       => __( 'An email option.' ),
                        'sanitize_callback' => 'sanitize_email',
                        'validate_callback' => function( $errors, $value ) {
                                test_validate_setting( $errors, $value, array( 'email' => true ) );
                        },
                        'show_in_rest'      => true,
                )
        );

        register_setting(
                'option_validation_tests',
                'ovt_bool',
                array(
                        'type'              => 'boolean',
                        'description'       => __( 'A boolean option.' ),
                        'sanitize_callback' => 'boolval',
                        'validate_callback' => function( $errors, $value ) {
                                test_validate_setting( $errors, $value, array( 'checked' => true ) );
                        },
                        'show_in_rest'      => true,
                )
        );

        register_setting(
                'option_validation_tests',
                'ovt_enum',
                array(
                        'type'              => 'string',
                        'description'       => __( 'An enum option.' ),
                        'sanitize_callback' => 'strip_tags',
                        'validate_callback' => function( $errors, $value ) {
                                test_validate_setting( $errors, $value, array( 'enum' => array( 'value1', 'value2', 'value3' ) ) );
                        },
                        'show_in_rest'      => true,
                )
        );
}
add_action( 'init', 'test_register_settings' );

function test_validate_setting( $errors, $value, $args = array() ) {
        if ( isset( $args['enum'] ) && ! in_array( $value, $args['enum'], true ) ) {
                $errors->add( 'value_not_in_enum', sprintf( __( 'The value %s is not one of the valid choices.' ), esc_html( $value ) ) );
        }

        if ( isset( $args['min'] ) && (float) $value < (float) $args['min'] ) {
                $errors->add( 'value_too_small', sprintf( __( 'The value %1$s is smaller than the allowed minimum %2$s.' ), number_format_i18n( (float) $value ), number_format_i18n( (float) $args['min'] ) ) );
        }

        if ( isset( $args['max'] ) && (float) $value > (float) $args['max'] ) {
                $errors->add( 'value_too_great', sprintf( __( 'The value %1$s is greater than the allowed maximum %2$s.' ), number_format_i18n( (float) $value ), number_format_i18n( (float) $args['max'] ) ) );
        }

        if ( isset( $args['email'] ) && $args['email'] && ! is_email( $value ) ) {
                $errors->add( 'value_not_an_email', sprintf( __( 'The value %s is not a valid email address.' ), esc_html( $value ) ) );
        }

        if ( isset( $args['checked'] ) && $args['checked'] && ! $value ) {
                $errors->add( 'value_unchecked', __( 'You must check the checkbox.' ) );
        }
}

function test_add_settings_fields() {
        add_settings_section( 'general', __( 'General' ), null, 'option_validation_tests' );

        add_settings_section( 'other', __( 'Other' ), null, 'option_validation_tests' );

        add_settings_field(
                'ovt_integer',
                __( 'Integer Option' ),
                function() {
                        ?>
                        <input type="number" name="ovt_integer" value="<?php echo esc_attr( get_option( 'ovt_integer' ) ); ?>" />
                        <p class="description"><?php esc_html_e( 'Enter a value greater than or equal to 3.' ); ?></p>
                        <?php
                },
                'option_validation_tests',
                'general',
                array( 'label_for' => 'ovt_integer' )
        );

        add_settings_field(
                'ovt_email',
                __( 'Email Option' ),
                function() {
                        ?>
                        <input type="email" name="ovt_email" value="<?php echo esc_attr( get_option( 'ovt_email' ) ); ?>" />
                        <p class="description"><?php esc_html_e( 'Enter an email address.' ); ?></p>
                        <?php
                },
                'option_validation_tests',
                'general',
                array( 'label_for' => 'ovt_email' )
        );

        add_settings_field(
                'ovt_bool',
                __( 'Boolean Option' ),
                function() {
                        ?>
                        <input type="checkbox" name="ovt_bool" <?php checked( get_option( 'ovt_bool' ) ); ?> />
                        <p class="description"><?php esc_html_e( 'Check this checkbox.' ); ?></p>
                        <?php
                },
                'option_validation_tests',
                'general',
                array( 'label_for' => 'ovt_bool' )
        );

        add_settings_field(
                'ovt_enum',
                __( 'Enum Option' ),
                function() {
                        ?>
                        <input type="text" name="ovt_enum" value="<?php echo esc_attr( get_option( 'ovt_enum' ) ); ?>" />
                        <p class="description"><?php esc_html_e( 'Enter either value1, value2, or value3.' ); ?></p>
                        <?php
                },
                'option_validation_tests',
                'other',
                array( 'label_for' => 'ovt_enum' )
        );
}
add_action( 'admin_init', 'test_add_settings_fields' );

function test_register_settings_page() {
        add_options_page( __( 'Option Validation Test' ), __( 'Option Validation Test' ), 'manage_options', 'option_validation_tests', 'test_render_settings_page' );
}
add_action( 'admin_menu', 'test_register_settings_page' );

function test_render_settings_page() {
        ?>
        <div class="wrap">
                <h1><?php _e( 'Option Validation Test' ); ?></h1>

                <form action="options.php" method="post" novalidate="novalidate">
                        <?php settings_fields( 'option_validation_tests' ); ?>
                        <?php do_settings_sections( 'option_validation_tests' ); ?>
                        <?php submit_button(); ?>
                </form>
        </div>
        <?php
}

function test_customize_register( $wp_customize ) {
        $wp_customize->add_setting( 'ovt_integer', array( 'type' => 'option' ) );
        $wp_customize->add_setting( 'ovt_email', array( 'type' => 'option' ) );
        $wp_customize->add_setting( 'ovt_bool', array( 'type' => 'option' ) );
        $wp_customize->add_setting( 'ovt_enum', array( 'type' => 'option' ) );

        $wp_customize->add_section(
                'option_validation_tests',
                array(
                        'title'      => __( 'Option Validation Test' ),
                        'capability' => 'manage_options',
                )
        );

        $wp_customize->add_control(
                'ovt_integer',
                array(
                        'section'     => 'option_validation_tests',
                        'type'        => 'number',
                        'label'       => __( 'Integer Option' ),
                        'description' => __( 'Enter a value greater than or equal to 3.' ),
                )
        );

        $wp_customize->add_control(
                'ovt_email',
                array(
                        'section'     => 'option_validation_tests',
                        'type'        => 'email',
                        'label'       => __( 'Email Option' ),
                        'description' => __( 'Enter an email address.' ),
                )
        );

        $wp_customize->add_control(
                'ovt_bool',
                array(
                        'section'     => 'option_validation_tests',
                        'type'        => 'checkbox',
                        'label'       => __( 'Boolean Option' ),
                        'description' => __( 'Check this checkbox.' ),
                )
        );

        $wp_customize->add_control(
                'ovt_enum',
                array(
                        'section'     => 'option_validation_tests',
                        'type'        => 'text',
                        'label'       => __( 'Enum Option' ),
                        'description' => __( 'Enter either value1, value2, or value3.' ),
                )
        );
}
add_action( 'customize_register', 'test_customize_register' );

Still looking for feedback on this, I consider it a valuable enhancement that could make plugin authors' lives a lot easier.

It also opens up possibilities for further improvements: For example, $wp_customize->add_setting() calls could automatically happen for options that are registered because providing those manually in such a case is often redundant. If someone registers a control with an option identifier for an option that is registered and no $wp_customize->add_setting() call is manually made, those could be "auto-filled".

[johnjamesjacoby]

I just ran into this issue today, and searching reminded me that this ticket exists, with a patch!

It needs a refresh again, but it's very close to being viable.

• Introduces validate_option() function.
• Introduces validate_option_{$option} action, which receives a WP_Error object to amend in case of validation errors, as well as the value to validate. The value is explicitly not to modify, keeping validation and sanitization separate.
• Allows passing a validate_callback argument to register_setting(), causing this callback to be added as an action for validate_option_{$option}.
• Integrates option validation via validate_option() into the following areas:
  • updating options via Settings API
  • updating options via REST API
  • updating options via Customizer

Note that validate_option() should be called separately from (and prior to) update_option(). If the validate_option() call then returns a WP_Error, the option should not be updated, i.e. the call should not be made. Alternatively this could be integrated into the main Option API, however then we would need to alter update/add_option() return values to support WP_Error etc., making this more complex and furthermore coupling the functionality too much IMO.

Trac ticket: https://core.trac.wordpress.org/ticket/43208

[flixos90]

Thanks for the ping @johnjamesjacoby! I agree, IMO (something like) this is still worth adding to core after all these years. @TimothyBlynJacobs What are your thoughts on this?

I've refreshed the patch via the above PR, which also makes it easier to review.