'kses_allowed_protocols' filter is not really filterable.

[turtlepod]

in "wp_allowed_protocols()", "kses_allowed_protocols" filter only loaded once.

And if "esc_url()" is called early, it will load "wp_allowed_protocols()" function and basically disable the filter.

How to debug:

1. add "mu-plugins" file with "esc_url()". example code:

<?php
/* no not add empty string */
esc_url( 'google.com' );

2. Now, no plugin will ever able to filter "kses_allowed_protocols"

adding new protocols will not work. example plugin:
​https://wordpress.org/plugins/fx-skype-link-enabler/

Reason:
Here's the code for "wp_allowed_protocols()"
​https://developer.wordpress.org/reference/functions/wp_allowed_protocols/

function wp_allowed_protocols() {
    static $protocols = array();
 
    if ( empty( $protocols ) ) {
        $protocols = array( 'http', 'https', 'ftp', 'ftps', 'mailto', 'news', 'irc', 'gopher', 'nntp', 'feed', 'telnet', 'mms', 'rtsp', 'svn', 'tel', 'fax', 'xmpp', 'webcal' );
 
        /**
         * Filter the list of protocols allowed in HTML attributes.
         *
         * @since 3.0.0
         *
         * @param array $protocols Array of allowed protocols e.g. 'http', 'ftp', 'tel', and more.
         */
        $protocols = apply_filters( 'kses_allowed_protocols', $protocols );
    }
 
    return $protocols;
}

so, the filter only loaded once when this function is loaded for the first time.

Suggestion:
change it to something like:

function wp_allowed_protocols() {
    static $protocols = array();
 
    if ( empty( $protocols ) ) {
        $protocols = array( 'http', 'https', 'ftp', 'ftps', 'mailto', 'news', 'irc', 'gopher', 'nntp', 'feed', 'telnet', 'mms', 'rtsp', 'svn', 'tel', 'fax', 'xmpp', 'webcal' );
    }

    return apply_filters( 'kses_allowed_protocols', $protocols );
}

and move the filters to final output of the function.

Because:
Correct me if I'm wrong:
this filter is basically useless if user call "esc_url()" which call "wp_allowed_protocols()" and practically disable this filter. and make other plugin loaded later unable to add proper protocol for valid uses.

-- David.

[SergeyBiryukov]

Previously brought up in comment:18:ticket:18268.

This was done (in [18826]) for performance reasons, as running apply_filters() on each esc_url() call would be a performance hit.

We can't remove the static variable entirely, but we could probably use the approach from [31104]. If esc_url() was called before plugins_loaded, it will not prevent plugins from filtering the value on plugins_loaded or init. See 36033.patch​.

[turtlepod]

Replying to SergeyBiryukov:

Previously brought up in comment:18:ticket:18268.

thanks for the link.

This was done (in [31104]) for performance reasons, as running apply_filters() on each esc_url() call would be a performance hit.

I don't think there's going to be any performance hit.

If you have the time, please explain further and maybe results in testing (?). Did anyone actually test this?

I'm sure you already know that "apply_filters()" is already/also running on every "esc_url()" call (?) so, removing it will improve the code ?

​https://developer.wordpress.org/reference/functions/esc_url/ (line 3460)

    /**
     * Filter a string cleaned and escaped for output as a URL.
     *
     * @since 2.3.0
     *
     * @param string $good_protocol_url The cleaned URL to be returned.
     * @param string $original_url      The URL prior to cleaning.
     * @param string $_context          If 'display', replace ampersands and single quotes only.
     */
    return apply_filters( 'clean_url', $good_protocol_url, $original_url, $_context );

We can't remove the static variable entirely, but we could probably use the approach from [31104]. If esc_url() was called before plugins_loaded, it will not prevent plugins from filtering the value on plugins_loaded or init. See 36033.patch​.

Correct me if I'm wrong, but the only reason we use static so the filter will only loaded once.
I would suggest to remove the static (?)
however using "did_action( 'wp_loaded' )" might solve this for most case.
(not sure how much performance gain for this workaround).

[jpolakovic]

When Jetpack is network activated, it will call Jetpack::normalize_url_protocol_agnostic() that invokes esc_url_raw on every page load before muplugins_loaded action, making the kses_allowed_protocols filter unreachable for majority of plugins.

Replying to SergeyBiryukov:

We can't remove the static variable entirely, but we could probably use the approach from [31104]. If esc_url() was called before plugins_loaded, it will not prevent plugins from filtering the value on plugins_loaded or init. See 36033.patch​.

I like your quick fix. What needs to happen in order to get this merged?

A more robust solution would deprecate the kses_allowed_protocols filter and implement explicit static "cache" invalidation, something along these lines:

class WP_Allowed_Protocols {
        private static $protocols = [ 'http', 'https', 'ftp', 'ftps', 'mailto', 'news', 'irc', 'gopher', 'nntp', 'feed', 'telnet', 'mms', 'rtsp', 'svn', 'tel', 'fax', 'xmpp', 'webcal', 'urn' ];

        public static function get() {
                return self::$protocols;
        }

        public static function add( $protocols ) {
                $protocols = (array) $protocols;

                $protocols = array_filter( $protocols, function( $protocol ) {
                        return preg_match( '~^[a-z][a-z0-9\+\.\-]*$~', $protocol );
                } );

                self::$protocols = array_unique( array_merge( self::$protocols, $protocols ) );
        }

        public static function remove( $protocols ) {
                $protocols = (array) $protocols;

                self::$protocols = array_diff( self::$protocols, $protocols );
        }
}

function wp_allowed_protocols() {
        return WP_Allowed_Protocols::get();
}

[SergeyBiryukov]

This was done (in [31104]) for performance reasons

Looks like I mentioned a wrong changeset there, [18826] is the correct one.

I'm sure you already know that "apply_filters()" is already/also running on every "esc_url()" call (?)

Yes, but that's not a good reason to add more calls, see comment:10:ticket:18268.

Let's get 36033.patch​ in.

[turtlepod]

thank you @SergeyBiryukov that would be great!

[SergeyBiryukov]

#40317 was marked as a duplicate.

[dd32]

Punting as there's been no activity in 9 months, and the change will need testing.
@SergeyBiryukov feel free to re-milestone when you commit, if you do so this release.

[SergeyBiryukov]

In 41990:

Formatting: Make sure wp_allowed_protocols() is filterable until wp_loaded has fired.

Fixes the issue with plugins not being able to use the kses_allowed_protocols filter if esc_url() was called too early.

Props turtlepod, SergeyBiryukov.
Fixes #36033.