wp_tempnam produces filenames longer than 255 characters

[doems]

Summary
Since the maximal filename length of the most filesystems out there is 255, the function should try to avoid filenames longer than 255 characters.

Reproduce
If the passed-in $filename parameter is longer than 245 characters (WordPress appends a string which has 10 characters), PHP will fail on the following line: ​https://github.com/WordPress/WordPress/blob/6995ec613e5f113cedea4f0481b9030f79fabc0a/wp-admin/includes/file.php#L183.

Fix

<?php
if ( strlen( $filename ) > 245 ) {
    $filename = substr( $filename, 0, 244);
}

[obenland]

Introduced in [35644] for #34562.

Most filesystems have a maximum filename length of 255 characters.
wp_tempnam() adds characters to the $filename to make it unique, which can push it beyond this limit.

fopen() will fail silently due to the @ error suppression operator, and the file won't be created.
The suppressed warning is:

Warning: failed to open stream: File name too long

Existing checks inside wp_tempnam() fail to capture this failure, and even if they did, simply recalling wp_tempnam() will produce the same issue.

This checks the size of the generated unique filename before calling fopen().
If it's over 255, the additional characters are subtracted from the initial $filename and it's passed to wp_tempnam() again.

Includes PHPUnit tests.

[costdev]

@azaozz @mikeschroder @audrasjb ​PR 4647 addresses the issue directly in wp_tempnam().

In the future, this fix could possibly be extended/moved to wp_unique_filename().

However, as wp_unique_filename() is used within the Media component and has other considerations/potential impacts, this would need investigation by Media component maintainers in a separate ticket. As we don't know how long this may take, we can fix wp_tempnam() in the meantime so its users are protected from this issue.

It would be ideal to get this fix considered for inclusion in Beta 1 as it touches a Filesystem API function

[mikeschroder]

I have not tested this manually, but I think the approach is solid, and am +1 on this.

I wish we could do it at the same time for wp_unique_filename(), but fixing it in one place doesn't keep us from doing future changes!

[oglekler]

wp_unique_filename() should not be responsible for file name length, it has one clear purpose. It is possible to introduce a new function, something like file_name_sanity_check() but it should be called before the wp_unique_filename().

[costdev]

@oglekler What would the new function do before wp_unique_filename() is run?

[oglekler]

@costdev you wrote on the PR that wp_unique_filename() is called a second time after cutting the filename, but calling a function a second time doesn't look like an optimal scenario. So, I believe that a better approach will be to check name length and cut it if needed, and only then call wp_unique_filename() once. If check of file length and cutting will be moved to another function, it will be accessible to use in other places.

[costdev]

If we cut the filename, then call wp_unique_filename(), it could add characters that make the filename go over the 255 character limit.

The return value of wp_unique_filename() is also filterable, so we can't anticipate the length accurately. To ensure the unique filename isn't over the limit, we need to check the difference between the result of wp_unique_filename() and 255, and cut the difference off the original filename before re-running.

[costdev]

@azaozz @oglekler I've added some more test cases to ​PR 4647 which protect the new behaviour. You can test ideas for alternative implementation locally with vendor/bin/phpunit --group 35755 (or npm run test:php -- --group 35755 if you're using wp-env).

[mrinal013]

This ticket is discussed in yesterday's bug-scrub meeting.

Seems like it is complex to test from the user's perspective. So, what are the steps to test?

[mrinal013]

PHPUnit Test result

npm run test:php -- --group 35755
......
OK (9 tests, 20 assertions)

wp --info

OS: Linux 5.19.0-46-generic #47~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Wed Jun 21 15:35:31 UTC 2 x86_64
......
PHP version: 8.2.7
.....
MySQL version: mysql Ver 8.0.33-0ubuntu0.22.04.2 for Linux on x86_64 ((Ubuntu))

[costdev]

Test plugin.

[costdev]

Testing Instructions

These steps define how to reproduce the issue, and indicate the expected behavior.

Steps to Reproduce

1. Install and activate the test plugin.
2. 🐞 Check the results of the admin notice.

Expected Results

When reproducing a bug:

• ❌ The notice should display the following:

Length of filename to be created: 311 ❌
Was the file created? No ❌

When testing a patch to validate it works as expected:

• ✅ The notice should display the following:

Length of filename to be created: 252 ✅
Was the file created? Yes ✅

[costdev]

@azaozz @mikeschroder What do you think, is this ready for commit consideration now ahead of 6.3 Beta 4 tomorrow?

[azaozz]

In 56186:

Filesystem API: Ensure wp_tempnam() does not produce file names longer than 255 characters as this is the limit on most filesystems.

Props: costdev, doems, mikeschroder, oglekler, mrinal013.
Fixes: #35755.

Committed in https://core.trac.wordpress.org/changeset/56186.

[costdev]

#52844 was marked as a duplicate.