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

BUG: SFTP gets in hard loop and send #1507

Closed
zabullet opened this issue Aug 5, 2020 · 4 comments
Closed

BUG: SFTP gets in hard loop and send #1507

zabullet opened this issue Aug 5, 2020 · 4 comments

Comments

@zabullet
Copy link

zabullet commented Aug 5, 2020

We noticed the issue appeared when upgrading from phpseclib 2.0.23 to 2.0.28 and delivering to Core SFTP server build 699. SFTP will deliver about 1.5MB and then get in an infinite loop while maxing out CPU and network.

We walked our phpseclib version to find out where the issue was introduced and this is what we found

2.0.23 - Good
2.0.24 - Bad
2.0.25 - Good
2.0.26 - Bad
2.0.27 - Bad
2.0.28 - Bad

I haven't had a chance to debug internally on the library, but if a maintainer would like to reach out I can provide an end-point to test against that demonstrates the behavior.

@zabullet
Copy link
Author

zabullet commented Aug 5, 2020

Similar or related to #1461

Did some more digging. Seems to get into a loop during SSH2::_send_channel_packet, in which the window_size_client_to_service drops to 0 and never expands and sits there spinning.

Given that 2.0.25...2.0.26 doesn't have too much in the way of a diff, the most obvious difference is the use of the NET_SFTP_UPLOAD_QUEUE_SIZE parameter.

I tried setting this to 32 seems to alleviate the problem and hence I'd assume calling _read_put_responses is important, but haven't digged deeper as to why yet.

@zabullet
Copy link
Author

zabullet commented Aug 6, 2020

@terrafrost Looking through the commit comments, I'm inclined to say this is a regression on the V2 path. While speed improvements are a noble direction to go in, I'm not convinced it should come at the cost of backwards compatibility. Would you consider setting this value to 32. For people wanting faster speeds, it should be up to them to increase values and test compatibility.

@terrafrost
Copy link
Member

Not having looked at the issue yet, myself, it does seem like you've done a fair amount of digging, yourself, so I do applaud you for that!

That said, the behavior should be the same that PuTTY (and therefore WinSCP) employ. Based on the name of your server (CoreSFTP) it's unclear if it offers SSH support but if WinSCP - an industry standard SFTP client - can't successfully upload files then I'd say that CoreSFTP is the package with the bigger problem.

You said the following in an earlier post:

I haven't had a chance to debug internally on the library, but if a maintainer would like to reach out I can provide an end-point to test against that demonstrates the behavior.

I'd like that. I'd like to see if WinSCP can, in fact, connect. If it can then what is it doing differently?

Worst case: I can add a special exception for CoreFTP like I did here:

c7d7b36

But I'd like to test it out first.

You can email the connection info to terrafrost@php.net

@terrafrost
Copy link
Member

Does 239bc63 fix the issue for you?

Your diagnostics have been really impressive and helpful btw - 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

No branches or pull requests

2 participants