Changeset 61003

Timestamp:

10/21/2025 03:48:20 AM (4 weeks ago)

Author:

dmsnell

Message:

HTML API: Replace PCRE in set_attribute() with new UTF-8 utility.

The HTML API has relied upon a single PCRE to determine whether to allow setting certain attribute names. This was because those names aren’t allowed to contain Unicode noncharacters, but detecting noncharacters without a UTF-8 parser is nontrivial.

In this change the direct PCRE has been replaced with a number of strcpn() calls and a call to the newer wp_has_noncharacters() function. Under the hood, this function will still defer to a PCRE if Unicode support is available, but otherwise will fall back to the UTF-8 pipeline in Core.

This change removes the platform variability, making the HTML API more reliable when Unicode support for PCRE is lacking.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/9798
Discussed in https://core.trac.wordpress.org/ticket/63863

See #63863.

Location:

trunk

Files:

• src/wp-includes/html-api/class-wp-html-tag-processor.php (modified) (1 diff)
• tests/phpunit/tests/html-api/wpHtmlTagProcessor.php (modified) (1 diff)

trunk/src/wp-includes/html-api/class-wp-html-tag-processor.php

@@ -3931 +3931 @@
         }
 
-        /*
+        $name_length = strlen( $name );
+
+        /**
          * WordPress rejects more characters than are strictly forbidden
          * in HTML5. This is to prevent additional security risks deeper
-         * in the WordPress and plugin stack. Specifically the
-         * less-than (<) greater-than (>) and ampersand (&) aren't allowed.
+         * in the WordPress and plugin stack. Specifically the following
+         * are not allowed to be set as part of an HTML attribute name:
          *
-         * The use of a PCRE match enables looking for specific Unicode
-         * code points without writing a UTF-8 decoder. Whereas scanning
-         * for one-byte characters is trivial (with `strcspn`), scanning
-         * for the longer byte sequences would be more complicated. Given
-         * that this shouldn't be in the hot path for execution, it's a
-         * reasonable compromise in efficiency without introducing a
-         * noticeable impact on the overall system.
+         *  - greater-than “>”
+         *  - ampersand “&”
          *
          * @see https://html.spec.whatwg.org/#attributes-2
-         *
-         * @todo As the only regex pattern maybe we should take it out?
-         *       Are Unicode patterns available broadly in Core?
          */
-        if ( preg_match(
-            '~[' .
-                // Syntax-like characters.
-                '"\'>&</ =' .
-                // Control characters.
-                '\x{00}-\x{1F}' .
-                // HTML noncharacters.
-                '\x{FDD0}-\x{FDEF}' .
-                '\x{FFFE}\x{FFFF}\x{1FFFE}\x{1FFFF}\x{2FFFE}\x{2FFFF}\x{3FFFE}\x{3FFFF}' .
-                '\x{4FFFE}\x{4FFFF}\x{5FFFE}\x{5FFFF}\x{6FFFE}\x{6FFFF}\x{7FFFE}\x{7FFFF}' .
-                '\x{8FFFE}\x{8FFFF}\x{9FFFE}\x{9FFFF}\x{AFFFE}\x{AFFFF}\x{BFFFE}\x{BFFFF}' .
-                '\x{CFFFE}\x{CFFFF}\x{DFFFE}\x{DFFFF}\x{EFFFE}\x{EFFFF}\x{FFFFE}\x{FFFFF}' .
-                '\x{10FFFE}\x{10FFFF}' .
-            ']~Ssu',
-            $name
-        ) ) {
+        if (
+            0 === $name_length ||
+            // Syntax-like characters.
+            strcspn( $name, '"\'>&</ =' ) !== $name_length ||
+            // Control characters.
+            strcspn(
+                $name,
+                "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0A\x0B\x0C\x0D\x0E\x0F" .
+                "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B\x1C\x1D\x1E\x1F"
+            ) !== $name_length ||
+            // Unicode noncharacters.
+            wp_has_noncharacters( $name )
+        ) {
             _doing_it_wrong(
                 __METHOD__,

trunk/tests/phpunit/tests/html-api/wpHtmlTagProcessor.php

@@ -310 +310 @@
 
         $this->assertSame( '<div data-enabled="abc">Test</div>', $processor->get_updated_html(), 'A case-insensitive set_attribute call did not update the existing attribute' );
+    }
+
+    /**
+     * Ensures that set_attribute doesn’t allow setting an
+     * attribute with an invalid name and thus break syntax.
+     *
+     * @ticket 63863
+     *
+     * @expectedIncorrectUsage WP_HTML_Tag_Processor::set_attribute
+     *
+     * @dataProvider data_invalid_attribute_names
+     *
+     * @param string $invalid_name Invalid attribute name.
+     */
+    public function test_set_attribute_rejects_invalid_names( $invalid_name ) {
+        $processor = new WP_HTML_Tag_Processor( '<div>' );
+        $processor->next_tag();
+
+        $this->assertFalse(
+            $processor->set_attribute( $invalid_name, true ),
+            'Should have rejected invalid attribute name.'
+        );
+    }
+
+    /**
+     * Data provider.
+     *
+     * @return array[]
+     */
+    public static function data_invalid_attribute_names() {
+        $invalid_names = array(
+            'Empty' => array( '' ),
+        );
+
+        // Syntax-like characters.
+        foreach ( str_split( '"\'>&</ =' ) as $c ) {
+            $invalid_names[ $c ] = array( "too{$c}late" );
+        }
+
+        // C0 controls.
+        for ( $i = 0; $i <= 0x1F; $i++ ) {
+            $c                                    = chr( $i );
+            $invalid_names[ "C0 Controls: {$i}" ] = array( "shut{$c}down" );
+        }
+
+        // Noncharacters.
+        for ( $i = 0xFDD0; $i <= 0xFDEF; $i++ ) {
+            $h                                       = dechex( $i );
+            $c                                       = mb_chr( $i );
+            $invalid_names[ "Noncharacter: U+{$h}" ] = array( "shut{$c}down" );
+        }
+
+        for ( $b = 0; $b <= 16; $b++ ) {
+            for ( $x = 0xFFFE; $x <= 0xFFFF; $x++ ) {
+                $i                                       = ( $b << 16 ) + $x;
+                $h                                       = dechex( $i );
+                $c                                       = mb_chr( $i );
+                $invalid_names[ "Noncharacter: U+{$h}" ] = array( "shut{$c}down" );
+            }
+        }
+
+        return $invalid_names;
     }