Fix avoid file_exists() on oversized strings in File detection to prevent PHP warnings

[the-hercules]

Closes #258

Summary

File::detectAndProcessFile() calls file_exists() while determining whether an
input string is a local path, before the plain-base64 branch is reached. When the
input is a large base64 payload (e.g. an image returned by a provider via
bytesBase64Encoded), the string exceeds the platform's maximum path length and
PHP emits:

file_exists(): File name is longer than the maximum allowed path length on this platform (4096): /9j/4AAQSkZJRg...

The warning message embeds the entire input string, so each occurrence writes
~1 MB to the error log. In practice this produced multi-megabyte error logs from
only a handful of image-generation calls. Base64-encoded JPEG data begins with
/9j/, which resembles an absolute path, so the string reaches file_exists()
before detection falls through to the base64 handling that processes it correctly.

Functionally the input was always handled correctly — this is a log-noise issue,
not a data-correctness one.

Change

Guard the filesystem check with a length comparison:

if (strlen($file) <= PHP_MAXPATHLEN && file_exists($file) && is_file($file)) {

AI Disclosure

Claude Opus 4.8 was used for identification and then verification of correctness of the solution.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: the-hercules <thehercules@git.wordpress.org>
Co-authored-by: felixarntz <flixos90@git.wordpress.org>
Co-authored-by: Infinite-Null <ankitkumarshah@git.wordpress.org>
Co-authored-by: tyrann0us <tyrannous@git.wordpress.org>
Co-authored-by: giacomolanzi <glanzi@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.58%. Comparing base (a31b0ec) to head (18aed1b).
⚠️ Report is 11 commits behind head on trunk.

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk     #267      +/-   ##
============================================
+ Coverage     86.49%   86.58%   +0.08%     
- Complexity     1327     1384      +57     
============================================
  Files            68       69       +1     
  Lines          4295     4449     +154     
============================================
+ Hits           3715     3852     +137     
- Misses          580      597      +17     

Flag	Coverage Δ
unit	86.58% <100.00%> (+0.08%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[felixarntz]

@the-hercules Great catch! I think we may be able to find a cleaner solution though.

[felixarntz]

not sure this is the adequate check - can't we instead perform a check for whether the string is possibly a file path? maybe check if it starts with / (since it always needs to be an absolute path in practice)

[tyrann0us]

[…] this is a log-noise issue […]

Not entirely. I came across this because on nginx (Apache untested), with Query Monitor active, image generation fails with:

The response is not a valid JSON response.

According to Claude Code, this is because Query Monitor takes the full warning text and puts it in an X-QM-php-errors-error-1 response header, causing a 502 "too big header" error, and thus failing the image generation/insertion. I confirmed it by deactivating Query Monitor. Alternatively, a mu-plugin like this would also work around the issue:

add_filter( 'qm/dispatch/rest', '__return_false' )
// or more specifically
add_filter( 'qm/outputter/headers', fn( $o ) => array_diff_key( $o, [ 'php_errors' => 1 ] ), 999 ); // untested

[the-hercules]

Hi @felixarntz
Thanks for the review! I looked at this closely, and a leading / check wouldn't actually prevent the reported warning on its own. Base64-encoded JPEGs start with /9j/, which already passes a "starts with slash" test.

The warning is triggered purely by the length, the length guard is the part that actually prevents the warning here. I feel this is the standard fix for this specific PHP behavior.

Adding a leading-slash requirement might also unnecessarily drop support for relative local paths.

@tyrann0us Yes it might be more than a log issue at this point.

[Infinite-Null]

Hi @the-hercules, thank you for the PR 🙌

I was wondering if it would make sense to suppress the warning from the filesystem probe as well?

if (strlen($file) < PHP_MAXPATHLEN && @file_exists($file) && @is_file($file)) {

I also think the test should cover the exact PHP_MAXPATHLEN boundary and a smaller Base64 payload, so we know both cases are covered.

One small thing to keep in mind: the current test's custom error handler will still capture warnings even when @file_exists() is used, so the test may need to account for the suppressed warning as well.

What do you think?

[the-hercules]

Hey @Infinite-Null, thanks for the review! 🙌

These are good suggestions, I agree with the points:

• < instead of <= — You're right, PHP_MAXPATHLEN includes the null terminator so strict less-than is the safer boundary. Will fix.
• @ suppression — Makes sense as a defensive way, masks error, but can it make debugging harder?
• Boundary + smaller payload tests — Yes, can add cases for the exact PHP_MAXPATHLEN boundary and a shorter base64 string.

[the-hercules]

Hey @Infinite-Null, all three are in 🙌

• < → <= was a real off-by-one: PHP warns at >= MAXPATHLEN, so exactly 4096 chars could still be logged.
• @ suppression is necessary, not just defensive. With open_basedir enabled, PHP can log the full input even when it’s below the length limit. The length guard alone therefore didn’t fully address the issue.
• Updated the error-handler check to (error_reporting() & $errno) !== 0, correctly handling @ suppression across PHP 7.4, 8.0, 8.4, and 8.5.

Added boundary and smaller-payload tests. All tests pass across the supported PHP versions.

One caveat: the warning tests only reproduce with open_basedir enabled, so they don’t fail against the old code in the default CI environment. Making them enforce the behavior would require running them in a separate process with open_basedir configured.