[c#] FTP Client - Invalid file/folder List from the FTP Server

Question

Hi, I'm developing my own FTP Client in C#, and - what's curious - sometimes (it's started being more then often) I receive invalid (broken) files/folders list from the FTP Server, it looks like that:

drwxr--r--   1 user     group          0 Jul 10 08:53 .\r
drwxr--r--   1 user     group          0 Jun 19 10:47 NetBeansProjects\r
drwxr--r--   1 user     group          0 May 28 22:07 NFS Most Wanted\r
0 May 28 18:57 My Skype Content\r
drwxr--r--   1 user     group      drwxr-

What's curious - sometimes that listing above is correct:

drwxr--r--   1 user     group          0 Jul 10 08:53 .\r
drwxr--r--   1 user     group          0 Jun 19 10:47 NetBeansProjects\r
drwxr--r--   1 user     group          0 May 28 22:07 NFS Most Wanted\r
drwxr--r--   1 user     group          0 May 28 18:57 My Skype Content\r

The method which receives the files/folders list from the FTP Server is: (I also attached the other methods which are connected with that method getRemoteFolders())

    public void getRemoteFolders()
    {
        string fileOrFolder;
        string folderList="";

        folderList = Encoding.ASCII.GetString(sendPassiveFTPcmd("LIST\r\n"));
        filesAndFolders = folderList.Split("\n".ToCharArray());

        [...] //the rest of the code, not important
    }

    public byte[] sendPassiveFTPcmd(string cmd)
    {
        byte[] szData;
        System.Collections.ArrayList al = new ArrayList();
        byte[] RecvBytes = new byte[Byte.MaxValue];
        Int32 bytes;
        Int32 totalLength = 0;
        szData = System.Text.Encoding.ASCII.GetBytes(cmd.ToCharArray());

        NetworkStream passiveConnection;
        passiveConnection = createPassiveConnection();

        tbStatus.Text += "\r\nSent:" + cmd;
        StreamReader commandStream = new StreamReader(NetStrm);
        NetStrm.Write(szData, 0, szData.Length);
        while (true)
        {
            bytes = passiveConnection.Read(RecvBytes, 0, RecvBytes.Length);
            if (bytes <= 0) break;
            totalLength += bytes;
            al.AddRange(RecvBytes);
        }
        al = al.GetRange(0, totalLength);
        tbStatus.Text += "\r\nRcvd:" + commandStream.ReadLine(); // 125
        tbStatus.Text += "\r\nRcvd:" + commandStream.ReadLine(); // 226
        return (byte[])al.ToArray((new byte()).GetType());
    }

    private NetworkStream createPassiveConnection()
    {
        string[] commaSeperatedValues;
        int highByte = 0;
        int lowByte = 0;
        int passivePort = 0;
        string response = "";
        response = sendFTPcmd("PASV\r\n");

        commaSeperatedValues = response.Split(",".ToCharArray());
        highByte = Convert.ToInt16(commaSeperatedValues[4]) * 256;
        commaSeperatedValues[5] =
        commaSeperatedValues[5].Substring(0,
        commaSeperatedValues[5].IndexOf(")"));
        lowByte = Convert.ToInt16(commaSeperatedValues[5]);
        passivePort = lowByte + highByte;
        TcpClient clientSocket = new TcpClient(server, passivePort);
        NetworkStream pasvStrm = clientSocket.GetStream();
        return pasvStrm;
    }

    public string sendFTPcmd(string cmd)
    {
        byte[] szData;
        string returnedData = "";
        StreamReader RdStrm = new StreamReader(NetStrm);
        szData = Encoding.ASCII.GetBytes(cmd.ToCharArray());
        NetStrm.Write(szData, 0, szData.Length);
        tbStatus.Text += "\r\nSent:" + cmd;
        returnedData = RdStrm.ReadLine();
        tbStatus.Text += "\r\nRcvd:" + returnedData;
        return returnedData;
    }

Do anyone knows where the problem is ?

Answer 1

Well for one thing, you're mostly ignoring the value returned by Read - you're calling al.AddRange(RecvBytes) as if it was full of valid data - which it may well not be.

There's a much easier way of reading a byte array from a stream though - use a MemoryStream. For example:

public static byte[] ReadFully(Stream input)
{
    byte[] buffer = new byte[16*1024];
    using (MemoryStream ms = new MemoryStream())
    {
        int read;
        while ((read = input.Read(buffer, 0, buffer.Length)) > 0)
        {
            ms.Write(buffer, 0, read);
        }
        return ms.ToArray();
    }
}

(Are you using .NET 1.1, by the way? If so, it's really worth avoiding the non-generic collections...)

I notice you're also not closing the stream - bad idea. Use a using statement to close the stream, like this (names changed to be more .NET idiomatic too):

public byte[] SendPassiveFTPcmd(string cmd)
{
    using (Stream passiveConnection = CreatePassiveConnections())
    {
        byte[] commandData = Encoding.ASCII.GetBytes(cmd);
        NetStrm.Write(commandData, 0, commandData.Length);
        tbStatus.Text += "\r\nSent:" + cmd;

        byte[] data = ReadFully(passiveConnection);

        StreamReader commandStream = new StreamReader(NetStrm);
        tbStatus.Text += "\r\nRcvd:" + commandStream.ReadLine(); // 125
        tbStatus.Text += "\r\nRcvd:" + commandStream.ReadLine(); // 226
        return data;
    }
}

Also note that you're creating a new StreamReader each time from NetStrm. This is probably a bad idea - I would create one StreamReader and one StreamWriter both wrapping NetStrm, and then you don't need to deal with the binary data at all. Otherwise, the new StreamReaders may read more data than just the lines you've asked for, causing you to miss data next time.