Fix uncaught TypeError in FileHandler::validate_size() on a failed fopen

[priskz]

Problem

FileHandler::validate_size() passes fopen()'s return value straight into fstat() without checking it:

$handle = fopen( $this->url, 'r' );
$stat   = fstat( $handle );

When fopen() fails it returns false, and on PHP 8 fstat(false) raises an uncaught TypeError. The request that happened to be writing a log entry dies with a 500, rather than the log write simply failing.

Why it is rarely seen, and where it does bite

The call is guarded by file_exists( $this->url ) in write(), so on local disk the two calls essentially never disagree — which is presumably why this has survived.

On a network-backed filesystem they can. WordPress VIP, for instance, serves wp-content/uploads over HTTP, so file_exists() and the subsequent fopen() are two separate remote operations. They can disagree because of a stale stat cache, a transient service error, or another process rotating the file between them.

Observed in production on WordPress VIP: two requests 500'd inside check_connection_status() — i.e. while rendering the wp-discourse settings screen, which is also where an administrator would go to switch logging back off. Enabling logging can therefore lock you out of the UI that disables it.

The fix

Check the handle, and close it.

On the return value: the failure branch returns true (treat the size as within the limit) rather than false, deliberately. A false return causes write() to ++$this->file_number, set must_rotate, and close(), after which rotate() unlinks old log files. Rotating — and discarding logs — because a read failed would be a poor trade for what is often a transient condition, so skipping the size check once is the safer recovery. Happy to flip it if you'd prefer the opposite bias.

Also closes the handle. The current code opens the file on every write once the log exists and never closes it.

Testing

php -l clean. I have not added a unit test — reproducing it requires an fopen() failure while file_exists() succeeds, which is awkward to simulate against the real filesystem in tests/phpunit/test-file-handler.php without a stream-wrapper fixture. Glad to add one if you can point me at the pattern you'd prefer.

Verified against main at the time of writing; the unchecked handle is still present there.