Bundled themes: Add escaping for get_search_query()

[himshekhar07]

In the Twenty Eleven theme folder, the file named search.php has improper escaping on line number 21 as per the VIP standard.

Issue screenshot:
​https://share.cleanshot.com/3rPjnj33GHPcFfyL0rKh

The present line of code

printf( __( 'Search Results for: %s', 'twentyeleven' ), '<span>' . get_search_query() . '</span>' );

Improve line of code:

printf( esc_html__( 'Search Results for: %s', 'twentyeleven' ), '<span>' . esc_html( get_search_query() ) . '</span>' );

[SergeyBiryukov]

Hi there, welcome back to WordPress Trac! Thanks for the patch.

Please note that WordPress core does not use the ​WordPress VIP standards, they are specific to Automattic projects.

Previously, the point of view here was that core translations (including bundled themes) are considered safe because we have a review process for them, see #42639 and the discussion in #30724. (Also related: #32233.)

In WordPress core and older bundled themes, strings are generally only escaped in attributes or in <option> tags.

Some other related tickets: #47384, #47385, #49535, #49536, #49537, #54127, #56110, #57133.

This was recently reconsidered for the Twenty Twenty-One theme, see the discussion in ​#core-themes on Slack.

As the purpose of bundled themes is to demonstrate best practices, they should use proper escaping so that the code copied from or based on these themes also uses correct escaping. This has been addressed for Twenty Twenty-One and will be addressed for newer bundled themes going forward.

For updating the escaping in older themes though, there is no consensus yet, see the ​second part of the discussion. This should probably be discussed with the Themes team. Personally, I think either way is fine. As these themes are periodically updated for better block editor support, I guess we could address the escaping as well, but it should ideally be done in a consistent way rather than just in a few random occurrences.

[sabernhardt]

I do not recommend escaping translatable strings on this ticket, and it likely is not worth doing if themes such as Twenty Eleven ​might be retired soon.

However, the search query escaping is inconsistent.

1. Twenty Twenty's $archive_title runs through wp_kses_post() in index.php:

   $archive_title = sprintf(
   	'%1$s %2$s',
   	'<span class="color-accent">' . __( 'Search:', 'twentytwenty' ) . '</span>',
   	'&ldquo;' . get_search_query() . '&rdquo;'
   );
   

2. Twenty Sixteen and Twenty Twenty-One have esc_html() inside a span:

   // Twenty Sixteen search.php
   printf( __( 'Search Results for: %s', 'twentysixteen' ), '<span>' . esc_html( get_search_query() ) . '</span>' );
   // Twenty Twenty-One search.php and template-parts\content\content-none.php
   '<span class="page-description search-term">' . esc_html( get_search_query() ) . '</span>'
   

3. Some search.php templates do not escape within the span:

   // pattern in Twenty Ten, Twenty Eleven, Twenty Twelve and Twenty Seventeen
   printf( __( 'Search Results for: %s', 'twentyseventeen' ), '<span>' . get_search_query() . '</span>' );
   // Twenty Nineteen search.php
   <span class="page-description"><?php echo get_search_query(); ?></span>
   

4. Other search.php templates do not escape inside the h1 (without a span):

   // pattern for Twenty Thirteen, Twenty Fourteen and Twenty Fifteen:
   printf( __( 'Search Results for: %s', 'twentyfifteen' ), get_search_query() );
   

5. The function output is not escaped inside value attributes (which may be unnecessary).

   // Twenty Sixteen searchform.php
   <input type="search" class="search-field" placeholder="<?php echo esc_attr_x( 'Search &hellip;', 'placeholder', 'twentysixteen' ); ?>" value="<?php echo get_search_query(); ?>" name="s" />
   // Twenty Seventeen searchform.php
   <input type="search" id="<?php echo $unique_id; ?>" class="search-field" placeholder="<?php echo esc_attr_x( 'Search &hellip;', 'placeholder', 'twentyseventeen' ); ?>" value="<?php echo get_search_query(); ?>" name="s" />
   // Twenty Twenty searchform.php
   <input type="search" id="<?php echo esc_attr( $twentytwenty_unique_id ); ?>" class="search-field" placeholder="<?php echo esc_attr_x( 'Search &hellip;', 'placeholder', 'twentytwenty' ); ?>" value="<?php echo get_search_query(); ?>" name="s" />
   // Twenty Twenty-One searchform.php
   <input type="search" id="<?php echo esc_attr( $twentytwentyone_unique_id ); ?>" class="search-field" value="<?php echo get_search_query(); ?>" name="s" />
   

[karmatosed]

The plan isn't set for things like retiring yet so I am going to leave this ticket to 'needs patch' as there are clearly some inconsistencies could be looked at. Removing 2nd-opinion as that is my belief that this is the right path for the ticket currently, we can review appetite as and when we have a patch or get to fewer tickets in default themes. Thanks everyone.

• esc_html() in search.php templates
• esc_attr() in searchform.php templates

Trac 58127

I updated each of the Search template headings to use esc_html without running esc_attr too (including Twenty Sixteen, Twenty Twenty and Twenty Twenty-One).
esc_html( get_search_query( false ) )

[sabernhardt]

plugin to identify where esc_html and esc_attr functions are called on the front end (view HTML source, not the broken page)

[sabernhardt]

Testing the pull request with each theme:

1. Create a post with an HTML tag in its title (for example, "First line <br>Second line"). If your site does not already have a search form, you could add a Search block to the post.
2. Publish the post and refresh the editor.
3. Verify that the block editor uses the title's HTML tag as HTML. If you still see "<br>" in the title, you may need to visit the Posts list and use Quick Edit.
4. Visit the site, and enter your post title in the search form so it returns a result.
5. View the search results page's HTML source and find the h1 element. It should have < in place of < and > instead of >. If you activate the attached plugin with the current PR applied, you should find |esc_html| but not |esc_attr|.

Twenty Twenty-One has an additional instance of get_search_query() in its content-none template. To test that, enter a search query that would return zero results (for example, "Unfindable <br>search query").