[wp-trac] [WordPress Trac] #58902: add_query_arg() should esc_url_raw() REQUEST_URI

[WordPress Trac]

#58902: add_query_arg() should esc_url_raw() REQUEST_URI
-------------------------------------------------+-------------------------
 Reporter:  jorbin                               |       Owner:  (none)
     Type:  defect (bug)                         |      Status:  new
 Priority:  normal                               |   Milestone:  Future
                                                 |  Release
Component:  Formatting                           |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch has-unit-tests dev-        |     Focuses:
  feedback has-test-info                         |
-------------------------------------------------+-------------------------
Changes (by SirLouen):

 * keywords:  has-patch has-unit-tests needs-testing needs-test-info => has-
     patch has-unit-tests dev-feedback has-test-info


Comment:

 == Reproduction Report
 === Description
 🟠 This report validates whether the issue can be reproduced with some
 warnings included in the notes.

 === Environment
 - WordPress: 6.9-alpha-60093-src
 - PHP: 8.4.6
 - Server: nginx/1.27.5
 - Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.4.6)
 - Browser: Chrome 136.0.0.0
 - OS: Windows 10/11
 - Theme: Twenty Twenty 2.9
 - MU Plugins: None activated
 - Plugins:
   * Test Reports 1.2.0

 === Reproduction Steps
 1. Add the coded provided in Supplemental Artifacts to whatever method you
 prefer to run code in WP
 2. Create a post and add the shortcode `[url_query_demo_shortcode]`
 2. 🐞 Resulting URLs are wrong

 === Expected Results (according to the report)
 - Using `add_query_arg` the resulting URL should be sanitized, hence right
 despite being poorly made by the developer.

 === Actual Results with the Patch
 - ✅ The resulting URL are correctly formatted.

 === Additional Notes
 - In this case, unit tests are pretty straightforward and testing could be
 done with them, but still, I have added a very simple tests to showcase
 this patch in case anyone wants to see visually where is the problem with
 this.
 - 🟠 I'm finding that not all proposed test cases are useful (they are
 still passing with or without URL sanitization, so they are basically
 useless). I think some of them can be trimmed, although none will cause
 any harm being left there.
 - ⚠️ I'm not 100% confident if it's the right solution that the URL is
 sanitized after adding a `add_query_arg`, or if it should return some sort
 of notice, for a poorly formatted URL.

 === Supplemental Artifacts
 Here I provide the code to create a shortcode and test this issue:

 {{{#!php
 function url_query_demo_shortcode() {
         $output  = '<div>';
         $output .= '<h2>URL Query String Demo</h2>';

         $urls_with_query_string = array(
                 'http://example.com/two words?foo=1' =>
 'http://example.com/two%20words?foo=1&bar=2',
                 'http;//example.com?foo=1'                       =>
 'http://example.com?foo=1&bar=2',
                 'example.com?foo=1'                              =>
 'http://example.com?foo=1&bar=2',
         );

         $output .= '<table class="url-test-table">';
         $output .= '<tr><th>Original URL</th><th>Expected</th><th>Result
 w/ QV</th></tr>';

         foreach ( $urls_with_query_string as $wrong_url => $expected ) {
                 $_SERVER['REQUEST_URI'] = $wrong_url;
                 $with_bar               = add_query_arg( array( 'bar' =>
 '2' ) );
                 $result = ( $with_bar !== $expected ) ? '❌' : '✅';
                 $output .= '<tr>';
                 $output .= '<td>' . $wrong_url . '</td>';
                 $output .= '<td>' . $expected . '</td>';
                 $output .= '<td>' . $with_bar . '</td>';
                 $output .= '<td>' . $result . '</td>';
                 $output .= '</tr>';
         }

         $output .= '</table>';

         $output .= '</div>';

         return $output;
 }
 add_shortcode( 'url_query_demo', 'url_query_demo_shortcode' );
 }}}

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