WordPress / WordPress/WordPress-Coding-Standards

Security/EscapeOutput: incorrect handling of namespaced functions named like array walking functions

Open
#2,674 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Component: Extra Priority: Low Type: Bug
Dominant language
PHP
Stars
2.8k
Forks
521
Avg merge
5d 20h
Merged PRs (30d)
1

Description

Bug Description

The WordPress.Security.EscapeOutput sniff incorrectly applies array walking function logic to namespaced functions that happen to share the same name as a valid global array walking function.

When the sniff encounters an array walking function like array_map() or map_deep(), it has special handling to check whether the callback properly escapes the data. However, this logic is triggered incorrectly for namespaced functions that share the same name, such as MyNamespace\array_map(), even though they are completely different functions.

Minimal Code Snippet

The issue happens when running this command:

phpcs --standard=WordPress --sniffs=WordPress.Security.EscapeOutput test.php

... over a file containing this code:

<?php

// Inconsistent handling - 2 errors for MyNamespace\array_map() and 1 error for MyNamespace\map_deep()
echo implode( '<br>', MyNamespace\array_map( 'esc_html', $items ) );
echo implode( '<br>', MyNamespace\map_deep( $items, 'esc_html' ) );

The sniff should recognize that MyNamespace\array_map() and MyNamespace\map_deep() are not the global array walking functions and handle them consistently as unescaped function calls. This would result in a single error for each line, instead of two errors for the first and one for the second.

Error Code

WordPress.Security.EscapeOutput.OutputNotEscaped

Environment

Question Answer
PHP version 8.5
PHP_CodeSniffer version 3.13.5
WordPressCS version develop
PHPCSUtils version 1.2.1
PHPCSExtra version 1.5.0
WordPressCS install type git clone
IDE (if relevant) N/A

Additional Context (optional)

This issue was split off from #2671 as suggested in https://github.com/WordPress/WordPress-Coding-Standards/issues/2671#issuecomment-3660791838.

The root cause is that the sniff checks if a function name matches an array walking function without first verifying whether the call is namespaced. With PHPCS 3.x tokenization, namespaced names are tokenized as separate T_STRING and T_NS_SEPARATOR tokens, so the sniff sees only array_map or map_deep and applies the special logic.

This issue likely does not occur when running WPCS with PHPCS 4.0 due to the changes in how namespaced names are tokenized. If that is the case, it may be preferable to wait for PHPCS 3.x support to be dropped rather than implementing a fix.

Tested Against develop Branch?

  • I have verified the issue still exists in the develop branch of WordPressCS.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by running the documented phpcs command against the provided namespaced-function example and inspect the WordPress.Security.EscapeOutput sniff's array-walking handling. Check how PHPCS 3.x tokenizes the namespace separator and compare the behavior with global array walking functions. Done means each namespaced call reports one OutputNotEscaped error while global-function handling remains unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.