Remove redundant dot in sanitize_file_name function

[ArtZ91]

Some web-servers returns Forbidden error if filename contains redundant dot, for example: filename..jpg

Maybe need to trim this dot inside sanitize_file_name.

​https://github.com/WordPress/wordpress-develop/pull/3713

Some web-servers returns Forbidden error if filename contains redundant dot, like filename..jpg.

Trac ticket: https://core.trac.wordpress.org/ticket/57242

[costdev]

Hi @ArtZ91, thanks for opening this ticket!

Can you provide any more information about why servers do this, for the benefit of others reading this ticket?

Also, a couple of questions:

1. What happens if you replace the first dot with %2E? i.e.

​https://example.org/wp-content/uploads/2022/12/test-fajl-s-kirilliczej%2E.png

2. Does this also occur if the filename begins with .., or contains .. anywhere else? i.e.

​https://example.org/wp-content/uploads/2022/12/..test-fajl-s-kirilliczej.png

Or

​https://example.org/wp-content/uploads/2022/12/test-fajl-s-kiril..liczej.png

[ArtZ91]

I dont know about server thrid-party configuration.

Test 1:
test-fajl-s-kirilliczej%2E.png
→
/wp-content/uploads/2022/12/test-fajl-s-kirilliczej2e.png
200 OK

Test 2:
..test-fajl-s-kirilliczej.png
→
/wp-content/uploads/2022/12/test-fajl-s-kirilliczej.png
200 OK

Test 3:
/wp-content/uploads/2022/12/test-fajl-s-kiril..liczej.png
→
403 Forbidden

Looks like server has some strange protection from .. double dot in URL.

[SergeyBiryukov]

Hi there, welcome back to WordPress Trac! Thanks for the ticket.

Replying to ArtZ91:

Some web-servers returns Forbidden error if filename contains redundant dot, for example: filename..jpg

It sounds like .. triggers some security rule on the server, e.g. to prevent directory traversal. This appears to be similar to #45368, although that ticket is about .. in post content.

Applying rtrim( $filename, '.' ) before appending the extension probably makes sense. On the other hand, as noted above, that does not fix the issue if .. is in the middle of the file name.

So I'm not quite sure about this change, curious to see what others think.

[costdev]

We could:

1. Replace all instances of .. (or more) with . (or -, etc)
2. Apply rtrim( $filename, '.' ).
3. Append the extension.

See ​https://3v4l.org/AeAuO

[SergeyBiryukov]

Replying to costdev:

We could:

1. Replace all instances of .. (or more) with . (or -, etc)
2. Apply rtrim( $filename, '.' ).
3. Append the extension.

Thanks! That makes sense to me. Seems similar to what sanitize_title_with_dashes()​ does:

1. It replaces all . characters with -:

   $title = str_replace( '.', '-', $title );
   

2. Then it replaces multiple instances with a single one:

   $title = preg_replace( '|-+|', '-', $title );
   

3. Then removes leading and trailing instances:

   $title = trim( $title, '-' );
   

It looks like sanitize_file_name() already does steps 2 and 3​, though perhaps too early, before the file name is separated from the extension.

So it might be a good idea to apply these steps to file names too.

[costdev]

Thanks @SergeyBiryukov!

After taking a closer look at sanitize_file_name(), this might be a little tricky.

Where steps 2 and 3 are performed, $filename may still include one/more extensions. So replacing consecutive . with - could result in filename-png.

---

I'm thinking if we replace consecutive . with a single ., that could work.

It means that ...file....name...png... would become .file.name.png., then the trim() you already pointed out would change this to file.name.png

Note: In reality, sanitize_file_name() actually produces file.name_.png - I believe the _ is added here, for some reason.

---

With the above regex, my test datasets pass, and the only failure I get for an existing test is:

• Tests_Functions::test_wp_unique_filename
• "Failed crazy file name"
• Expected: '12af34567890@..^_qwerty-fghjkl-zx.png'
• Actual: '12af34567890@.^_qwerty-fghjkl-zx.png'

Which I think we could live with. 😅

---

I'll push up a PR for review along with the above, and leave the existing test failure in place as a reference.

On some servers, consecutive periods in a filename can cause a 403 Forbidden response.

This change replaces consecutive periods with a single period.

Trac ticket: https://core.trac.wordpress.org/ticket/57242

Replaced with ​https://github.com/WordPress/wordpress-develop/pull/3723

@costdev The unit tests fail, which must be fixed. 

@mukeshpanchal27 See the PR's description:

This PR has an expected test failure in Tests_Functions::test_wp_unique_filename(). If this PR's approach is supported, I'll update the failing test to facilitate this change in Core.

I'm keeping the test failure so that reviewers of this approach see that an existing test will need to change to facilitate this.

Thanks @costdev

[audrasjb]

In 55209:

Media: Replace consecutive periods in sanitize_file_name().

On some servers, consecutive periods in a filename can cause a 403 Forbidden response.
This changeset replaces consecutive periods with a single period, and adds related unit tests.

Props ArtZ91, costdev, SergeyBiryukov, arthurshlain, mukesh27.
Fixes #57242.

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