Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stream wrapper does not work on PHP 7.4.15 or 8.0.2 #1617

Closed
stingray-11 opened this issue Feb 21, 2021 · 6 comments
Closed

stream wrapper does not work on PHP 7.4.15 or 8.0.2 #1617

stingray-11 opened this issue Feb 21, 2021 · 6 comments

Comments

@stingray-11
Copy link
Contributor

I'm actually not sure if this should be considered a PHP bug, or a phpseclib bug.

The phpseclib stream wrapper uses the magic __call function to do logging, but in the PHP versions I've tested, __call is never actually called in the stream wrapper in most cases (possibly never), meaning the stream wrapper does not work at all. I started by debugging mkdir - I never saw the call get to _mkdir() or to __call(), but when I added the actual mkdir() function to Stream.php, it worked.

By replacing "private function _" with "public function ", and fixing a few conflicting method signatures I was able to get the stream wrapper working. I'm not sure if this is the direction you would want to go for fixing this, since it breaks the intended logging.

@terrafrost
Copy link
Member

I tried it out and it worked just fine for me on PHP 8.0.2.

For good measure I took the sample stream wrapper at https://www.php.net/manual/en/stream.streamwrapper.example-1.php and modified all the method names to be prepended with _'s, like how phpseclib does it. I then added the same __call() method that phpseclib has and it worked without issue on all the PHP versions 3v4l.org tests against:

https://3v4l.org/QBBel

I mean, I don't have a problem removing the __call, especially if it's breaking things in a duplicatable way, but I'm not able to duplicate it...

@stingray-11
Copy link
Contributor Author

Interesting. I'll have to do some more digging...

@stingray-11
Copy link
Contributor Author

stingray-11 commented Feb 22, 2021

Update - it seems like at least some of the stream wrapper is working for me. I tried reading a file with the original stream wrapper and it worked fine. Reading a directory also works fine.

But mkdir (what I started with) still does not work. The mkdir() call on my end is returning false, and when I add echo "phpseclib call $name\n"; to the beginning of __call() in phpseclib, all I see is phpseclib call url_stat twice. If I add a _mkdir() to the PHP demo website you linked it does appear to work. So not sure yet what the issue is...

I added some debug to _url_stat() - when creating a folder "sftp://{$sftp}/home/test" the first call to url_stat is to /home which returns a stat structure, and the second call is to /home/test which returns false (because it doesn't exist). After that there is never a mkdir.

I've tried both 7.4 and 8.0. As before, adding public function mkdir to Stream.php works (though I still don't see a mkdir in __call() which leads to me to believe this is a PHP issue). I also tried Ubuntu's PHP 7.4.9 package rather than ondrej's 7.4.15 - same issue. Also tried a clean php.ini.

@stingray-11
Copy link
Contributor Author

stingray-11 commented Feb 22, 2021

Okay now I'm getting somewhere... while _mkdir is never called in Stream.php, the base mkdir() in SFTP.php is getting called.

Perhaps when there is a method in the base class that matches, PHP never calls __call() on the child class?

Indeed that seems to be the problem, as this example shows - https://3v4l.org/PFpvi

PHP's documentation says "__call() is triggered when invoking inaccessible methods in an object context. " so it would make sense that when it finds a public mkdir() in SFTP.php, it would not bother with __call on Stream.php.

Unfortunately it seems like this will probably require an API change to fix. Or maybe Stream doesn't need to inherit from SFTP?

@terrafrost
Copy link
Member

Ooh interesting. That makes sense about mkdir.

Try this:

eeabad1

@stingray-11
Copy link
Contributor Author

Works! Looking at the code I would guess rmdir/rename were also broken, both work now. I also tried fread/fwrite/fseek/ftruncate/unlink/touch, all work fine. Glad the fix was that simple - thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants