[wp-trac] [WordPress Trac] #48689: PHP warnings after updating to WP 5.3: ftp_nlist() and ftp_pwd() expect missing parameters

[WordPress Trac]

#48689: PHP warnings after updating to WP 5.3: ftp_nlist() and ftp_pwd() expect
missing parameters
-------------------------------------------------+-------------------------
 Reporter:  Hinjiriyo                            |       Owner:
                                                 |  adamsilverstein
     Type:  defect (bug)                         |      Status:  assigned
 Priority:  normal                               |   Milestone:  6.4
Component:  Filesystem API                       |     Version:  5.3
 Severity:  minor                                |  Resolution:
 Keywords:  php80 needs-unit-tests has-patch     |     Focuses:  php-
  needs-testing 2nd-opinion                      |  compatibility
-------------------------------------------------+-------------------------
Changes (by hellofromTonya):

 * focuses:   => php-compatibility
 * milestone:  Future Release => 6.4


Comment:

 >The issue arrises because we pass the file link to several ftp functions
 without first validating it. In certain conditions (for example, when
 WordPress lacks the correct file permissions), the file link is `null`.

 [attachment:"48689.diff"] adds property validation to ''silently'' bail
 out early. But the `$link` property is documented to always be a resource.
 This ticket identifies scenarios where the file link may fail.

 I'm thinking about:
 * Should the file link property be documented as `resource|null`? Is
 `null` a valid value?
 * Where these "certain conditions" first happen, is there error logging to
 alert developers why it failed?
 * Should a failure stop the progression of the work in the filesystem,
 meaning the code should not get to the methods where the warning / fatal
 are thrown?
 * If `null` is a valid value for the file link property, then protecting
 the methods that act upon it makes sense ''if'' those methods are
 reachable in the call stack.

 Silently bailing out without an early error can make debugging harder
 while potentially leaving users and developers wondering why their action
 failed.

 I'm marking this ticket as a PHP 8.0 `php-compatibility` issue and pulling
 it into the 6.4 cycle to hopefully move it forward.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/48689#comment:25>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform