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

Fixed FTP directory listing blocking forever #1086

Merged
merged 1 commit into from Jun 4, 2016

Conversation

Projects
None yet
3 participants
@binary1248
Member

binary1248 commented May 8, 2016

Fixes #1025.

In certain situations, both the first (150) and second (226) responses to NLST might be read from the command socket in a single call. getResponse() would return the 150 response successfully and discard anything else it read from the socket, this could include the full 226 response. When getResponse() would be called the second time, it would start a blocking TCP read on the socket that would never complete because the expected data was already read as part of the previous call.

Code and steps to reproduce are already given in #1025.

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini May 10, 2016

Member

I just had a quick look; the proposed changes seem fine.

There are a few places where we could add more error handling, such as on lines 421, 434 or 448, but those are probably very exotic edge cases so I'm fine if this gets merged as is.

Member

mantognini commented May 10, 2016

I just had a quick look; the proposed changes seem fine.

There are a few places where we could add more error handling, such as on lines 421, 434 or 448, but those are probably very exotic edge cases so I'm fine if this gets merged as is.

@eXpl0it3r eXpl0it3r added this to the 2.4 milestone May 10, 2016

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r May 16, 2016

Member

Couldn't reproduce the issue, both versions seem to work for me. Tested with FileZilla Server and Baby FTP and the SFML FTP example.

Member

eXpl0it3r commented May 16, 2016

Couldn't reproduce the issue, both versions seem to work for me. Tested with FileZilla Server and Baby FTP and the SFML FTP example.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 May 16, 2016

Member

You need both a really fast server and a really slow client computer and optimal network conditions. 😉

To simulate this scenario, throw in a sleep(100) before this line, like @jcowgill suggessted in #1025.

Member

binary1248 commented May 16, 2016

You need both a really fast server and a really slow client computer and optimal network conditions. 😉

To simulate this scenario, throw in a sleep(100) before this line, like @jcowgill suggessted in #1025.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r May 17, 2016

Member

With that I can indeed reproduce it and this PR fixes the bug.

Member

eXpl0it3r commented May 17, 2016

With that I can indeed reproduce it and this PR fixes the bug.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r May 24, 2016

Member

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

Member

eXpl0it3r commented May 24, 2016

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

Fixed FTP directory listing blocking forever if both expected respons…
…es are read from the command socket in a single call. (#1025)

@eXpl0it3r eXpl0it3r merged commit c15172e into master Jun 4, 2016

14 checks passed

debian-gcc-64 Build #141 done.
Details
freebsd-gcc-64 Build #141 done.
Details
osx-clang-el-capitan Build #21 done.
Details
static-analysis Build #141 done.
Details
windows-gcc-492-tdm-32 Build #26 done.
Details
windows-gcc-492-tdm-64 Build #26 done.
Details
windows-gcc-530-mingw-32 Build #26 done.
Details
windows-gcc-530-mingw-64 Build #26 done.
Details
windows-vc11-32 Build #142 done.
Details
windows-vc11-64 Build #142 done.
Details
windows-vc12-32 Build #142 done.
Details
windows-vc12-64 Build #140 done.
Details
windows-vc14-32 Build #141 done.
Details
windows-vc14-64 Build #143 done.
Details

@eXpl0it3r eXpl0it3r deleted the bugfix/ftp_response branch Jun 4, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment