WP_Filesystem_SSH2::put_contents() assumes that PHP's file_put_contents() always returns in int

[pbiron]

See https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-wp-filesystem-ssh2.php#L285.

The ​PHP manual for file_put_contents() says

This function returns the number of bytes that were written to the file, or false on failure.

I'm not sure it file_put_contents() could always return a boolean false or whether than is a PHP 8 thing, but we should make sure that the Filesystem API accounts for it

[afragen]

From ​php.net:file_put_contents

This function returns the number of bytes that were written to the file, or false on failure.

Warning
This function may return Boolean false, but may also return a non-Boolean value which evaluates to false. Please read the section on Booleans for more information. Use the === operator for testing the return value of this function.

[afragen]

Are we trying to mimic the response of file_put_contents() in $wp_filesystem::put_contents()?

[pbiron]

No. And WP_Filesystem_SSH2::put_contents() does use strict comparison when checking the return value from file_put_contents() so I don't think any errors/warnings would be raised if file_put_contents() were to return false.

So, maybe there's no change needed, but I just wanted to get other opinions about whether we should change the conditional to:

<?php
if ( false === $ret || strlen( $contents ) !== $ret ) {
   return false;
}

to avoid the call to strlen() when it's not necessary for an early return.

[pbiron]

Just looked into how PHP's strlen() is implemented.

It's O(1) (there's a len property associated with every string and that's what's returned), so I guess there's not a big performance hit for a needless call to strlen().

So, I'll close this as wontfix.