Make WordPress Core

Opened 4 weeks ago

Last modified 4 weeks ago

#61631 new defect (bug)

sanitize_option_{$option} inconsistencies

Reported by: liedekef's profile liedekef Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 6.5.5
Component: Options, Meta APIs Keywords:
Focuses: Cc:

Description

There are several inconsistencies with the sanitize_option_{$option} doc and code:

1) in the doc at https://developer.wordpress.org/reference/hooks/sanitize_option_option/ ==> the example

add_filter('sanitize_option_contact_email', 'sanitize_email');

should be

add_filter('sanitize_option_contact_email', 'sanitize_email', 10, 2);

2) in the code wp-includes/option.php, the filter is created in the function "register_setting" but the option name is not passed, leaving people to jump through holes to create the correct filter.

add_filter( "sanitize_option_{$option_name}", $args['sanitize_callback'] );

should become

add_filter( "sanitize_option_{$option_name}", $args['sanitize_callback'], 10, 2 );

(or even "10,3" since the the call to apply the filter always uses 3 arguments). This doesn't affect any current usage of the filter (php ignores it if too many arguments are passed to a function) but would make the life of developers easier :-)

Change History (4)

#1 @roytanck
4 weeks ago

add_filter's third and fourth argument are optional, and default to 10 and 1 respectively. So if your sanitize function only requires the value to be sanitized, it's completely valid to leave them out. In this case, the hooked function is called with a single argument, the value of the option that needs to be sanitized.

In the case of this example (1) the contact email address can probably be sanitized without the info in the second and third argument.

It seems to me like register_setting (2) uses a number of factors to determine whether the sanitize callback needs one or more arguments, and calls add_filter accordingly. This looks correct to me. Please note that register_setting is not adding a filter (which would be done using apply_filters) but instead uses the existing filter.

#2 @liedekef
4 weeks ago

The example as-in on https://developer.wordpress.org/reference/hooks/sanitize_option_option/ for sanitize_option_contact_email would cause a php error, so it needs to be corrected. The current example would cause the sanitize_email function to be called with 1 param only while that function in the example requires 2 params.
And in the register_setting function definition in wp-includes/option.php the add_filter code is being called with only 1 param (the callback function name):

        if ( ! empty( $args['sanitize_callback'] ) ) {
                add_filter( "sanitize_option_{$option_name}", $args['sanitize_callback'] );
        }

There's no logic on the number of params, so it results in the callback function to be called later on with only 1 param at all times.

#3 @roytanck
4 weeks ago

The 'sanitize_email' function expects only a single argument, so adding 10, 2 would actually cause a PHP error.
https://developer.wordpress.org/reference/functions/sanitize_email/

In case of (2), the callback function is defined when register_setting is called to register a new setting (usually from a plugin or theme). Unless the option is a default option, the callback is called with a single argument, the option's value. This is simply how the Setting API works. Since the option's key is path of the hook's name, there's little point in supplying it as a argument.

#4 @liedekef
4 weeks ago

In the example on https://developer.wordpress.org/reference/hooks/sanitize_option_option/ , the function sanitize_email takes 2 arguments, so then that needs to be corrected:

function sanitize_email($value, $option) {
//...
}

Concerning register_setting: many people use an array that contains all their own options (just separate options) and use that logic to call register_setting:

        foreach ( $options as $opt ) {
                register_setting ( 'my-options', $opt, 'my_callback' );
        }

Because certain options need different sanitizing, it requires different callback functions to be created currently, with if-statements inside the foreach. And since some options are arrays on their own, it requires yet more logic.
If the callback had the option name added, developers could do with just 1 callback function and put the logic in there. Currently my way around this limitation is:

        foreach ( $options as $opt ) {
                register_setting ( 'my-options', $opt );
                add_filter ( 'sanitize_option_' . $opt, 'my_callback', 10, 2 );
        }

Adding the option name to the callback wouldn't even have any impact on current filters ...

Note: See TracTickets for help on using tickets.