[wp-trac] [WordPress Trac] #60347: wp_kses breaking text fragments links

[WordPress Trac]

#60347: wp_kses breaking text fragments links
-------------------------------------------------+-------------------------
 Reporter:  asafm7                               |       Owner:  (none)
     Type:  defect (bug)                         |      Status:  new
 Priority:  normal                               |   Milestone:  Awaiting
                                                 |  Review
Component:  Formatting                           |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  good-first-bug has-unit-tests        |     Focuses:
  changes-requested has-testing-info             |
-------------------------------------------------+-------------------------
Changes (by SirLouen):

 * keywords:  good-first-bug has-patch has-unit-tests needs-testing => good-
     first-bug has-unit-tests changes-requested has-testing-info


Comment:

 == Test Report
 === Description
 🟠 This report validates that the indicated patch works as expected, with
 some caveats

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

 === Environment
 - WordPress: 6.9-alpha-60093-src
 - PHP: 8.2.28
 - Server: nginx/1.27.4
 - Database: mysqli (Server: 8.4.4 / Client: mysqlnd 8.2.28)
 - Browser: Chrome 134.0.0.0
 - OS: Windows 10/11
 - Theme: My Twenty Twenty Child Theme 1.0
 - MU Plugins: None activated
 - Plugins:
   * Test Reports 1.2.0
   * Text Fragment Link Tester 1.0

 === Bug Reproduction
 1. Add the supplemental code for testing to functions.php, a plugin, or
 anywhere you can execute it
 2. Go to the Tools > Text Fragment Tester page
 3. 🐞 Bug appears, the URL is not shown as expected

 === Expected Results
 - We can confirm that the URL remains unmodified in the script

 === Actual Results
 - 🟠 Issue resolved with the patch, but it appears that the patch could be
 improved.

 === Additional Notes
 1. As @azaozz has pointed out in the PR, there are some issues with
 filtering that should be covered without generating a security issue
 https://developer.mozilla.org/en-US/docs/Web/URI/Reference/Fragment
 2. Check if it is possible to generalize to more potential fragments
 types, not just text fragments
 3. Also, the `wp_kses_bad_protocol` function is specifically checking for
 protocols, consider if it's out of context given that a fragment is not
 technically a protocol, maybe a new specific function makes sense for this
 like `wp_kses_bad_fragment`
 4. @johnbillion anything `kses` is **never** a good first bug...

 === Supplemental Artifacts
 Here is the code for testing

 {{{#!php
 <?php

 add_action('admin_menu', 'text_fragment_tester_menu');

 function text_fragment_tester_menu() {
     add_management_page(
         'Text Fragment Tester',
         'Text Fragment Tester',
         'manage_options',
         'text-fragment-tester',
         'text_fragment_tester_page'
     );
 }

 function text_fragment_tester_page() {
     $original_link = '<a href="#:~:text=highlight">Original Link</a>';
     $allowed_html = array(
         'a' => array(
             'href' => array(),
         )
     );
     $filtered_link = wp_kses($original_link, $allowed_html);

     echo '<div class="wrap">';
     echo '<h1>Text Fragment Link Test</h1>';

     echo '<h2>Original HTML:</h2>';
     echo '<pre>' . esc_html($original_link) . '</pre>';
     echo '<div>' . $original_link . '</div>';

     echo '<h2>After wp_kses:</h2>';
     echo '<pre>' . esc_html($filtered_link) . '</pre>';
     echo '<div>' . $filtered_link . '</div>';
     echo '</div>';
 }
 }}}

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/60347#comment:12>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform