Make WordPress Core

Changeset 63380


Ignore:
Timestamp:
08/28/2026 12:04:49 AM (2 weeks ago)
Author:
dmsnell
Message:

HTML API: Refactor wp_get_admin_notice().

Refactors wp_get_admin_notice() to rely on the HTML API for HTML generation. This provides more reliable parsing and hardening for and against the provided arguments, but introduces behavioral changes that should only affect already-broken cases.

Namely, the function previously generated its output and then mandated that calling code run it through wp_kses(). This meant that certain kinds of corruption were possible which would break boundaries within the generated HTML and result in mangled output through wp_kses(). Now, the function preserves all boundaries when generating the output, meaning that wp_kses() will receive already-normalized content.

Developed in: https://github.com/WordPress/wordpress-develop/pull/13273
Discussed in: https://core.trac.wordpress.org/ticket/65984

Props dmsnell, joedolson, johnbillion, jorbin.
See #65984.

Location:
trunk
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/wp-includes/functions.php

    r63366 r63380  
    92389238         * @param string $message The message for the admin notice.
    92399239         */
    9240         $args       = apply_filters( 'wp_admin_notice_args', $args, $message );
    9241         $id         = '';
    9242         $classes    = 'notice';
    9243         $attributes = '';
     9240        $args = apply_filters( 'wp_admin_notice_args', $args, $message );
     9241
     9242        $wrap_with_p  = false !== $args['paragraph_wrap'];
     9243        $wrap_opener  = $wrap_with_p ? '<p>' : '';
     9244        $wrap_closer  = $wrap_with_p ? '</p>' : '';
     9245        $html_builder = new WP_HTML_Tag_Processor( "<div class=\"notice\">{$wrap_opener}" );
     9246        $html_builder->next_token();
    92449247
    92459248        if ( is_string( $args['id'] ) ) {
     
    92479250
    92489251                if ( '' !== $trimmed_id ) {
    9249                         $id = 'id="' . $trimmed_id . '" ';
     9252                        $html_builder->set_attribute( 'id', $trimmed_id );
    92509253                }
    92519254        }
     
    92549257                $type = trim( $args['type'] );
    92559258
    9256                 if ( str_contains( $type, ' ' ) ) {
     9259                if ( strlen( $type ) !== strcspn( $type, " \f\t\r\n" ) ) {
    92579260                        _doing_it_wrong(
    92589261                                __FUNCTION__,
     
    92679270
    92689271                if ( '' !== $type ) {
    9269                         $classes .= ' notice-' . $type;
     9272                        $html_builder->add_class( "notice-{$type}" );
    92709273                }
    92719274        }
    92729275
    92739276        if ( true === $args['dismissible'] ) {
    9274                 $classes .= ' is-dismissible';
     9277                $html_builder->add_class( 'is-dismissible' );
    92759278        }
    92769279
    92779280        if ( is_array( $args['additional_classes'] ) && ! empty( $args['additional_classes'] ) ) {
    9278                 $classes .= ' ' . implode( ' ', $args['additional_classes'] );
     9281                foreach ( $args['additional_classes'] as $class_name ) {
     9282                        $html_builder->add_class( $class_name );
     9283                }
    92799284        }
    92809285
    92819286        if ( is_array( $args['attributes'] ) && ! empty( $args['attributes'] ) ) {
    9282                 $attributes = '';
    9283                 foreach ( $args['attributes'] as $attr => $val ) {
    9284                         if ( is_bool( $val ) ) {
    9285                                 $attributes .= $val ? ' ' . $attr : '';
    9286                         } elseif ( is_int( $attr ) ) {
    9287                                 $attributes .= ' ' . esc_attr( trim( $val ) );
    9288                         } elseif ( $val ) {
    9289                                 $attributes .= ' ' . $attr . '="' . esc_attr( trim( $val ) ) . '"';
     9287                foreach ( $args['attributes'] as $name => $value ) {
     9288                        if ( is_int( $name ) ) {
     9289                                /*
     9290                                 * Boolean attributes may have been appended as numeric list items,
     9291                                 * for example, with `$args['attributes'][] = 'disabled'`. They should
     9292                                 * be recorded with the value serving as their name.
     9293                                 */
     9294                                $html_builder->set_attribute( $value, true );
     9295                        } elseif ( true === $value ) {
     9296                                $html_builder->set_attribute( $name, true );
     9297                        } elseif ( false !== $value ) {
     9298                                $html_builder->set_attribute( $name, trim( (string) $value ) );
    92909299                        }
    92919300                }
    92929301        }
    92939302
    9294         if ( false !== $args['paragraph_wrap'] ) {
    9295                 $message = "<p>$message</p>";
    9296         }
    9297 
    9298         $markup = sprintf( '<div %1$sclass="%2$s"%3$s>%4$s</div>', $id, $classes, $attributes, $message );
     9303        $markup  = $html_builder->get_updated_html();
     9304        $markup .= $message;
     9305        $markup .= "{$wrap_closer}</div>";
    92999306
    93009307        /**
  • trunk/tests/phpunit/tests/functions/wpAdminNotice.php

    r57100 r63380  
    2626                $actual = ob_get_clean();
    2727
    28                 $this->assertSame( $expected, $actual );
     28                $this->assertEqualHTML( $expected, $actual );
    2929        }
    3030
     
    155155                                        'type' => '"><script>alert("Howdy,admin!");</script>',
    156156                                ),
    157                                 'expected' => '<div class="notice notice-">alert("Howdy,admin!");"&gt;<p>A notice with an unsafe type.</p></div>',
     157                                'expected' => '<div class="notice notice-&quot;><script>alert(&quot;Howdy,admin!&quot;);</script>"><p>A notice with an unsafe type.</p></div>',
    158158                        ),
    159159                        'an unsafe ID'                              => array(
     
    162162                                        'id' => '"><script>alert( "Howdy, admin!" );</script> <div class="notice',
    163163                                ),
    164                                 'expected' => '<div id="">alert( "Howdy, admin!" ); <div class="notice"><p>A notice with an unsafe ID.</p></div>',
     164                                'expected' => '<div id="&quot;><script>alert( &quot;Howdy, admin!&quot; );</script> <div class=&quot;notice" class="notice"><p>A notice with an unsafe ID.</p></div>',
    165165                        ),
    166166                        'unsafe additional classes'                 => array(
     
    169169                                        'additional_classes' => array( '"><script>alert( "Howdy, admin!" );</script> <div class="notice' ),
    170170                                ),
    171                                 'expected' => '<div class="notice ">alert( "Howdy, admin!" ); <div class="notice"><p>A notice with unsafe additional classes.</p></div>',
     171                                'expected' => '<div class="notice &quot;><script>alert( &quot;Howdy, admin!&quot; );</script> <div class=&quot;notice"><p>A notice with unsafe additional classes.</p></div>',
    172172                        ),
    173173                        'a type that is not a string'               => array(
  • trunk/tests/phpunit/tests/functions/wpGetAdminNotice.php

    r57100 r63380  
    2222         */
    2323        public function test_should_return_admin_notice( $message, $args, $expected ) {
    24                 $this->assertSame( $expected, wp_get_admin_notice( $message, $args ) );
     24                $this->assertEqualHTML( $expected, wp_get_admin_notice( $message, $args ) );
    2525        }
    2626
     
    151151                                        'type' => '"><script>alert("Howdy,admin!");</script>',
    152152                                ),
    153                                 'expected' => '<div class="notice notice-"><script>alert("Howdy,admin!");</script>"><p>A notice with an unsafe type.</p></div>',
     153                                'expected' => '<div class="notice notice-&quot;><script>alert(&quot;Howdy,admin!&quot;);</script>"><p>A notice with an unsafe type.</p></div>',
    154154                        ),
    155155                        'an unsafe ID'                              => array(
     
    158158                                        'id' => '"><script>alert( "Howdy, admin!" );</script> <div class="notice',
    159159                                ),
    160                                 'expected' => '<div id=""><script>alert( "Howdy, admin!" );</script> <div class="notice" class="notice"><p>A notice with an unsafe ID.</p></div>',
     160                                'expected' => '<div id="&quot;><script>alert( &quot;Howdy, admin!&quot; );</script> <div class=&quot;notice" class="notice"><p>A notice with an unsafe ID.</p></div>',
    161161                        ),
    162162                        'unsafe additional classes'                 => array(
     
    165165                                        'additional_classes' => array( '"><script>alert( "Howdy, admin!" );</script> <div class="notice' ),
    166166                                ),
    167                                 'expected' => '<div class="notice "><script>alert( "Howdy, admin!" );</script> <div class="notice"><p>A notice with unsafe additional classes.</p></div>',
     167                                'expected' => '<div class="notice &quot;><script>alert( &quot;Howdy, admin!&quot; );</script> <div class=&quot;notice"><p>A notice with unsafe additional classes.</p></div>',
    168168                        ),
    169169                        'a type that is not a string'               => array(
Note: See TracChangeset for help on using the changeset viewer.