Opened 3 years ago
Last modified 3 years ago
#55688 new defect (bug)
Update size function in WP_Filesystem_Direct
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 2.5 |
Component: | Filesystem API | Keywords: | has-patch close dev-feedback |
Focuses: | Cc: |
Change History (3)
This ticket was mentioned in PR #2685 on WordPress/wordpress-develop by mukeshpanchal27.
3 years ago
#1
#2
@
3 years ago
Thanks for the PR!
This change has backward compatibility concerns, as the method would no longer return false
on failure, and it would not be possible to tell the difference between a file of 0 bytes or an error reading the file size. That might seem acceptable for media functions, for which wp_filesize()
was introduced, but not for the WP_Filesystem_*
classes, which are heavily used for updating plugins, themes, and WordPress core itself.
So I'm not sure this should be changed at all, especially in 6.0, which is only two weeks away, and only regressions are addressed at this point.
Frankly, I'm not a fan of the type casting to int
in wp_filesize()
and would prefer for it to mirror the native PHP function, with additional filters. However, it looks like it was suggested in comment:8:ticket:49412 mostly for ease of use where the difference between a zero size and an error doesn't matter that much, and I can see that point as well.
#3
@
3 years ago
- Component changed from Media to Filesystem API
- Keywords close dev-feedback added
- Milestone changed from 6.0 to Awaiting Review
- Version changed from trunk to 2.5
As this isn't a regression introduced during the 6.0 cycle, I'm removing this from the milestone.
As Sergey has said, the PR introduces a BC break, as the method can no longer return false
, and it's in a mission-critical component.
This change would also make it possible to filter the result via the 'pre_wp_filesize' and 'wp_filesize' filters, so this could end up breaking BC in a much larger way than removing the false
return.
I'm adding close
as I think this is too risky.
Adding dev-feedback
to see what others think about this. If there's a desire to have this method call wp_filesize()
, it needs extremely careful consideration. For BC, the checks to cause a false
return would need to be run before calling wp_filesize()
.
Trac ticket: https://core.trac.wordpress.org/ticket/55688