1
Vote

FTP Data Socket Connection Problem (With Solution)

description

When using HigLabo.Net.Ftp.FtpClient (4.0), and opening a passive data socket to the remote ftp server, the FTP library parses the IP address and port from the FTP server response to the pasv command. The client uses SocketClient to connect to the parsed IP and port, but SocketClient.SetSocket() does not try to interpret the string address as an IP address - only as a hostname. Hostname lookups for the IP address failed (at least on my machine) so changing the function to the following allows for an IP address to be specified by the FTP client (among other clients) with minimal impact.

Lines 114 - 139 of SocketClient.Net4.cs:
    protected void SetSocket()
    {
        Socket tc = null;
        IPHostEntry hostEntry = null;

        IPAddress parsedAddress;
        if (IPAddress.TryParse(this.ServerName, out parsedAddress))
        {
            tc = this.TryGetSocket(parsedAddress);
        }
        if (tc == null)
        {
            //サーバー名からIPアドレスのリストを取得します。
            hostEntry = this.GetHostEntry();
            //有効なIPアドレスかどうか判別し、有効なIPアドレスにセットされたソケットを取得します。
            if (hostEntry != null)
            {
                foreach (IPAddress address in hostEntry.AddressList)
                {
                    tc = this.TryGetSocket(address);
                    if (tc != null) { break; }
                }
            }
        }
        this.Socket = tc;
    }

comments

charles_culver wrote Jan 8, 2015 at 8:11 PM

Related, I also have a fix for downloading files from an FTP server. It seems that the SocketClient believes that if it hasn't filled up its buffer, the server is finished sending data. This is not the case; data can arrive in any fashion. In fact, in the case of the FTP protocol, the server informs the client of the number of bytes it is going to be sending but this library ignores that information and simply closes the data connection when it encounters fewer than 8192 bytes on any read. This is bad. My only problem is that I have solved it for my specific case - not the general case. Responses among different FTP servers may vary, and this may require some more in-depth client-side parsing. If this can be fixed officially in this library, that would be great. The cleanest solution, in my opinion, would be to derive a new class from SocketClient for data connections.

higty wrote Jan 12, 2015 at 11:05 PM

Hi.

Thank you for your solution about SetSocket method.
I will fix it with it.

And I want to know your solution of the problem about reading data from server if you have.
I'll check and rewrite internal logic of reading data from server.

sincerely.

charles_culver wrote Aug 10, 2015 at 4:39 PM

Higty,

Sorry I haven't gotten back to you on this issue in so long. I was refactoring some code and came across the above described issue and I thought I should share with you the part that I didn't before. I see that you incorporated my above code - thank you for that.

The problem lies in HigLabo.Net.Ftp/Core/FtpClient.cs around line 320 in the DownloadFile(string, string) function. At line 320, I inserted the following code segment:
int idx = rs.Text.LastIndexOf("(");
int idx2 = rs.Text.LastIndexOf("bytes");
var bytestxt = rs.Text.Substring(idx + 1, idx2 - idx - 1);
int bytes;
int? bytesFinal = null;
if(int.TryParse(bytestxt, out bytes))
    bytesFinal = bytes;

ConnectDataSocket(); // Old line 320
As I mentioned in my comment in January, this is for the specific FTP server I'm working with in my application. Since other users' mileage may vary, it may be best to replace this with a Func<string, int?> that gets passed into DownloadFile() and defaults to null (e.g. public Int64 DownloadFile(string remoteFilePath, string localFilePath, Func<string, Int32?> = null) { }) so the user can parse the file size themselves. I would not expect this to work with every FTP server out there - it's very rudimentary.

Further down at the present line 338, I replaced that line with the following:
this._DataSocket.GetResponseStream(stm, bytesFinal);
You'll note that part of the code I changed refers to a function that doesn't yet exist in SocktClient.Net4.cs: GetResponseStream(Stream, int?). It's simply a modification of the existing function with a default value of null. The int? gets passed down to the underlying DataReceiveContext. I'm including that here:
public void GetResponseStream(Stream stream, int? RequiredBytes = null)
{
    this.GetResponseStream(new DataReceiveContext(stream, this.ResponseEncoding, RequiredBytes));
}
As for the DataReceiveContext constructor, that is due to a few changes I made in that class. I added a private Int32? field named _TotalBytesToRead that defaults to null in each constructor as well as a private Int32 field named _ReadBytes that keeps track of how much has already been read from the stream. These two prevent the connection from closing before all expected (if any) bytes have been read.

So on to the code:
private Int32 _ReadBytes = 0;
private Int32? _TotalBytesToRead;

public DataReceiveContext(Encoding encoding, Int32? bytesToRead = null)
    : base(new MemoryStream(), encoding)
{
    this._TotalBytesToRead = bytesToRead;
}

public DataReceiveContext(Stream stream, Encoding encoding, Int32? bytesToRead = null)
    : base(stream, encoding)
{
    this._TotalBytesToRead = bytesToRead;
}

public Boolean ReadBuffer(Int32 size)
{
    if (size == 0) { return false; }
    var bl = this.ParseBuffer(size);
    this._ReadCount += 1;
    this._ReadBytes += size;
    if (this._ReadCount > 1000000) { throw new SocketClientException("Too much read count.Perhaps parser could not parse correctly."); }
    return bl || MoreToRead;
}

protected bool MoreToRead
{
    get
    {
        if (_TotalBytesToRead == null)
            return false;

        return _ReadBytes < _TotalBytesToRead.Value;
    }
}
Finally, back to FtpClient.Net4.cs. Instead of this.Disconnect(); at the present line 349, I changed it to CloseDataSocket(); This allows the user to download multiple files without reconnecting, but is surely an implementation-specific choice that you may want to leave in place for personal reasons. I did the same thing in the UploadFile() method for consistency on my side.

Another convenience-related support function I added was similar to the one I just described changing: downloading a file directly to a stream for in-memory processing. I add that code here in the hopes that it is included as well.
public Int64 DownloadFile(String remoteFilePath, Stream stm)
{
    FtpCommandResult rs = null;
    Int64 size = 0;

    if (this.EnsureOpen() == FtpConnectionState.Disconnected) { throw new FtpClientException("Connection must be opened"); }
    if (this.State == FtpConnectionState.Connected)
    {
        if (this.Authenticate() == false) { throw new FtpClientException("Authenticate failed"); }
    }
    this.OpenDataSocket();

    rs = this.ExecuteType(this.DataType);

    OpenDataSocket();

    rs = this.ExecuteRetr(remoteFilePath);
    switch (rs.StatusCode)
    {
        case FtpCommandResultCode.DataConnectionAlreadyOpenTransferStarting: break;
        case FtpCommandResultCode.FileStatusOkayAboutToOpenDataConnection: break;
        default: throw new FtpClientException(rs);
    }

    int idx = rs.Text.LastIndexOf("(");
    int idx2 = rs.Text.LastIndexOf("bytes");
    var bytestxt = rs.Text.Substring(idx + 1, idx2 - idx - 1);
    int bytes;
    int? bytesFinal = null;
    if (int.TryParse(bytestxt, out bytes))
        bytesFinal = bytes;

    ConnectDataSocket();
    
    this._DataSocket.GetResponseStream(stm, bytesFinal);
    size = stm.Length;
    stm.Seek(0, SeekOrigin.Begin);

    CloseDataSocket();
    var response = this.GetResponse();

    return size;
}
Hope this helps improve your library. Good luck.

charles_culver wrote Aug 10, 2015 at 4:40 PM

That should read FtpClient.cs, not FtpClient.Net4.cs.

higty wrote Aug 20, 2015 at 2:40 AM

Hi.

Thank you for your great contribution!
I'll review and merge your patch to my library.
Please wait for few days.