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

OpenRead with EnableThreadSafeDataConnections = true does ASCII when it shouldn't #428

Closed
ts678 opened this issue Jul 10, 2019 · 2 comments

Comments

Projects
None yet
2 participants
@ts678
Copy link

commented Jul 10, 2019

FTP OS: Linux

FTP Server: inetutils-ftpd

Computer OS: Windows

Download corruption was originally noticed as size expansion by about 1/256 of original size. Further investigation found that the OpenRead which should have defaulted to binary was in ASCII somehow.

The symptom of the undesired TYPE A is downloaded file corruption of binary files when the TYPE A causes translations, for example from a Linux FTP server where end-of-line is \n. Windows FTP servers probably just send CR LF unchanged. Windows FileZilla Server was used to watch control connection.

While setting EnableThreadSafeDataConnections = false stops the corruption, it's not totally clear that it's safe for the application, so code change is desired. Primary cause of the problem appears to be Fix download file data type bug. #202 where a commit moved SetDataType to after the CloneConnection for unknown reasons, unless it was to test that the associated CloneConnection enhancement worked.

The suspected cause of the TYPE A appears to be because the cloned connection is treated as ASCII because the CurrentDataType was never set up to anything, and ASCII is the enum result for the zero.

The reason TYPE A actually got sent is thought to be the file presence check of GetFileSize returning from temporary binary mode (to get SIZE) to (the uninitialized-zero-but-ASCII) CurrentDataType here.

A fix might be done (and was patched and tested in a debugger) using one or both of these changes:

  1. Move SetDataType back to after CloneConnection (as all other methods do) to finish setup.

  2. Add CurrentDataType to CloneConnection, so an oddly early SetDataType gets propagated.

There's more information available, but that's the summary. Thanks for producing FluentFTP.
Looking forward to a code change. I can do a pull request if need be but I can't source build.

Logs :

Rough test is basically the OpenRead.cs code example doing an OpenRead in an added Main()

# OpenRead("binary_content", Binary, 0)

# Connect()
Status:   Connecting to ***:21
Response: 220 Please visit https://filezilla-project.org/
Response: 220-FileZilla Server 0.9.60 beta
Response: 220-written by Tim Kosse (tim.kosse@filezilla-project.org)
Status:   Detected FTP server: FileZilla
Command:  USER ***
Response: 331 Password required for username
Command:  PASS ***
Response: 230 Logged on
Command:  FEAT
Response: 211 End
Response: 211-Features:
Response: MDTM
Response: REST STREAM
Response: SIZE
Response: MLST type*;size*;modify*;
Response: MLSD
Response: UTF8
Response: CLNT
Response: MFMT
Response: EPSV
Response: EPRT
Status:   Text encoding: System.Text.UTF8Encoding
Command:  OPTS UTF8 ON
Response: 202 UTF8 mode is always enabled. No need to send this command.
Command:  SYST
Response: 215 UNIX emulated by FileZilla
Status:   Auto-detected UNIX listing parser
Command:  TYPE I
Response: 200 Type set to I

# Connect()
Status:   Connecting to ***:21
Response: 220 Please visit https://filezilla-project.org/
Response: 220-FileZilla Server 0.9.60 beta
Response: 220-written by Tim Kosse (tim.kosse@filezilla-project.org)
Status:   Detected FTP server: FileZilla
Command:  USER ***
Response: 331 Password required for username
Command:  PASS ***
Response: 230 Logged on
Status:   Text encoding: System.Text.UTF8Encoding
Command:  OPTS UTF8 ON
Response: 202 UTF8 mode is always enabled. No need to send this command.
Command:  SYST
Response: 215 UNIX emulated by FileZilla
Status:   Auto-detected UNIX listing parser

# GetWorkingDirectory()
Command:  PWD
Response: 257 "/" is current directory.

# SetWorkingDirectory("/")
Command:  CWD /
Response: 250 CWD successful. "/" is current directory.

# GetFileSize("binary_content")
Command:  TYPE I
Response: 200 Type set to I
Command:  SIZE binary_content
Response: 213 0
Command:  TYPE A
Response: 200 Type set to A

# OpenPassiveDataStream(AutoPassive, "RETR binary_content", 0)
Command:  EPSV
Response: 229 Entering Extended Passive Mode (|||55130|)
Status:   Connecting to ***:55130
Command:  RETR binary_content
Response: 150 Opening data channel for file download from server of "/binary_content"

Status:   Disposing FtpSocketStream...
Status:   Disposing FtpSocketStream...

# Dispose()
Status:   Disposing FtpClient object...
Command:  QUIT
Response: 221 Goodbye
Status:   Disposing FtpSocketStream...
Status:   Disposing FtpSocketStream...

# Dispose()
Status:   Disposing FtpClient object...
Status:   There is stale data on the socket, maybe our connection timed out or you did not call GetReply(). Re-connecting...
Status:   The stale data was: 226 Successfully transferred "/binary_content"
Status:   Disposing FtpSocketStream...
Status:   Not sending QUIT because the connection has already been closed.
Status:   Disposing FtpSocketStream...
Status:   Disposing FtpSocketStream...

@robinrodricks

This comment has been minimized.

Copy link
Owner

commented Jul 12, 2019

Please check this..

https://we.tl/t-b3r6kOO0BP

@robinrodricks

This comment has been minimized.

Copy link
Owner

commented Jul 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.