wp.sanitize.stripTags should iterate instead of using recursion

[jrchamp]

The changes to wp-sanitize.js seemed wasteful and like it produced dead code in the final return.

​https://github.com/WordPress/WordPress/commit/90d9bdc54c82229b3aa5a6e60d775f14b1f9a9dc#diff-9bedaa5d6abc6cb127b2f496bb2f7835

When I got into it to fix the dead code, I also felt a way about the recursion, because stack and memory exhaustion are two other potential issues. So this way should *at most* be one function call and only duplicate memory instead of being O(n*memory).

​https://github.com/WordPress/WordPress/commit/d9b128f50546a65c9cf0944ee61237e35ff269c0

This is probably a drive-by commit and for that I apologize. I hope it's more useful than not, but your mileage may vary. Thank you for making WordPress.

[sabernhardt]

applying changes to /js/_enqueues/wp/sanitize.js

[sabernhardt]

@jrchamp Thanks for the patch!

I simply moved those changes to the file that creates wp-includes/js/wp-sanitize.js.

[jrchamp]

Thanks @sabernhardt!

[flixos90]

Reviewing this as part of a bug scrub, it feels rather trivial to commit. Looks like nobody followed up here all the years, but it should be a small quick win to add.

This is the refresh

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

[SirLouen]

Test Report

Description

✅ This report validates that the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/8980.diff

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.27.5
• Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
• Browser: Chrome 137.0.0.0
• OS: Windows 10/11
• Theme: Twenty Twenty-Five 1.2
• MU Plugins: None activated
• Plugins:
  • Test Reports 1.2.0

Testing Instructions

1. Add the code provided in the artifacts to a plugin or functions.php or wherever it can be executed
2. Remember to build JS files before and after testing
3. Add the shortcode [wp_sanitize_demo] to a post
4. Check if the results are identical before and after the patch.

Actual Results

1. ✅ Patch works correctly, as expected

Additional Notes

I agree with @jrchamp, the recursion here seems completely unnecessary adds stack risks, also recursion is always more difficult to understand, and it's self-contained after all. I cannot see any advantages of using here the recursion version.

Anyway, I would have preferred here to see trouble in action, other than just a code refactor suggestion as code refactors are not generally well seen (and only up to be taken by certain committers that seem to be happy that day).

Supplemental Artifacts

Some code aimed for testers to test that works after and before:

add_shortcode( 'wp_sanitize_demo', 'wp_sanitize_demo_shortcode' );

function wp_sanitize_demo_shortcode() {
    wp_enqueue_script( 'wp-sanitize' );
    $html = '<div class="test"><script>alert("test");</script><p>This is a <b>test</b> with <i>HTML</i> tags</p> to be stripped</div>';
    
    $output = '<div id="wp-sanitize-demo">';
    $output .= '<h3>Original HTML:</h3>';
    $output .= '<pre>' . esc_html($html) . '</pre>';
    $output .= '<h3>Sanitized Result:</h3>';
    $output .= '<div id="sanitized-result"></div>';
    $output .= '<script>
        document.addEventListener( "DOMContentLoaded", function() {
            var original = ' . json_encode($html) . ';
            var sanitized = wp.sanitize.stripTags(original);
            document.getElementById("sanitized-result").textContent = sanitized;
        });
    </script>';
    $output .= '</div>';
    
    return $output;
}