SharpFTP - implementacja prostego servera (beta)

Cześć wszystkim,
Ostatnio coś sobie tam grzebałem i napisałem prosty serwer FTP. Póki co serwer implementuje podstawowe komendy wspomniane w tym dokumencie :

http://www.iana.org/assignments/ftp-commands-extensions/ftp-commands-extensions.xhtml

plus kilka innych. W przyszłości będę grzebał dalej ale już się da łączyć z klientem + wysyłać/odbierać, zmieniać nazwy itd.

Źródła oczywiście na githubie: https://github.com/NeuroXiq/SharpFTP

Tak więc zapraszam do oceny ;)

Zaktualizowałem trochę kod, poprawiałem miejsca o których pisał msm i dodałem kilka nowych rzeczy :)

Do kodu doszły 2 nowe klasy FileMutex i DirectoryMutex:
https://github.com/NeuroXiq/SharpFTP/tree/master/FileSystem
Tak w skrócie: istniał mały problem z edytowaniem/pobieranie jednego pliku przez kilku kilentów. Np. jeden chciałbym go usunąć, inny pobrać i pojawia się problem z dostępem. Powyższe klasy mają trywialną funkcję która umożliwia dostęp do konkretnego pliku tylko jednemu kilentowi (problem dotyczy także całych folderów stąd DirecotryMutex)

(załącznik: ftpmutex.png)

Małe zmiany także nastąpiły w FTPDynamicServerState:
https://github.com/NeuroXiq/SharpFTP/blob/master/Server/FTPDynamicServerState.cs
Teraz nie ma już tego całego ++port tylko system sam wybiera sobie port który jest wolny i otwiera go. Można także ustawić swoje własne port z których serwer ma korzystać (metoda GetPasvTcpListener w linku powyżej)

Zrobiłem też przy okazji taką małą refaktoryzację, powywalałem zbędne komentarze, niekiedy w dłuższych metodach podzieliłem kod na mniejsze metody itp.
Warto dodać, że dostęp do użytkowników/ścieżek dostępu odbywa się za pomocą klasy abstrakcyjnej (którą przyjmuje konstruktor klasy serwera). Jest to mała klasa prosta w implementacji, przykładowy jej kod (dla użytkowników: admin, anonymous):
https://github.com/NeuroXiq/SharpFTP/blob/master/Server/EXAMPLE_UserDataContext.cs

        public void RunSession()
        {
            protocolInterpreter.PrepareToNewClient();
            try
            {
                while (protocolInterpreter.ProcessNextCommand())
                {
                    if (!acceptedClient.Connected)
                        break;
                }
            }
            catch (System.Exception)
            {

            }
                        
        }

Sugeruję chociaż logować wyjątek. Oraz, jeśli nie używasz System.Exception, możesz użyć po prostu catch { }.

    public static class FTPDynamicServerState
    {
        public static IPAddress GetIp()
        {
            return address;
        }
        private static int port = 5000;
        private static IPAddress address = IPAddress.Loopback;

        public static int GetPasvPort()
        {
            return ++port;
        }

        public static void SetIp(IPAddress ipaddress)
        {
            address = ipaddress;
        }
    }

Uwaga - fakt jest faktem że FTP jest dramatycznie niebezpieczny (insecure) "by default", i czego by nie robić to będzie dziurawy. Ale jeśli specyfikacja zakłada że używamy losowego portu, to zazwyczaj ma do tego dobry powód. Generalnie implementacja tego w taki sposób jak tutaj to security vulnerability - czyli da się zrobić półpraktyczny atak w wyniku którego można wykraść dane innego użytkownika.

Losowy port poważnie będzie lepszy..

        private static string GetSystemType()
        {
            return "UNIX Type: L8";
        }

            int memoryToAllocate;
            if (int.TryParse(data[1], out memoryToAllocate))
            {
                MemoryToAllocate = memoryToAllocate;
                replySender.SendReply(200);
            }
            else
            {
                throw new Exception("incorrect parameter");
            }

Generalnie rzucanie Exception to zawsze zły pomysł. Zależnie od sytuacji lepiej wybrać konkretniejszy rodzaj wyjątku, na przykład ArgumentException czy coś.

Tutaj zamiast rzucać Exception już lepiej by sobie było darować w ogóle to if{} else{} -> można zrobić przecież po prostu MemoryToAllocate = int.Parse(data[1]) i będzie 6x krócej a efekt ten sam (wyjątek przy złym argumencie).

Analogiczne uwagi do wyjątków wszędzie indzie - new Exception() = zło

    [Flags]
    public enum FilePermission
    {
        Read,
        Write,
        ACCESS_DENIED
    }

Aż mi się przypomniało:
http://thedailywtf.com/articles/What_Is_Truth_0x3f_

:>.

        private string GetWindowsOriginDirectory(string windowsOriginDirectory)
        {
            //windows dir should have '\' at the end.
            string winDir = windowsOriginDirectory.Trim(' ','\\') + "\\";
            return winDir;
        }

Szczegół, ale jest coś takiego jak Path.Separator czy jakoś tak (zamiast hardcodowania ''). Szczegół 2, po prostu return windowsOriginDirectory.Trim(' ','\\') + "\\";.

Generalnie dużo lepszy projekt niż się spodziewałem, wygląda na całkiem porządny serwer. A myślałem że to będzie toy projekt z 200 linijkami kodu ;).
Kod nie jest idealny (nie czepiałem się już głupot typu zakomentowany kod w losowych miejscach, dziwne wcięcia, niepotrzebne zmienne, nierówne wcięcia, etc), ale trzyma poziom.

Ja z niego i tak nie skorzystam, bo FTP to protokół który powinien umrzeć i zostać zniszczony, ale generalnie kod jako kod ujdzie ;>.