Skip to content

Don't flag php:// stream wrappers as remote requests - #896

Merged
GaryJones merged 1 commit into
developfrom
GaryJones/506-file-get-contents-php-stream-wrappers
Aug 18, 2026
Merged

Don't flag php:// stream wrappers as remote requests#896
GaryJones merged 1 commit into
developfrom
GaryJones/506-file-get-contents-php-stream-wrappers

Conversation

@GaryJones

Copy link
Copy Markdown
Contributor

Summary

The WordPressVIPMinimum.Performance.FetchingRemoteData sniff discourages remote requests made through file_get_contents(), pointing developers towards wpcom_vip_file_get_contents() or vip_safe_wp_remote_get() instead. It decided a call was remote by looking for :// anywhere in the string argument.

That heuristic misfires on PHP's php:// stream wrappers. php://input, php://stdin, php://memory, php://temp and friends are local, in-process streams, not remote HTTP requests, so neither suggested alternative applies. php://input is especially awkward: it is the canonical (and effectively unavoidable) way to read a raw request body, for example when handling an incoming webhook, and there is no VIP helper that replaces it. The result was a warning the developer could only silence with an ignore annotation.

This change exempts the php:// scheme from the remote-file check. Matching is anchored to the start of the string and is case-insensitive, so PHP://input is also recognised, while a php:// substring that is not the URL scheme (such as a query-string value on an https:// URL) continues to be flagged as remote.

Fixes #506.

Test plan

Unit tests are included in FetchingRemoteDataUnitTest.inc/.php covering the exempted wrappers, the case-insensitive scheme, and the regression guard that a masked php:// substring on a genuine remote URL is still flagged.

  • composer test -- --filter FetchingRemoteData

FetchingRemoteData treated any `file_get_contents()` argument containing
`://` as a remote request and steered users towards
`wpcom_vip_file_get_contents()` / `vip_safe_wp_remote_get()`. That misfires
for PHP's `php://` stream wrappers, which are local, in-process streams
rather than remote HTTP requests. `php://input` in particular is the
canonical, unavoidable way to read a raw request body (for example when
handling a webhook), so the warning was a false positive with no valid
alternative to offer.

Exempt the case-insensitive `php://` scheme from the remote-file check.
A `php://` substring that is not the URL scheme (such as a query-string
value on an https URL) is still flagged.

Fixes #506.
@GaryJones
GaryJones requested a review from a team as a code owner July 30, 2026 15:01
@GaryJones
GaryJones merged commit 89fc5dd into develop Aug 18, 2026
26 checks passed
@GaryJones
GaryJones deleted the GaryJones/506-file-get-contents-php-stream-wrappers branch August 18, 2026 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WordPressVIPMinimum.Performance.FetchingRemoteData.FileGetContentsRemoteFile incorrect for php://input

1 participant