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

access $disconnect_reasons using self instead of static #1953

Merged
merged 1 commit into from Oct 21, 2023

Conversation

MarkLittlewood
Copy link
Contributor

@MarkLittlewood MarkLittlewood commented Oct 18, 2023

Fatal error: Uncaught Error: Cannot access property phpseclib3\Net\SFTP::$disconnect_reasons was being received when login failed.
changing static:: to self:: fixed the issue

@terrafrost
Copy link
Member

I'm not able to reproduce the issue that this PR attempting to fix.

So you're modifying the filter() method of SSH2.php. I added the following right after the function definition (just before the switch statement) and then ran the PHP script:

print_r(static::$disconnect_reasons); exit;

I got no errors, not even after calling error_reporting(E_ALL); before initializing the SFTP() object and I'm running PHP 8.2.9. I mean, print_r(self::$disconnect_reasons) works, as well, but I'd like to know what, specifically, is causing the issue for you.

And, to be fair, altho the static keyword enables late static binding it's not of huge concern because $disconnect_reasons is private in SSH2.php.

@terrafrost
Copy link
Member

I tried it with PHP 8.3.0-dev, as well, and am still not able to reproduce the issue you described...

In both cases I did print_r(static::$disconnect_reasons) and print_r(self::$disconnect_reasons) and both worked just as well as one other.

@terrafrost
Copy link
Member

Maybe you can post the output of phpinfo()?

@MarkLittlewood
Copy link
Contributor Author

MarkLittlewood commented Oct 20, 2023

Unfortunatley I am still stuck on php v7.3.33 - some organisations take a long time to move versions.
That may be why I am getting the issue and you are not.

I realise that you can't be expected to support old versions of php, but as the change works perfectly well in php 8.3, I wonder if you could do me a solid and include it?

@terrafrost terrafrost merged commit eb04913 into phpseclib:3.0 Oct 21, 2023
34 of 37 checks passed
@terrafrost
Copy link
Member

I was able to reproduce this on 7.3.33 and, in light of that, went ahead and merged this!

Even if PHP 7.3.33 is old I "advertise" phpseclib 3.0 as working on PHP 5.6+ and I'm committed to that! The master branch (which will eventually become v4.0) is a different story but you're not using that version!

@MarkLittlewood
Copy link
Contributor Author

Fantastic thank you

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

Successfully merging this pull request may close these issues.

None yet

2 participants