Make WordPress Core

#63865 closed enhancement (fixed)

Replace warning-suppression with error-handler in _wp_can_use_pcre_u

Reported by: dmsnell Owned by: dmsnell
Priority: normal Milestone: 6.9
Component: Charset Version: 6.9
Severity: normal Keywords: has-patch
Cc: Focuses:

Description

Follow-up to [45611].
See #47632.

A WPCS “ignore” comment was added to _wp_can_use_pcre_u() to silence alarms about the use of warning-suppression, which generally is problematic. While this silenced WPCS, it left the issue in Core.

Warning suppression can be problematic:

  • It can mask and conflate unintended warnings.
  • It bypasses existing error-handling.
  • Some errors are not suppressed.

In this case, the WPCS rule is actually helpful in pointing out risk, though it has not led to updating the code. The function can implement a specific error-handler to focus on the specific detection it wants to make: whether Unicode PCRE patterns are supported.

Change History (8)

This ticket was mentioned in PR #9576 on WordPress/wordpress-develop by @dmsnell.


13 months ago
#1

  • Keywords has-patch added

Trac ticket: Core-63865

Trac ticket:

This ticket was mentioned in PR #9618 on WordPress/wordpress-develop by prashantsuwalka-orange.


12 months ago
#2

This patch removes the use of @preg_match warning suppression in the
_wp_can_use_pcre_u() function and replaces it with an explicit error handler.

Changes:

  • Removed the @ operator from preg_match.
  • Added error detection using error_get_last() to log any PCRE/u detection failures.
  • Preserves cached results and supports reset functionality.
  • Manual overrides (TRUE/FALSE) continue to work as expected.
  • Fully tested in local WordPress development environment to ensure all code paths function correctly.

This improves security and debugging by avoiding silenced warnings while maintaining the
original functionality of the function.
Trac ticket: 63865

This ticket was mentioned in Slack in #core by cyberorange. View the logs.


12 months ago

#4 @dmsnell
12 months ago

  • Owner set to dmsnell
  • Resolutionfixed
  • Status newclosed

In 60694:

Compat: Replace warning-suppression in _wp_can_use_pcre_u()

This patch replaces the use of the problematic error-suppresssion operator with a specific error-handler to catch and report Unicode PCRE support without raising the related issues of error-suppression: notably conflating errors and failing to prevent completely the logging of the warnings.

In this case, the WPCS rule against using error-suppression was actually helpful in pointing out the risk, but the code was left in place with an “ignore” comment to silence the violation; this patch addresses the risk and removes the need for the comment.

Developed in https://github.com/WordPress/wordpress-develop/pull/9576
Discussed in https://core.trac.wordpress.org/ticket/63865

Follow-up to: [45611].

Props dmsnell.
Fixes #63865.

#5 @dmsnell
12 months ago

  • Resolution fixed
  • Status closedreopened

Reopening to update sanitize_file_name() to call this instead of performing its own check-with-error-suppression.

This ticket was mentioned in PR #9678 on WordPress/wordpress-develop by @dmsnell.


12 months ago
#6

Trac ticket: Core-63865.

The sanitize_file_name() function attempts to make its own manual detection of Unicode PCRE support, but WordPress already provides a more-robust method of doing so. In order to avoid re-computation, it also stores the result in a static var inside the function. That check is already stored in a static var, however, inside _wp_can_use_pcre_u() check and so the extra cache is largely ineffective.

This patch leans on the recently-updated _wp_can_use_pcre_u() check and removes the superfluous static var.

Follow-up to #9576 Follow-up to [60694]

@dmsnell commented on PR #9678:


12 months ago
#7

Merged in [60695]

#8 @dmsnell
12 months ago

  • Resolutionfixed
  • Status reopenedclosed
Note: See TracTickets for help on using tickets.