WordPress.org

Make WordPress Core

Opened 6 months ago

Last modified 2 weeks ago

#52241 accepted defect (bug)

Infinite loop in clean_dirsize_cache()

Reported by: raubvogel Owned by: SergeyBiryukov
Milestone: 5.9 Priority: normal
Severity: major Version: 5.6
Component: Filesystem API Keywords: needs-unit-tests has-patch needs-testing
Focuses: multisite Cc:

Description

Since WP 5.6 there exsists

<?php
function clean_dirsize_cache( $path ){}

at functions.php:7725.
There we have

<?php
        while ( DIRECTORY_SEPARATOR !== $path && '.' !== $path && '..' !== $path ) {
                $path = dirname( $path );
                unset( $directory_cache[ $path ] );
        }

at functions.php:7735 which loops „forever“ if $path is something like c:\dir\subdir on Windows.

<?php
$path = dirname( $path );

returns the parent path in each step and gets „stuck“ at c:\.

Attachments (4)

s1.png (126.0 KB) - added by raubvogel 5 months ago.
Debug Screenshot 1
s2.png (121.9 KB) - added by raubvogel 5 months ago.
Debug Screenshot 1
s3.png (118.2 KB) - added by raubvogel 5 months ago.
Debug Screenshot 3
Fixed_infinite_loop_on_Windows_systems_.patch (749 bytes) - added by raubvogel 5 months ago.

Download all attachments as: .zip

Change History (20)

#1 in reply to: ↑ description @raubvogel
6 months ago

Btw. this breaks the media upload in my WordPress because media upload calls clean_dirsize_cache().

#2 @SergeyBiryukov
6 months ago

  • Component changed from General to Filesystem API
  • Focuses multisite added
  • Keywords needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to 5.6.1
  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


5 months ago

#4 @audrasjb
5 months ago

  • Milestone changed from 5.6.1 to Future Release

As per today's 5.6.1 bug scrub, moving this ticket to Future release pending further informations/context on the issue/bug (and a patch/unit tests).

@raubvogel
5 months ago

Debug Screenshot 1

@raubvogel
5 months ago

Debug Screenshot 1

@raubvogel
5 months ago

Debug Screenshot 3

#5 @raubvogel
5 months ago

  • Keywords has-patch added; needs-patch removed

#6 @ocean90
5 months ago

#52292 was marked as a duplicate.

#7 @codezen8
4 months ago

Is there a schedule for this patch? I can confirm that media uploads are broken in WordPress 5.6.x for multi-site running on a Windows server. I have tested the patch above and can also confirm that with that line in place media uploads work as expected.

#8 @sjlevy
4 months ago

To comment also in support of this patch: it appears to work fine and needed to be re-applied after the last WP update.

I encountered the infinite loop issue on 3 of my windows server-based multisites a couple of months ago-- located the offending line of code, and then subsequently found this ticket. Thank you

#9 @drosmog
3 months ago

Bug continues to affect IIS in WordPress 5.7. Line changed to 7813 in functions.php.

#10 @teachlynx
3 months ago

Same as @drosmog, bug continues to affect IIS 10 in WordPress 5.7. Line changed to 7813 in functions.php.

#11 @SergeyBiryukov
3 months ago

  • Milestone changed from Future Release to 5.8

#12 @EkoJr
3 months ago

May not be the most elegant solution, but it is a small simple change, and I'm a little surprised this ticket wouldn't have a high priority & got postponed to a future subversion considering it affects IIS servers and would appear to be a blocker bug for media.

Basically, the modified code compares the $last_path with the current $path, and if the $path is the same as it previously was in the previous iteration, it will then break out of the loop.

Original Code (WP 5.7)
wp-includes/functions.php:7812 > function clean_dirsize_cache( $path )

<?php
while ( DIRECTORY_SEPARATOR !== $path && '.' !== $path && '..' !== $path ) {
        $path = dirname( $path );
        unset( $directory_cache[ $path ] );
}

Modified Code

<?php
$last_path = '';
while ( DIRECTORY_SEPARATOR !== $path && '.' !== $path && '..' !== $path && $last_path !== $path ) {
        $last_path = $path;
        $path      = dirname( $path );
        unset( $directory_cache[ $path ] );
}

It's more of a conceptual solution, and someone may have a better answer. So, I am a bit hesitant creating a commit for this (plus, it's been awhile since I touched SVN).

#13 @bartoszgrzesik
2 months ago

I have problem with this bug. @EkoJr resolve is good to me.

#14 follow-ups: @hellofromTonya
2 weeks ago

  • Keywords needs-testing added
  • Milestone changed from 5.8 to 5.9

Today is 5.8 Beta 1. This one needs tests and testing. Punting to 5.9 to continue progress towards resolution.

#15 in reply to: ↑ 14 @teachlynx
2 weeks ago

While there is probably a lot of things to do to push out 5.8, I am a bit disappointed that for a bug like this, which has a fix already checked by many users on this page, we have yet to wait for another release because this one "needs testing". For people like me who host their WP sites on IIS 10, we need to manually fix each of our multisite installs, so this is not hell, but it is clearly a waist of time.

#16 in reply to: ↑ 14 @codezen8
2 weeks ago

Replying to hellofromTonya:

Today is 5.8 Beta 1. This one needs tests and testing. Punting to 5.9 to continue progress towards resolution.

This is ridiculous that core WordPress files continue to need to be modified in a multisite environment because of one line of code that multiple people have tested.

What is the process that needs to be done here? I know I was not aware that this needed further steps in order to be included in 5.8. This is a critical bug that disables key functionality in a multisite environment and there have been many releases since it was identified.

Note: See TracTickets for help on using tickets.