SFTP::nlist returns integer entries for files names consisting of digits only

[Mellthas]

If I run $sftp->nlist() in a directory with a file (or directory) named 20210228, I get the following result from var_dump:

array(3) {
  [0]=>
  string(1) "."
  [1]=>
  string(2) ".."
  [2]=>
  int(20210228)
}

Shouldn't the file names always be string? The integer entries will result in warnings if I try something like this:

foreach ($sftp->nlist() as $path) {
  var_dump($sftp->is_dir($path));
}

PHP Warning: Trying to access array offset on value of type int in [...]/phpseclib/Net/SFTP.php on line 731

phpseclib version 2.0.30
Tested on PHP 7.4.15 and 8.0.2

[Mellthas]

After a quick glance into the code, this might be related to the entries being stored as array keys in the _list() method, and the fact that integer-like keys are being converted to int keys automatically by PHP.

[terrafrost]

Interesting. https://stackoverflow.com/a/35180513/569976 suggests that there is kinda a workaround for it but an imperfect one.

I suppose the parameter to is_dir could be explicitly cast to a string but I feel that that's somewhat imperfect, as well. It might be the best solution, however, for 1.0 / 2.0 / 3.0.

As for fixing this going forward... #1418 would do the trick. It wouldn't be an array - it'd be an object that looked like an array. But that's a BC breaking change that wouldn't be included in 1.0 / 2.0 / 3.0.

[Mellthas]

Are you concerned about changing the return value of nlist wrt. to types, because someone might rely on the current behavior (getting int for certain file names)? If not, changing

phpseclib/phpseclib/Net/SFTP.php

Line 983 in f2733c5

return $raw ? $contents : array_keys($contents);

to

return $raw ? $contents : array_map('strval', array_keys($contents));

might do the trick, as far as I can see; so casting the list to strings explicitly before returning. In the other case, the behavior is not surprising at least in the case that you are aware of PHP implicitly casting array keys to int, as an array is returned.

[terrafrost]

Are you concerned about changing the return value of nlist wrt. to types, because someone might rely on the current behavior (getting int for certain file names)?

No. getting int for certain file names is undocumented behavior. What I'm saying is a BC break is the idea of returning an ArrayObject instance instead of an array. Like if you're passing the output of nlist() into a method that, through the use of type hinting, is expecting an array... that'd be the bigger BC break of concern.

Anyway, your proposed solution seems reasonable. I'll try to implement it in the next few days!

[terrafrost]

The proposed code change is now live!:

a45ccba

Thanks!