Make WordPress Core

Opened 9 months ago

Closed 9 months ago

#59116 closed defect (bug) (fixed)

Integer Casting Error in WP_Site_Health::get_test_available_updates_disk_space() wordpress

Reported by: mathsgrinds's profile mathsgrinds Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.3.1 Priority: normal
Severity: normal Version: 6.3
Component: Site Health Keywords: has-patch fixed-major dev-reviewed commit
Focuses: Cc:

Description

In WP_Site_Health::get_test_available_updates_disk_space() when one has too much free space the variable $available_space becomes negative and so the condition $available_space < 20 * MB_IN_BYTES is incorrectly triggered and reports on the Site Health page that there is not enough free space. A fix for now is to go to wp-admin/includes/class-wp-site-health.php and on line 1947 change ? (int) $available_space to ? $available_space

See: https://wordpress.org/support/topic/available-disk-space-is-critically-low-less-than-20-mb-available-proceed-with/#post-16975478

Attachments (2)

59116.diff (591 bytes) - added by Presskopp 9 months ago.
59116.2.diff (1.7 KB) - added by SergeyBiryukov 9 months ago.

Download all attachments as: .zip

Change History (12)

#1 @rajinsharwar
9 months ago

Proposal: Should we check if the $available_space is 0 or not, and then make it 0, so that the negative value isn't passed through the test?

#2 @SergeyBiryukov
9 months ago

  • Component changed from General to Site Health

#3 @SergeyBiryukov
9 months ago

  • Milestone changed from Awaiting Review to 6.3.1
  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

@Presskopp
9 months ago

#4 @Presskopp
9 months ago

The patch checks the maximum possible int value, and if the available space is larger than this, it is simply set to the maximum value (because that is enough and it doesn't matter how large it is exactly).

#5 @SergeyBiryukov
9 months ago

  • Keywords has-patch added; needs-patch removed

Thanks for the patch!

I think we can avoid the type casting altogether and optimize the logic a bit to reduce the number of checks in certain scenarios, e.g. skip further checks if $available_space is false. See 59116.2.diff.

#6 @SergeyBiryukov
9 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 56401:

Site Health: Correct the check for disk space available to safely perform updates.

If the available disk space exceeds the PHP_INT_MAX value, i.e. a 32-bit PHP version is in use with more than 2 GB free, the type casting to (int) could cause an overflow, and the Site Health test would then erroneously report that there is not enough free space.

This commit removes the unnecessary type casting and uses the result from disk_free_space() directly.

Includes optimizing the logic to skip further checks if the available disk space could not be determined.

Follow-up to [55720].

Props mathsgrinds, Presskopp, rajinsharwar, SergeyBiryukov.
Fixes #59116.

#7 @SergeyBiryukov
9 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 6.3.1 consideration.

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


9 months ago

#9 @audrasjb
9 months ago

  • Keywords dev-reviewed commit added

As per today's bug scrub, marking this as dev-reviewed.
Props @azaozz for the review.

#10 @audrasjb
9 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 56428:

Site Health: Correct the check for disk space available to safely perform updates.

If the available disk space exceeds the PHP_INT_MAX value, i.e. a 32-bit PHP version is in use with more than 2 GB free, the type casting to (int) could cause
an overflow, and the Site Health test would then erroneously report that there is not enough free space.

This commit removes the unnecessary type casting and uses the result from disk_free_space() directly.

Includes optimizing the logic to skip further checks if the available disk space could not be determined.

Follow-up to [55720].

Props mathsgrinds, Presskopp, rajinsharwar, SergeyBiryukov.
Reviewed by azaozz, audrasjb.
Merges [56401] to the 6.3 branch.
Fixes #59116.

Note: See TracTickets for help on using tickets.