Replace most strip_tags() with wp_strip_tags()

[ipajen]

Originally reported as an issue with strip_tags() in wp-admin/admin-header.php, this ticket's scope as grown to initiative to replace strip_tags() with wp_strip_tags().

Proposal:

• Replace most calls to strip_tags() with calls to wp_strip_all_tags()

@peterwilsoncc notes:

this is a slight compatibility change as it will strip the contents of <style> and <script> tags, but I don't think that's a bad thing

• Add an early check in wp-admin/admin-header.php to set up the global variables.

@jrf notes:

I can see one more alternative option, which could (should) possible be applied in conjunction with the solution suggested by @peterwilsoncc: for wp-admin/admin-header.php to do an early check for the global variables it requires to be set and to set those to a default value (for example, an empty string for $title) if the variable is not available.

The original reported issue:

When using the plugin HD Quiz this warning is given in version 6.2-alpha-55159 with PHP 8.1.

Deprecated: strip_tags(): Passing null to parameter #1 ($string) of type string is deprecated in /public_html/blog/wp-admin/admin-header.php on line 36

Link to ticket for plugin:
​https://wordpress.org/support/topic/strip_tags-php8-1-warning/#new-topic-0

"Core expects the $title global to always be set when wp-admin/admin-header.php is included." Is there something Core can do to improve the experience here?

[peterwilsoncc]

This is due to a change to strip_tags in PHP which started throwing a warning when null is passed to the function, and a fatal error passed for other values. ​See this example page for how PHP has changed error handling for the function.

Folks are currently discussing how to handle this in the helper function wp_strip_all_tags() on the ticket #56434.

As WordPress calls strip_tags() directly in a number of cases, on this ticket let's discuss how to handle the deprecation warning elsewhere.

I see two options:

1. replace most calls to strip_tags() with calls to wp_strip_all_tags() -- this is a slight compatibility change as it will strip the contents of <style> and <script> tags, but I don't think that's a bad thing
2. add wp_strip_tags() as a wrapper for strip_tags() to maintain the current behavior

FWIW, I think option 1 is preferable as I think that's closer to the intended use of the function throughout WordPress.

Thanks for the ticket, I've moved it to the 6.2 milestone for visibility and consideration.

[jrf]

Thanks @ipajen for reporting this!

I concur with @peterwilsoncc that option 1 is the preferred path forward, though also want to mention this sounds like this is primarily an issue with a plugin doing_it_wrong and that the plugin should be fixed (as well).

I can see one more alternative option, which could (should) possible be applied in conjunction with the solution suggested by @peterwilsoncc: for wp-admin/admin-header.php to do an early check for the global variables it requires to be set and to set those to a default value (for example, an empty string for `$title) if the variable is not available.

What do you think @peterwilsoncc ?

[SergeyBiryukov]

Replying to jrf:

I can see one more alternative option, which could (should) possible be applied in conjunction with the solution suggested by @peterwilsoncc: for wp-admin/admin-header.php to do an early check for the global variables it requires to be set and to set those to a default value (for example, an empty string for `$title) if the variable is not available.

Indeed, that sounds like a good idea too. We already do this for $current_screen:

// Catch plugins that include admin-header.php before admin.php completes.
if ( empty( $current_screen ) ) {
	set_current_screen();
}

Might be worth doing the same for other variables, though I think I would do this on a case-by-case basis for those that are known to cause notices if not set, like $title here.

[peterwilsoncc]

@SergeyBiryukov @jrf Yeah, that makes sense too.

https://core.trac.wordpress.org/ticket/57579

[zamandevs]

The above issue causing due to get_admin_page_title() producing a null.
For other strip_tags uses we can consider wp_strip_all_tags() as a replacement.

[costdev]

This ticket was discussed in the bug scrub and it was agreed to move this ticket to 6.3.

This avoids the following warning on pages where a title is not defined:

PHP Deprecated:  strip_tags(): Passing null to parameter #1 ($string) of type string is deprecated in /wp-admin/admin-header.php on line 36

This should not normally happen, but some folks may pass null to add_submenu_page() when creating new pages, to hide that page from the admin menu. This may not be the best approach, but it is one that is documented in the codex and used in the wild: ​https://developer.wordpress.org/reference/functions/add_submenu_page/#comment-445

• Related Core trac ticket: https://core.trac.wordpress.org/ticket/57579
• Related PR: #4084

[jeherve]

I ran into the same issue on my end; I'm glad to see there is already a ticket and a proposed patch!

In the Pull Request linked above, I took a different approach from the original patch to fix that specific deprecation notice.

[oglekler]

This ticket was discussed in the recent bug scrub and due to lack of activity it was decided to move it into future release.

The solution still needs to be worked out. It also needs a unit test that only runs on PHP 8.1+ to verify that no deprecation notice is fired.

It can be moved to the next available milestone when it will be ready or into the current 6.3 if it will be ready before RC1.

Props: @costdev @Clorith

[hellofromTonya]

This ticket's scope has grown past the original reported issue. Thus, I've updated the Summary and Description to help contributors know what is being proposed and the work yet to be done.

I've also added the php-compatibility focus to include strip_tags() usage in Core in the PHP 8.1 compat exclusion list.

[slimndap]

This should not normally happen, but some folks may pass null to add_submenu_page() when creating new pages, to hide that page from the admin menu.

This also happens if you pass an empty string ('') as the first parameter ($parent_slug) to add_submenu_page().

[SergeyBiryukov]

#61841 was marked as a duplicate.