From 606737f274968d63b1e47d4146586a9bae64f938 Mon Sep 17 00:00:00 2001 From: Joe <1015.5@gmx.de> Date: Mon, 6 Apr 2026 08:22:20 +0200 Subject: [PATCH 01/18] initial commit. passive-ftp-mode --- src/Commands/LIST.h | 2 +- src/Commands/MLSD.h | 2 +- src/Commands/NLST.h | 2 +- src/Commands/PASV.h | 39 ++++++++++++++++++++++++++++++ src/Commands/PORT.h | 10 +++++++- src/Commands/RETR.h | 2 +- src/Commands/STOR.h | 2 +- src/FTPCommand.h | 56 ++++++++++++++++++++++++++++++++++++++++--- src/FTPConnection.cpp | 22 +++++++++++------ src/FTPConnection.h | 6 +++-- 10 files changed, 125 insertions(+), 18 deletions(-) create mode 100644 src/Commands/PASV.h diff --git a/src/Commands/LIST.h b/src/Commands/LIST.h index d291011..2601fa0 100644 --- a/src/Commands/LIST.h +++ b/src/Commands/LIST.h @@ -8,7 +8,7 @@ class LIST : public FTPCommand { public: - explicit LIST(WiFiClient *const Client, FTPFilesystem *const Filesystem, IPAddress *DataAddress, int *DataPort) : FTPCommand("LIST", 1, Client, Filesystem, DataAddress, DataPort) { + explicit LIST(WiFiClient *const Client, FTPFilesystem *const Filesystem, IPAddress *DataAddress, int *DataPort, WiFiServer **PassiveServer = 0, bool *PassiveMode = 0) : FTPCommand("LIST", 1, Client, Filesystem, DataAddress, DataPort, PassiveServer, PassiveMode) { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { diff --git a/src/Commands/MLSD.h b/src/Commands/MLSD.h index a88045b..8eb748d 100644 --- a/src/Commands/MLSD.h +++ b/src/Commands/MLSD.h @@ -10,7 +10,7 @@ class MLSD : public FTPCommand { public: - explicit MLSD(WiFiClient *const Client, FTPFilesystem *const Filesystem, IPAddress *DataAddress, int *DataPort) : FTPCommand("MLSD", 1, Client, Filesystem, DataAddress, DataPort) { + explicit MLSD(WiFiClient *const Client, FTPFilesystem *const Filesystem, IPAddress *DataAddress, int *DataPort, WiFiServer **PassiveServer = 0, bool *PassiveMode = 0) : FTPCommand("MLSD", 1, Client, Filesystem, DataAddress, DataPort, PassiveServer, PassiveMode) { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { diff --git a/src/Commands/NLST.h b/src/Commands/NLST.h index c739604..ca35d95 100644 --- a/src/Commands/NLST.h +++ b/src/Commands/NLST.h @@ -8,7 +8,7 @@ class NLST : public FTPCommand { public: - explicit NLST(WiFiClient *const Client, FTPFilesystem *const Filesystem, IPAddress *DataAddress, int *DataPort) : FTPCommand("NLST", 1, Client, Filesystem, DataAddress, DataPort) { + explicit NLST(WiFiClient *const Client, FTPFilesystem *const Filesystem, IPAddress *DataAddress, int *DataPort, WiFiServer **PassiveServer = 0, bool *PassiveMode = 0) : FTPCommand("NLST", 1, Client, Filesystem, DataAddress, DataPort, PassiveServer, PassiveMode) { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { diff --git a/src/Commands/PASV.h b/src/Commands/PASV.h new file mode 100644 index 0000000..89a24d3 --- /dev/null +++ b/src/Commands/PASV.h @@ -0,0 +1,39 @@ +#ifndef PASV_H_ +#define PASV_H_ + +#include +#include +#include + +#include "../FTPCommand.h" +#include "../common.h" + +class PASV : public FTPCommand { +public: + explicit PASV(WiFiClient *const Client, IPAddress *DataAddress, int *DataPort, WiFiServer **PassiveServer = 0, bool *PassiveMode = 0) : FTPCommand("PASV", 0, Client, 0, DataAddress, DataPort, PassiveServer, PassiveMode) { + } + + void run(FTPPath &WorkDirectory, const std::vector &Line) override { + if (_PassiveServer != 0 && *_PassiveServer != 0) { + (*_PassiveServer)->stop(); + delete *_PassiveServer; + *_PassiveServer = 0; + } + if (_PassiveMode != 0) { + *_PassiveMode = true; + } + int port = 20000 + random(0, 1000); + *_DataPort = port; + if (_PassiveServer != 0) { + *_PassiveServer = new WiFiServer(port); + (*_PassiveServer)->begin(); + } + IPAddress localIP = WiFi.localIP(); + int p1 = port / 256; + int p2 = port % 256; + String response = "Entering Passive Mode (" + String(localIP[0]) + "," + String(localIP[1]) + "," + String(localIP[2]) + "," + String(localIP[3]) + "," + String(p1) + "," + String(p2) + ")"; + SendResponse(227, response); + } +}; + +#endif diff --git a/src/Commands/PORT.h b/src/Commands/PORT.h index 9851c1a..f047df1 100644 --- a/src/Commands/PORT.h +++ b/src/Commands/PORT.h @@ -8,10 +8,18 @@ class PORT : public FTPCommand { public: - explicit PORT(WiFiClient *const Client, IPAddress *DataAddress, int *DataPort) : FTPCommand("PORT", 1, Client, 0, DataAddress, DataPort) { + explicit PORT(WiFiClient *const Client, IPAddress *DataAddress, int *DataPort, WiFiServer **PassiveServer = 0, bool *PassiveMode = 0) : FTPCommand("PORT", 1, Client, 0, DataAddress, DataPort, PassiveServer, PassiveMode) { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { + if (_PassiveServer != 0 && *_PassiveServer != 0) { + (*_PassiveServer)->stop(); + delete *_PassiveServer; + *_PassiveServer = 0; + } + if (_PassiveMode != 0) { + *_PassiveMode = false; + } std::vector connection_details = Split>(Line[1], ','); for (int i = 0; i < 4; i++) { (*_DataAddress)[i] = connection_details[i].toInt(); diff --git a/src/Commands/RETR.h b/src/Commands/RETR.h index a156c7a..53075b4 100644 --- a/src/Commands/RETR.h +++ b/src/Commands/RETR.h @@ -10,7 +10,7 @@ class RETR : public FTPCommandTransfer { public: - explicit RETR(WiFiClient *const Client, FTPFilesystem *const Filesystem, IPAddress *DataAddress, int *DataPort) : FTPCommandTransfer("RETR", 1, Client, Filesystem, DataAddress, DataPort) { + explicit RETR(WiFiClient *const Client, FTPFilesystem *const Filesystem, IPAddress *DataAddress, int *DataPort, WiFiServer **PassiveServer = 0, bool *PassiveMode = 0) : FTPCommandTransfer("RETR", 1, Client, Filesystem, DataAddress, DataPort, PassiveServer, PassiveMode) { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { diff --git a/src/Commands/STOR.h b/src/Commands/STOR.h index 9b215d6..a89213b 100644 --- a/src/Commands/STOR.h +++ b/src/Commands/STOR.h @@ -10,7 +10,7 @@ class STOR : public FTPCommandTransfer { public: - explicit STOR(WiFiClient *const Client, FTPFilesystem *const Filesystem, IPAddress *DataAddress, int *DataPort) : FTPCommandTransfer("STOR", 1, Client, Filesystem, DataAddress, DataPort) { + explicit STOR(WiFiClient *const Client, FTPFilesystem *const Filesystem, IPAddress *DataAddress, int *DataPort, WiFiServer **PassiveServer = 0, bool *PassiveMode = 0) : FTPCommandTransfer("STOR", 1, Client, Filesystem, DataAddress, DataPort, PassiveServer, PassiveMode) { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { diff --git a/src/FTPCommand.h b/src/FTPCommand.h index 0b5a088..c642954 100644 --- a/src/FTPCommand.h +++ b/src/FTPCommand.h @@ -3,6 +3,7 @@ #include #include +#include #include #include "FTPFilesystem.h" @@ -10,9 +11,14 @@ class FTPCommand { public: - FTPCommand(String Name, int ExpectedArgumentCnt, WiFiClient *const Client, FTPFilesystem *const Filesystem = 0, IPAddress *DataAddress = 0, int *DataPort = 0) : _Name(Name), _ExpectedArgumentCnt(ExpectedArgumentCnt), _Filesystem(Filesystem), _DataAddress(DataAddress), _DataPort(DataPort), _Client(Client), _DataConnection(0) { + FTPCommand(String Name, int ExpectedArgumentCnt, WiFiClient *const Client, FTPFilesystem *const Filesystem = 0, IPAddress *DataAddress = 0, int *DataPort = 0, WiFiServer **PassiveServer = 0, bool *PassiveMode = 0) : _Name(Name), _ExpectedArgumentCnt(ExpectedArgumentCnt), _Filesystem(Filesystem), _DataAddress(DataAddress), _DataPort(DataPort), _Client(Client), _PassiveServer(PassiveServer), _PassiveMode(PassiveMode), _DataConnection(0) { } virtual ~FTPCommand() { + if (_DataConnection != 0) { + _DataConnection->stop(); + delete _DataConnection; + _DataConnection = 0; + } } String getName() const { @@ -34,6 +40,33 @@ class FTPCommand { if (_DataConnection->connected()) { _DataConnection->stop(); } + if (_PassiveMode != 0 && *_PassiveMode) { + if (_PassiveServer == 0 || *_PassiveServer == 0) { + SendResponse(425, "No passive server"); + return false; + } + WiFiServer *server = *_PassiveServer; + unsigned long start = millis(); + while (!server->hasClient() && millis() - start < 10000) { + delay(10); + } + if (!server->hasClient()) { + StopPassiveServer(); + SendResponse(425, "No data connection"); + return false; + } + WiFiClient client = server->available(); + if (!client) { + StopPassiveServer(); + SendResponse(425, "No data connection"); + return false; + } + *_DataConnection = client; + StopPassiveServer(); + *_PassiveMode = false; + SendResponse(150, "Accepted data connection"); + return true; + } _DataConnection->connect(*_DataAddress, *_DataPort); if (!_DataConnection->connected()) { _DataConnection->stop(); @@ -73,7 +106,22 @@ class FTPCommand { } void CloseDataConnection() { - _DataConnection->stop(); + if (_DataConnection != 0) { + _DataConnection->stop(); + } + if (_PassiveMode != 0 && *_PassiveMode) { + StopPassiveServer(); + *_PassiveMode = false; + } + } + +private: + void StopPassiveServer() { + if (_PassiveServer != 0 && *_PassiveServer != 0) { + (*_PassiveServer)->stop(); + delete *_PassiveServer; + *_PassiveServer = 0; + } } protected: @@ -82,6 +130,8 @@ class FTPCommand { FTPFilesystem *const _Filesystem; IPAddress *const _DataAddress; int *const _DataPort; + WiFiServer **const _PassiveServer; + bool *const _PassiveMode; private: WiFiClient *const _Client; @@ -90,7 +140,7 @@ class FTPCommand { class FTPCommandTransfer : public FTPCommand { public: - FTPCommandTransfer(String Name, int ExpectedArgumentCnt, WiFiClient *const Client, FTPFilesystem *const Filesystem = 0, IPAddress *DataAddress = 0, int *DataPort = 0) : FTPCommand(Name, ExpectedArgumentCnt, Client, Filesystem, DataAddress, DataPort) { + FTPCommandTransfer(String Name, int ExpectedArgumentCnt, WiFiClient *const Client, FTPFilesystem *const Filesystem = 0, IPAddress *DataAddress = 0, int *DataPort = 0, WiFiServer **PassiveServer = 0, bool *PassiveMode = 0) : FTPCommand(Name, ExpectedArgumentCnt, Client, Filesystem, DataAddress, DataPort, PassiveServer, PassiveMode) { } virtual void workOnData() = 0; diff --git a/src/FTPConnection.cpp b/src/FTPConnection.cpp index f801b44..f7797a4 100644 --- a/src/FTPConnection.cpp +++ b/src/FTPConnection.cpp @@ -9,6 +9,7 @@ #include "Commands/MKD.h" #include "Commands/MLSD.h" #include "Commands/NLST.h" +#include "Commands/PASV.h" #include "Commands/PORT.h" #include "Commands/PWD.h" #include "Commands/RETR.h" @@ -20,19 +21,20 @@ #include "ESP-FTP-Server-Lib.h" #include "common.h" -FTPConnection::FTPConnection(const WiFiClient &Client, std::list &UserList, FTPFilesystem &Filesystem) : _ClientState(Idle), _Client(Client), _Filesystem(Filesystem), _UserList(UserList), _AuthUsername("") { - std::shared_ptr retr = std::shared_ptr(new RETR(&_Client, &_Filesystem, &_DataAddress, &_DataPort)); - std::shared_ptr stor = std::shared_ptr(new STOR(&_Client, &_Filesystem, &_DataAddress, &_DataPort)); +FTPConnection::FTPConnection(const WiFiClient &Client, std::list &UserList, FTPFilesystem &Filesystem) : _ClientState(Idle), _Client(Client), _Filesystem(Filesystem), _UserList(UserList), _AuthUsername(""), _PassiveServer(nullptr), _PassiveMode(false) { + std::shared_ptr retr = std::shared_ptr(new RETR(&_Client, &_Filesystem, &_DataAddress, &_DataPort, &_PassiveServer, &_PassiveMode)); + std::shared_ptr stor = std::shared_ptr(new STOR(&_Client, &_Filesystem, &_DataAddress, &_DataPort, &_PassiveServer, &_PassiveMode)); _FTPCommands.push_back(std::shared_ptr(new CDUP(&_Client))); _FTPCommands.push_back(std::shared_ptr(new CWD(&_Client, &_Filesystem))); _FTPCommands.push_back(std::shared_ptr(new DELE(&_Client, &_Filesystem))); _FTPCommands.push_back(std::shared_ptr(new STAT(&_Client, &_Filesystem))); - _FTPCommands.push_back(std::shared_ptr(new LIST(&_Client, &_Filesystem, &_DataAddress, &_DataPort))); - _FTPCommands.push_back(std::shared_ptr(new NLST(&_Client, &_Filesystem, &_DataAddress, &_DataPort))); - _FTPCommands.push_back(std::shared_ptr(new MLSD(&_Client, &_Filesystem, &_DataAddress, &_DataPort))); + _FTPCommands.push_back(std::shared_ptr(new LIST(&_Client, &_Filesystem, &_DataAddress, &_DataPort, &_PassiveServer, &_PassiveMode))); + _FTPCommands.push_back(std::shared_ptr(new NLST(&_Client, &_Filesystem, &_DataAddress, &_DataPort, &_PassiveServer, &_PassiveMode))); + _FTPCommands.push_back(std::shared_ptr(new MLSD(&_Client, &_Filesystem, &_DataAddress, &_DataPort, &_PassiveServer, &_PassiveMode))); _FTPCommands.push_back(std::shared_ptr(new MKD(&_Client, &_Filesystem))); - _FTPCommands.push_back(std::shared_ptr(new PORT(&_Client, &_DataAddress, &_DataPort))); + _FTPCommands.push_back(std::shared_ptr(new PORT(&_Client, &_DataAddress, &_DataPort, &_PassiveServer, &_PassiveMode))); + _FTPCommands.push_back(std::shared_ptr(new PASV(&_Client, &_DataAddress, &_DataPort, &_PassiveServer, &_PassiveMode))); _FTPCommands.push_back(std::shared_ptr(new PWD(&_Client))); _FTPCommands.push_back(retr); _FTPCommands.push_back(std::shared_ptr(new RMD(&_Client, &_Filesystem))); @@ -60,6 +62,11 @@ FTPConnection::FTPConnection(const WiFiClient &Client, std::list &UserL } FTPConnection::~FTPConnection() { + if (_PassiveServer != nullptr) { + _PassiveServer->stop(); + delete _PassiveServer; + _PassiveServer = nullptr; + } #ifndef NO_GLOBAL_INSTANCES Serial.println("Connection closed!"); #else @@ -126,6 +133,7 @@ bool FTPConnection::handle() { _Client.println("211- Extensions suported:"); _Client.println(" UTF8"); _Client.println(" MLSD"); + _Client.println(" PASV"); _Client.println("211 End."); _Line = ""; return true; diff --git a/src/FTPConnection.h b/src/FTPConnection.h index 24e97c8..327aa04 100644 --- a/src/FTPConnection.h +++ b/src/FTPConnection.h @@ -46,8 +46,10 @@ class FTPConnection { std::list &_UserList; String _AuthUsername; - IPAddress _DataAddress; - int _DataPort; + IPAddress _DataAddress; + int _DataPort; + WiFiServer *_PassiveServer; + bool _PassiveMode; FTPPath _WorkDirectory; From 704cd19c8375dc2c132d26297b1a5b3a40829691 Mon Sep 17 00:00:00 2001 From: Joe <1015.5@gmx.de> Date: Mon, 6 Apr 2026 10:44:39 +0200 Subject: [PATCH 02/18] try to fix build (depricated packatge), add small timeout on data_read (see issues) --- platformio.ini | 2 +- src/Commands/PASV.h | 18 +++++++++++++----- src/FTPCommand.h | 9 +++++++++ 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/platformio.ini b/platformio.ini index d56b186..0df9b4e 100644 --- a/platformio.ini +++ b/platformio.ini @@ -10,7 +10,7 @@ check_flags = check_skip_packages = yes [env:ttgo-ESP8266] -platform = espressif8266 @ 4.0.0 +platform = espressif8266 @ 4.2.0 board = esp_wroom_02 framework = arduino test_build_src = yes diff --git a/src/Commands/PASV.h b/src/Commands/PASV.h index 89a24d3..160fdbb 100644 --- a/src/Commands/PASV.h +++ b/src/Commands/PASV.h @@ -1,9 +1,13 @@ #ifndef PASV_H_ #define PASV_H_ -#include #include #include +#if defined(ESP32) +#include +#elif defined(ESP8266) +#include +#endif #include "../FTPCommand.h" #include "../common.h" @@ -28,10 +32,14 @@ class PASV : public FTPCommand { *_PassiveServer = new WiFiServer(port); (*_PassiveServer)->begin(); } - IPAddress localIP = WiFi.localIP(); - int p1 = port / 256; - int p2 = port % 256; - String response = "Entering Passive Mode (" + String(localIP[0]) + "," + String(localIP[1]) + "," + String(localIP[2]) + "," + String(localIP[3]) + "," + String(p1) + "," + String(p2) + ")"; + IPAddress localIP = WiFi.localIP(); + if (localIP == IPAddress(0, 0, 0, 0)) { + SendResponse(425, "No local IP address for passive mode"); + return; + } + int p1 = port / 256; + int p2 = port % 256; + String response = "Entering Passive Mode (" + String(localIP[0]) + "," + String(localIP[1]) + "," + String(localIP[2]) + "," + String(localIP[3]) + "," + String(p1) + "," + String(p2) + ")"; SendResponse(227, response); } }; diff --git a/src/FTPCommand.h b/src/FTPCommand.h index c642954..72eebfe 100644 --- a/src/FTPCommand.h +++ b/src/FTPCommand.h @@ -102,6 +102,15 @@ class FTPCommand { if ((_DataConnection != 0) && (_DataConnection->available() > 0)) { return _DataConnection->readBytes(c, l); } + // Brief wait for initial data (max 100ms in 1ms intervals) + if (_DataConnection != 0) { + for (int i = 0; i < 100; i++) { + delay(1); + if (_DataConnection->available() > 0) { + return _DataConnection->readBytes(c, l); + } + } + } return 0; } From 9e8c657c744ff61b8d37c325c6a1817874efb0f6 Mon Sep 17 00:00:00 2001 From: Joe <1015.5@gmx.de> Date: Mon, 6 Apr 2026 10:53:15 +0200 Subject: [PATCH 03/18] try to fix pipeline --- .github/workflows/main.yml | 2 +- src/Commands/MLSD.h | 6 +++--- src/FTPPath.cpp | 2 +- src/FTPPath.h | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 6aa3aa3..d8abd8a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -85,7 +85,7 @@ jobs: - name: Run cppcheck and create html run: docker run --rm -v ${PWD}:/src facthunder/cppcheck:latest /bin/bash -c "cppcheck --xml $CPPCHECK_ARGS 2> report.xml && cppcheck-htmlreport --file=report.xml --report-dir=output" - name: Upload report - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: Cppcheck Report path: output diff --git a/src/Commands/MLSD.h b/src/Commands/MLSD.h index 8eb748d..9d6faba 100644 --- a/src/Commands/MLSD.h +++ b/src/Commands/MLSD.h @@ -46,9 +46,9 @@ class MLSD : public FTPCommand { data_print(";"); // modify=YYYYMMDDHHMMSS; // GMT (!!!!) - char buf[128]; - time_t ft = f.getLastWrite(); - struct tm *t = localtime(&ft); + char buf[128]; + time_t ft = f.getLastWrite(); + const struct tm *t = localtime(&ft); sprintf(buf, "modify=%4d%02d%02d%02d%02d%02d;", (t->tm_year) + 1900, (t->tm_mon) + 1, t->tm_mday, t->tm_hour, t->tm_min, t->tm_sec); data_print(String(buf)); diff --git a/src/FTPPath.cpp b/src/FTPPath.cpp index 57749ec..37c6973 100644 --- a/src/FTPPath.cpp +++ b/src/FTPPath.cpp @@ -52,7 +52,7 @@ std::list FTPPath::splitPath(String path) { return p; } -String FTPPath::createPath(std::list path) { +String FTPPath::createPath(const std::list &path) { if (path.size() == 0) { return "/"; } diff --git a/src/FTPPath.h b/src/FTPPath.h index 7f62ba1..4bfccdd 100644 --- a/src/FTPPath.h +++ b/src/FTPPath.h @@ -17,7 +17,7 @@ class FTPPath { String getFilePath(String filename) const; static std::list splitPath(String path); - static String createPath(std::list path); + static String createPath(const std::list &path); private: std::list _Path; From dcd522bc6bebe6f74ac20ba7468534cfb396f186 Mon Sep 17 00:00:00 2001 From: Joe <1015.5@gmx.de> Date: Mon, 6 Apr 2026 11:23:29 +0200 Subject: [PATCH 04/18] fix compiler-carnings --- src/Commands/STOR.h | 2 +- src/ESP-FTP-Server-Lib.cpp | 2 +- src/FTPCommand.h | 4 ++-- src/FTPFilesystem.h | 3 +++ 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Commands/STOR.h b/src/Commands/STOR.h index a89213b..6f3e022 100644 --- a/src/Commands/STOR.h +++ b/src/Commands/STOR.h @@ -35,7 +35,7 @@ class STOR : public FTPCommandTransfer { int nb = data_read(buffer, FTP_BUF_SIZE); if (nb > 0) { const auto wb = _file.write(buffer, nb); - if (wb != nb) { + if (wb != static_cast(nb)) { _file.close(); this->_Filesystem->remove(_ftpFsFilePath.c_str()); diff --git a/src/ESP-FTP-Server-Lib.cpp b/src/ESP-FTP-Server-Lib.cpp index 752448a..c96b07d 100644 --- a/src/ESP-FTP-Server-Lib.cpp +++ b/src/ESP-FTP-Server-Lib.cpp @@ -37,7 +37,7 @@ bool isNotConnected(const std::shared_ptr &con) { void FTPServer::handle() { if (_Server.hasClient()) { - std::shared_ptr connection = std::shared_ptr(new FTPConnection(_Server.available(), _UserList, _Filesystem)); + std::shared_ptr connection = std::shared_ptr(new FTPConnection(_Server.accept(), _UserList, _Filesystem)); _Connections.push_back(connection); } for (std::shared_ptr con : _Connections) { diff --git a/src/FTPCommand.h b/src/FTPCommand.h index 72eebfe..618795c 100644 --- a/src/FTPCommand.h +++ b/src/FTPCommand.h @@ -11,7 +11,7 @@ class FTPCommand { public: - FTPCommand(String Name, int ExpectedArgumentCnt, WiFiClient *const Client, FTPFilesystem *const Filesystem = 0, IPAddress *DataAddress = 0, int *DataPort = 0, WiFiServer **PassiveServer = 0, bool *PassiveMode = 0) : _Name(Name), _ExpectedArgumentCnt(ExpectedArgumentCnt), _Filesystem(Filesystem), _DataAddress(DataAddress), _DataPort(DataPort), _Client(Client), _PassiveServer(PassiveServer), _PassiveMode(PassiveMode), _DataConnection(0) { + FTPCommand(String Name, int ExpectedArgumentCnt, WiFiClient *const Client, FTPFilesystem *const Filesystem = 0, IPAddress *DataAddress = 0, int *DataPort = 0, WiFiServer **PassiveServer = 0, bool *PassiveMode = 0) : _Name(Name), _ExpectedArgumentCnt(ExpectedArgumentCnt), _Filesystem(Filesystem), _DataAddress(DataAddress), _DataPort(DataPort), _PassiveServer(PassiveServer), _PassiveMode(PassiveMode), _Client(Client), _DataConnection(0) { } virtual ~FTPCommand() { if (_DataConnection != 0) { @@ -55,7 +55,7 @@ class FTPCommand { SendResponse(425, "No data connection"); return false; } - WiFiClient client = server->available(); + WiFiClient client = server->accept(); if (!client) { StopPassiveServer(); SendResponse(425, "No data connection"); diff --git a/src/FTPFilesystem.h b/src/FTPFilesystem.h index 2d8836d..d44497d 100644 --- a/src/FTPFilesystem.h +++ b/src/FTPFilesystem.h @@ -107,7 +107,10 @@ class FTPFileImpl : public fs::FileImpl { return true; }; const char *fullName() const override { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wreturn-local-addr" return ("/" + _Name).c_str(); +#pragma GCC diagnostic pop }; bool isFile() const override { return true; From 1e4b142cbb78c52fdc26e68be54f2292d548ed74 Mon Sep 17 00:00:00 2001 From: Joe91 <1015.5@gmx.de> Date: Sat, 18 Apr 2026 15:46:18 +0200 Subject: [PATCH 05/18] fix: review-issues --- src/Commands/PASV.h | 15 +++++++++------ src/FTPCommand.h | 11 +++++++---- src/FTPConnection.cpp | 2 +- src/FTPFilesystem.h | 8 ++++---- 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/Commands/PASV.h b/src/Commands/PASV.h index 160fdbb..86e4ee6 100644 --- a/src/Commands/PASV.h +++ b/src/Commands/PASV.h @@ -23,8 +23,13 @@ class PASV : public FTPCommand { delete *_PassiveServer; *_PassiveServer = 0; } - if (_PassiveMode != 0) { - *_PassiveMode = true; + IPAddress localIP = WiFi.localIP(); + if (localIP == IPAddress(0, 0, 0, 0)) { + if (_PassiveMode != 0) { + *_PassiveMode = false; + } + SendResponse(425, "No local IP address for passive mode"); + return; } int port = 20000 + random(0, 1000); *_DataPort = port; @@ -32,10 +37,8 @@ class PASV : public FTPCommand { *_PassiveServer = new WiFiServer(port); (*_PassiveServer)->begin(); } - IPAddress localIP = WiFi.localIP(); - if (localIP == IPAddress(0, 0, 0, 0)) { - SendResponse(425, "No local IP address for passive mode"); - return; + if (_PassiveMode != 0) { + *_PassiveMode = true; } int p1 = port / 256; int p2 = port % 256; diff --git a/src/FTPCommand.h b/src/FTPCommand.h index 618795c..9c8edbc 100644 --- a/src/FTPCommand.h +++ b/src/FTPCommand.h @@ -45,10 +45,13 @@ class FTPCommand { SendResponse(425, "No passive server"); return false; } - WiFiServer *server = *_PassiveServer; - unsigned long start = millis(); - while (!server->hasClient() && millis() - start < 10000) { - delay(10); + WiFiServer *server = *_PassiveServer; + const unsigned long passiveAcceptTimeoutMs = 100; + const unsigned long passivePollDelayMs = 5; + unsigned long start = millis(); + while (!server->hasClient() && millis() - start < passiveAcceptTimeoutMs) { + yield(); + delay(passivePollDelayMs); } if (!server->hasClient()) { StopPassiveServer(); diff --git a/src/FTPConnection.cpp b/src/FTPConnection.cpp index f7797a4..c6b4244 100644 --- a/src/FTPConnection.cpp +++ b/src/FTPConnection.cpp @@ -130,7 +130,7 @@ bool FTPConnection::handle() { _Line = ""; return true; } else if (command == "FEAT") { - _Client.println("211- Extensions suported:"); + _Client.println("211- Extensions supported:"); _Client.println(" UTF8"); _Client.println(" MLSD"); _Client.println(" PASV"); diff --git a/src/FTPFilesystem.h b/src/FTPFilesystem.h index d44497d..d5547bb 100644 --- a/src/FTPFilesystem.h +++ b/src/FTPFilesystem.h @@ -107,10 +107,9 @@ class FTPFileImpl : public fs::FileImpl { return true; }; const char *fullName() const override { -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wreturn-local-addr" - return ("/" + _Name).c_str(); -#pragma GCC diagnostic pop + // Update the cached full path and return its pointer + _FullName = "/" + _Name; + return _FullName.c_str(); }; bool isFile() const override { return true; @@ -150,6 +149,7 @@ class FTPFileImpl : public fs::FileImpl { private: String _Name; + mutable String _FullName; // Cache for fullName() to avoid dangling pointer std::list _Filesystems; }; From 9a831e231563239dad73ca050de48ba91922b22f Mon Sep 17 00:00:00 2001 From: Joe91 <1015.5@gmx.de> Date: Sat, 18 Apr 2026 16:06:22 +0200 Subject: [PATCH 06/18] Security improvements: Enhanced input validation and error handling - Added parameter and filename validation to CWD, LIST, and NLST commands - Enhanced FTPPath::isValidFilename() with path traversal protection - Added Windows reserved name checking and length limits - Created FTPResponseCodes.h for standardized response codes - Added comprehensive test coverage for validation functions - Updated README with security features documentation - Improved error messages and response code consistency --- README.md | 19 ++++++++- src/Commands/CDUP.h | 3 +- src/Commands/CWD.h | 15 ++++++- src/Commands/DELE.h | 15 +++++-- src/Commands/LIST.h | 15 +++++-- src/Commands/MKD.h | 15 +++++-- src/Commands/NLST.h | 15 +++++-- src/Commands/PORT.h | 2 +- src/Commands/PWD.h | 3 +- src/Commands/RETR.h | 13 +++++- src/Commands/RMD.h | 15 +++++-- src/Commands/RNFR_RNTO.h | 27 +++++++++--- src/Commands/STOR.h | 15 +++++-- src/Commands/TYPE.h | 7 +-- src/FTPCommand.h | 15 ++++--- src/FTPPath.cpp | 43 +++++++++++++++++++ src/FTPPath.h | 1 + src/FTPResponseCodes.h | 46 ++++++++++++++++++++ test/FTPPath/validation_tests.cpp | 71 +++++++++++++++++++++++++++++++ 19 files changed, 313 insertions(+), 42 deletions(-) create mode 100644 src/FTPResponseCodes.h create mode 100644 test/FTPPath/validation_tests.cpp diff --git a/README.md b/README.md index f372065..316c7bb 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,14 @@ This library will provide a simple and modern FTP server for your ESP32 or ESP8266 device. You can setup multiple users and mutliple filesystems (SD-Card, MMC-Card or/and SPIFFS). +## Security Features + +This library includes comprehensive input validation and security protections: +- **Filename validation**: Prevents illegal characters in filenames (?, *, :, ", <, >, |, \) +- **Parameter validation**: Validates command syntax and required parameters +- **Path traversal protection**: Helps prevent directory traversal attacks +- **Error handling**: Proper FTP response codes for security-related errors + ## Examples In the example folder you can find a very simple usage of the FTP server. You just need to setup the users, add the filesystems which you want to use, and call the handle function in the loop. @@ -15,6 +23,7 @@ Currently all kind of simple commands are known to the server: * DELE * LISST * MKD +* PASV * PORT * PWD * RETR @@ -33,4 +42,12 @@ Currently all kind of simple commands are known to the server: Some commands are still missing, if you need them create a ticket :) -Currently just the active mode is supported. For the passive mode you need to wait until version 1.0.0. +**Security Improvements Completed:** +- Input validation for all critical commands (DELE, MKD, RETR, RMD, RNFR, RNTO, STOR) +- Filename validation with illegal character detection +- Proper error responses with standard FTP codes + +**Still missing:** +- Additional FTP commands: FEAT, MDTM, SIZE, REST +- Rate limiting and connection throttling +- Enhanced logging capabilities diff --git a/src/Commands/CDUP.h b/src/Commands/CDUP.h index a88aba7..68a8a54 100644 --- a/src/Commands/CDUP.h +++ b/src/Commands/CDUP.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" class CDUP : public FTPCommand { public: @@ -12,7 +13,7 @@ class CDUP : public FTPCommand { void run(FTPPath &WorkDirectory, const std::vector &Line) override { WorkDirectory.goPathUp(); - SendResponse(250, "Ok. Current directory is " + WorkDirectory.getPath()); + SendResponse(FTPResponse::FILE_ACTION_OK, "Ok. Current directory is " + WorkDirectory.getPath()); } }; diff --git a/src/Commands/CWD.h b/src/Commands/CWD.h index 5071d1c..5fd991a 100644 --- a/src/Commands/CWD.h +++ b/src/Commands/CWD.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" class CWD : public FTPCommand { public: @@ -11,18 +12,28 @@ class CWD : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { + if (Line.size() < 2) { + SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); + return; + } + FTPPath path = WorkDirectory; if (Line[1] == "..") { path.goPathUp(); } else { + // Validate path for illegal characters + if (!FTPPath::isValidFilename(Line[1])) { + SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename or path"); + return; + } path.changePath(Line[1]); } File dir = _Filesystem->open(path.getPath()); if (dir.isDirectory()) { WorkDirectory = path; - SendResponse(250, "Ok. Current directory is " + WorkDirectory.getPath()); + SendResponse(FTPResponse::FILE_ACTION_OK, "Ok. Current directory is " + WorkDirectory.getPath()); } else { - SendResponse(550, "Directory does not exist"); + SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "Directory does not exist"); } } }; diff --git a/src/Commands/DELE.h b/src/Commands/DELE.h index c8ec30c..da240cd 100644 --- a/src/Commands/DELE.h +++ b/src/Commands/DELE.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" class DELE : public FTPCommand { public: @@ -11,15 +12,23 @@ class DELE : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { + if (Line.size() < 2) { + SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); + return; + } + if (!FTPPath::isValidFilename(Line[1])) { + SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename"); + return; + } String filepath = WorkDirectory.getFilePath(Line[1]); if (!_Filesystem->exists(filepath)) { - SendResponse(550, "File " + filepath + " not found"); + SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "File " + filepath + " not found"); return; } if (_Filesystem->remove(filepath)) { - SendResponse(250, " Deleted \"" + filepath + "\""); + SendResponse(FTPResponse::FILE_ACTION_OK, " Deleted \"" + filepath + "\""); } else { - SendResponse(450, "Can't delete \"" + filepath + "\""); + SendResponse(FTPResponse::FILE_ACTION_ABORTED, "Can't delete \"" + filepath + "\""); } } }; diff --git a/src/Commands/LIST.h b/src/Commands/LIST.h index 2601fa0..84c4582 100644 --- a/src/Commands/LIST.h +++ b/src/Commands/LIST.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" #include "../common.h" class LIST : public FTPCommand { @@ -12,13 +13,21 @@ class LIST : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { + // Validate optional path parameter + if (Line.size() > 1 && !FTPPath::isValidFilename(Line[1])) { + SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename or path"); + return; + } + if (!ConnectDataConnection()) { return; } - File dir = _Filesystem->open(WorkDirectory.getPath()); // + + String listPath = (Line.size() > 1) ? WorkDirectory.getFilePath(Line[1]) : WorkDirectory.getPath(); + File dir = _Filesystem->open(listPath); // if (!dir || !dir.isDirectory()) { CloseDataConnection(); - SendResponse(550, "Can't open directory " + WorkDirectory.getPath()); + SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "Can't open directory " + WorkDirectory.getPath()); return; } int cnt = 0; @@ -43,7 +52,7 @@ class LIST : public FTPCommand { f = dir.openNextFile(); } CloseDataConnection(); - SendResponse(226, String(cnt) + " matches total"); + SendResponse(FTPResponse::TRANSFER_COMPLETE, String(cnt) + " matches total"); } }; diff --git a/src/Commands/MKD.h b/src/Commands/MKD.h index e1c1765..363c58d 100644 --- a/src/Commands/MKD.h +++ b/src/Commands/MKD.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" class MKD : public FTPCommand { public: @@ -11,15 +12,23 @@ class MKD : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { + if (Line.size() < 2) { + SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); + return; + } + if (!FTPPath::isValidFilename(Line[1])) { + SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename"); + return; + } String filepath = WorkDirectory.getFilePath(Line[1]); if (_Filesystem->exists(filepath)) { - SendResponse(521, "Can't create \"" + filepath + "\", Directory exists"); + SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "Can't create \"" + filepath + "\", Directory exists"); return; } if (_Filesystem->mkdir(filepath)) { - SendResponse(257, "\"" + filepath + "\" created"); + SendResponse(FTPResponse::PATHNAME_CREATED, "\"" + filepath + "\" created"); } else { - SendResponse(550, "Can't create \"" + filepath + "\""); + SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "Can't create \"" + filepath + "\""); } } }; diff --git a/src/Commands/NLST.h b/src/Commands/NLST.h index ca35d95..e484290 100644 --- a/src/Commands/NLST.h +++ b/src/Commands/NLST.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" #include "../common.h" class NLST : public FTPCommand { @@ -12,13 +13,21 @@ class NLST : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { + // Validate optional path parameter + if (Line.size() > 1 && !FTPPath::isValidFilename(Line[1])) { + SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename or path"); + return; + } + if (!ConnectDataConnection()) { return; } - File dir = _Filesystem->open(WorkDirectory.getPath()); // + + String listPath = (Line.size() > 1) ? WorkDirectory.getFilePath(Line[1]) : WorkDirectory.getPath(); + File dir = _Filesystem->open(listPath); // if (!dir || !dir.isDirectory()) { CloseDataConnection(); - SendResponse(550, "Can't open directory " + WorkDirectory.getPath()); + SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "Can't open directory " + WorkDirectory.getPath()); return; } int cnt = 0; @@ -31,7 +40,7 @@ class NLST : public FTPCommand { f = dir.openNextFile(); } CloseDataConnection(); - SendResponse(226, String(cnt) + " matches total"); + SendResponse(FTPResponse::TRANSFER_COMPLETE, String(cnt) + " matches total"); } }; diff --git a/src/Commands/PORT.h b/src/Commands/PORT.h index f047df1..7d634b2 100644 --- a/src/Commands/PORT.h +++ b/src/Commands/PORT.h @@ -25,7 +25,7 @@ class PORT : public FTPCommand { (*_DataAddress)[i] = connection_details[i].toInt(); } *_DataPort = connection_details[4].toInt() * 256 + connection_details[5].toInt(); - SendResponse(200, "PORT command successful"); + SendResponse(FTPResponse::COMMAND_OK, "PORT command successful"); } }; diff --git a/src/Commands/PWD.h b/src/Commands/PWD.h index 9aa8dfc..bd04651 100644 --- a/src/Commands/PWD.h +++ b/src/Commands/PWD.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" class PWD : public FTPCommand { public: @@ -11,7 +12,7 @@ class PWD : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { - SendResponse(257, "\"" + WorkDirectory.getPath() + "\" is your current directory"); + SendResponse(FTPResponse::PATHNAME_CREATED, "\"" + WorkDirectory.getPath() + "\" is your current directory"); } }; diff --git a/src/Commands/RETR.h b/src/Commands/RETR.h index 53075b4..9e92ac7 100644 --- a/src/Commands/RETR.h +++ b/src/Commands/RETR.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" #include "../common.h" #define FTP_BUF_SIZE 4096 @@ -17,6 +18,14 @@ class RETR : public FTPCommandTransfer { if (trasferInProgress()) { return; } + if (Line.size() < 2) { + SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); + return; + } + if (!FTPPath::isValidFilename(Line[1])) { + SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename"); + return; + } if (!ConnectDataConnection()) { return; } @@ -24,7 +33,7 @@ class RETR : public FTPCommandTransfer { _file = _Filesystem->open(path); if (!_file || _file.isDirectory()) { CloseDataConnection(); - SendResponse(550, "Can't open " + path); + SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "Can't open " + path); return; } workOnData(); @@ -38,7 +47,7 @@ class RETR : public FTPCommandTransfer { return; } CloseDataConnection(); - SendResponse(226, "File successfully transferred"); + SendResponse(FTPResponse::TRANSFER_COMPLETE, "File successfully transferred"); _file.close(); } diff --git a/src/Commands/RMD.h b/src/Commands/RMD.h index dad6f52..a7e2ada 100644 --- a/src/Commands/RMD.h +++ b/src/Commands/RMD.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" class RMD : public FTPCommand { public: @@ -11,15 +12,23 @@ class RMD : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { + if (Line.size() < 2) { + SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); + return; + } + if (!FTPPath::isValidFilename(Line[1])) { + SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename"); + return; + } String filepath = WorkDirectory.getFilePath(Line[1]); if (!_Filesystem->exists(filepath)) { - SendResponse(550, "Folder " + filepath + " not found"); + SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "Folder " + filepath + " not found"); return; } if (_Filesystem->rmdir(filepath)) { - SendResponse(250, " Deleted \"" + filepath + "\""); + SendResponse(FTPResponse::FILE_ACTION_OK, " Deleted \"" + filepath + "\""); } else { - SendResponse(450, "Can't delete \"" + filepath + "\""); + SendResponse(FTPResponse::FILE_ACTION_ABORTED, "Can't delete \"" + filepath + "\""); } } }; diff --git a/src/Commands/RNFR_RNTO.h b/src/Commands/RNFR_RNTO.h index 6a66e9d..034b168 100644 --- a/src/Commands/RNFR_RNTO.h +++ b/src/Commands/RNFR_RNTO.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" class RNFR_RNTO : public FTPCommand { public: @@ -19,30 +20,44 @@ class RNFR_RNTO : public FTPCommand { } void from(const FTPPath &WorkDirectory, const std::vector &Line) { + if (Line.size() < 2) { + SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); + return; + } String filepath = WorkDirectory.getFilePath(Line[1]); if (!_Filesystem->exists(filepath)) { - SendResponse(550, "File " + Line[1] + " not found"); + SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "File " + Line[1] + " not found"); return; } _fromSet = true; _from = filepath; - SendResponse(350, "RNFR accepted - file exists, ready for destination"); + SendResponse(FTPResponse::FILE_ACTION_PENDING, "RNFR accepted - file exists, ready for destination"); } void to(const FTPPath &WorkDirectory, const std::vector &Line) { + if (Line.size() < 2) { + SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); + return; + } if (!_fromSet) { - SendResponse(503, "Need RNFR before RNTO"); + SendResponse(FTPResponse::BAD_SEQUENCE, "Need RNFR before RNTO"); + return; + } + if (!FTPPath::isValidFilename(Line[1])) { + SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename"); + _fromSet = false; + _from = ""; return; } String filepath = WorkDirectory.getFilePath(Line[1]); if (_Filesystem->exists(filepath)) { - SendResponse(553, "File " + Line[1] + " already exists"); + SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "File " + Line[1] + " already exists"); return; } if (_Filesystem->rename(_from, filepath)) { - SendResponse(250, "File successfully renamed or moved"); + SendResponse(FTPResponse::FILE_ACTION_OK, "File successfully renamed or moved"); } else { - SendResponse(451, "Rename/move failure"); + SendResponse(FTPResponse::FILE_ACTION_ABORTED_LOCAL_ERROR, "Rename/move failure"); } _fromSet = false; _from = ""; diff --git a/src/Commands/STOR.h b/src/Commands/STOR.h index 6f3e022..d80c51b 100644 --- a/src/Commands/STOR.h +++ b/src/Commands/STOR.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" #include "../common.h" #define FTP_BUF_SIZE 4096 @@ -17,13 +18,21 @@ class STOR : public FTPCommandTransfer { if (trasferInProgress()) { return; } + if (Line.size() < 2) { + SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); + return; + } + if (!FTPPath::isValidFilename(Line[1])) { + SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename"); + return; + } if (!ConnectDataConnection()) { return; } _ftpFsFilePath = WorkDirectory.getFilePath(Line[1]); _file = _Filesystem->open(_ftpFsFilePath, "w"); if (!_file) { - SendResponse(451, "Can't open/create " + _ftpFsFilePath); + SendResponse(FTPResponse::FILE_ACTION_ABORTED_LOCAL_ERROR, "Can't open/create " + _ftpFsFilePath); CloseDataConnection(); return; } @@ -39,14 +48,14 @@ class STOR : public FTPCommandTransfer { _file.close(); this->_Filesystem->remove(_ftpFsFilePath.c_str()); - SendResponse(552, "Error occured while STORing"); + SendResponse(FTPResponse::EXCEEDED_STORAGE, "Error occured while STORing"); CloseDataConnection(); } return; } - SendResponse(226, "File successfully transferred"); + SendResponse(FTPResponse::TRANSFER_COMPLETE, "File successfully transferred"); CloseDataConnection(); _file.close(); } diff --git a/src/Commands/TYPE.h b/src/Commands/TYPE.h index 522e2ab..c5224b9 100644 --- a/src/Commands/TYPE.h +++ b/src/Commands/TYPE.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" class TYPE : public FTPCommand { public: @@ -12,13 +13,13 @@ class TYPE : public FTPCommand { void run(FTPPath &WorkDirectory, const std::vector &Line) override { if (Line[1] == "A") { - SendResponse(200, "TYPE is now ASCII"); + SendResponse(FTPResponse::COMMAND_OK, "TYPE is now ASCII"); return; } else if (Line[1] == "I") { - SendResponse(200, "TYPE is now 8-bit binary"); + SendResponse(FTPResponse::COMMAND_OK, "TYPE is now 8-bit binary"); return; } - SendResponse(504, "Unknow TYPE"); + SendResponse(FTPResponse::COMMAND_NOT_IMPLEMENTED_FOR_PARAMETER, "Unknown TYPE"); } }; diff --git a/src/FTPCommand.h b/src/FTPCommand.h index 9c8edbc..c05ab56 100644 --- a/src/FTPCommand.h +++ b/src/FTPCommand.h @@ -8,6 +8,7 @@ #include "FTPFilesystem.h" #include "FTPPath.h" +#include "FTPResponseCodes.h" class FTPCommand { public: @@ -42,7 +43,7 @@ class FTPCommand { } if (_PassiveMode != 0 && *_PassiveMode) { if (_PassiveServer == 0 || *_PassiveServer == 0) { - SendResponse(425, "No passive server"); + SendResponse(FTPResponse::NO_DATA_CONNECTION, "No passive server"); return false; } WiFiServer *server = *_PassiveServer; @@ -55,28 +56,28 @@ class FTPCommand { } if (!server->hasClient()) { StopPassiveServer(); - SendResponse(425, "No data connection"); + SendResponse(FTPResponse::NO_DATA_CONNECTION, "No data connection"); return false; } WiFiClient client = server->accept(); if (!client) { StopPassiveServer(); - SendResponse(425, "No data connection"); + SendResponse(FTPResponse::NO_DATA_CONNECTION, "No data connection"); return false; } *_DataConnection = client; StopPassiveServer(); *_PassiveMode = false; - SendResponse(150, "Accepted data connection"); + SendResponse(FTPResponse::DATA_CONNECTION_OPEN, "Accepted data connection"); return true; } _DataConnection->connect(*_DataAddress, *_DataPort); if (!_DataConnection->connected()) { _DataConnection->stop(); - SendResponse(425, "No data connection"); + SendResponse(FTPResponse::NO_DATA_CONNECTION, "No data connection"); return false; } - SendResponse(150, "Accepted data connection"); + SendResponse(FTPResponse::DATA_CONNECTION_OPEN, "Accepted data connection"); return true; } @@ -164,7 +165,7 @@ class FTPCommandTransfer : public FTPCommand { void abort() { if (_file) { CloseDataConnection(); - SendResponse(426, "Transfer aborted"); + SendResponse(FTPResponse::CONNECTION_CLOSED, "Transfer aborted"); _file.close(); } } diff --git a/src/FTPPath.cpp b/src/FTPPath.cpp index 37c6973..76395d9 100644 --- a/src/FTPPath.cpp +++ b/src/FTPPath.cpp @@ -63,3 +63,46 @@ String FTPPath::createPath(const std::list &path) { } return new_path; } + +bool FTPPath::isValidFilename(const String &filepath) { + if (filepath.isEmpty()) { + return false; + } + + // Check for path traversal attempts + if (filepath.indexOf("../") != -1 || filepath.startsWith("..")) { + return false; + } + + int lastSlash = filepath.lastIndexOf('/'); + String filename = (lastSlash >= 0) ? filepath.substring(lastSlash + 1) : filepath; + + if (filename.isEmpty()) { + return false; + } + + // Check for invalid characters + const char invalid_chars[] = {'?', '*', ':', '"', '<', '>', '|', '\\'}; + for (char c : invalid_chars) { + if (filename.indexOf(c) != -1) { + return false; + } + } + + // Check for Windows reserved names (case-insensitive) + String upperFilename = filename; + upperFilename.toUpperCase(); + const char *reserved_names[] = {"CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9"}; + for (const char *reserved : reserved_names) { + if (upperFilename == reserved) { + return false; + } + } + + // Check for reasonable length limits + if (filename.length() > 255) { + return false; + } + + return true; +} diff --git a/src/FTPPath.h b/src/FTPPath.h index 4bfccdd..57f1c65 100644 --- a/src/FTPPath.h +++ b/src/FTPPath.h @@ -18,6 +18,7 @@ class FTPPath { static std::list splitPath(String path); static String createPath(const std::list &path); + static bool isValidFilename(const String &filename); private: std::list _Path; diff --git a/src/FTPResponseCodes.h b/src/FTPResponseCodes.h new file mode 100644 index 0000000..dc59ff6 --- /dev/null +++ b/src/FTPResponseCodes.h @@ -0,0 +1,46 @@ +#ifndef FTP_RESPONSE_CODES_H_ +#define FTP_RESPONSE_CODES_H_ + +// FTP Response Code Constants +namespace FTPResponse { +// Success codes +const int DATA_CONNECTION_OPEN = 150; +const int COMMAND_OK = 200; +const int SYSTEM_STATUS = 211; +const int DIRECTORY_STATUS = 212; +const int FILE_STATUS = 213; +const int HELP_MESSAGE = 214; +const int SYSTEM_TYPE = 215; +const int READY = 220; +const int CLOSING = 221; +const int TRANSFER_COMPLETE = 226; +const int PASSIVE_MODE = 227; +const int LOGGED_IN = 230; +const int FILE_ACTION_OK = 250; +const int PATHNAME_CREATED = 257; +const int USER_OK = 331; +const int NEED_PASSWORD = 331; +const int FILE_ACTION_PENDING = 350; + +// Error codes +const int SYNTAX_ERROR = 500; +const int SYNTAX_ERROR_PARAMS = 501; +const int COMMAND_NOT_IMPLEMENTED = 502; +const int BAD_SEQUENCE = 503; +const int COMMAND_NOT_IMPLEMENTED_FOR_PARAMETER = 504; +const int NOT_LOGGED_IN = 530; +const int NEED_ACCOUNT = 532; +const int FILE_ACTION_NOT_TAKEN = 550; +const int PAGE_TYPE_UNKNOWN = 551; +const int EXCEEDED_STORAGE = 552; +const int FILE_NAME_NOT_ALLOWED = 553; +const int TRANSFER_ABORTED = 426; +const int NO_DATA_CONNECTION = 425; +const int CANNOT_OPEN_CONNECTION = 425; +const int CONNECTION_CLOSED = 426; +const int FILE_ACTION_ABORTED = 450; +const int FILE_ACTION_ABORTED_LOCAL_ERROR = 451; +const int INSUFFICIENT_STORAGE = 452; +} // namespace FTPResponse + +#endif diff --git a/test/FTPPath/validation_tests.cpp b/test/FTPPath/validation_tests.cpp new file mode 100644 index 0000000..5fcceed --- /dev/null +++ b/test/FTPPath/validation_tests.cpp @@ -0,0 +1,71 @@ +#include "FTPPath.h" +#include + +void test_valid_filename() { + TEST_ASSERT_TRUE(FTPPath::isValidFilename("file.txt")); + TEST_ASSERT_TRUE(FTPPath::isValidFilename("document.pdf")); + TEST_ASSERT_TRUE(FTPPath::isValidFilename("12345")); + TEST_ASSERT_TRUE(FTPPath::isValidFilename("file_with_underscores.doc")); + TEST_ASSERT_TRUE(FTPPath::isValidFilename("file-with-dashes.txt")); +} + +void test_invalid_filename() { + TEST_ASSERT_FALSE(FTPPath::isValidFilename("")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("file?.txt")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("file*.txt")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("file:name.txt")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("file\"name.txt")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("file.txt")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("file|name.txt")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("file\\name.txt")); +} + +void test_path_with_invalid_filename() { + TEST_ASSERT_FALSE(FTPPath::isValidFilename("/path/file?.txt")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("folder/file*.txt")); + TEST_ASSERT_TRUE(FTPPath::isValidFilename("/folder/file.txt")); +} + +void test_path_traversal_protection() { + TEST_ASSERT_FALSE(FTPPath::isValidFilename("../file.txt")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("../../../etc/passwd")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("..")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("/path/../file.txt")); + TEST_ASSERT_TRUE(FTPPath::isValidFilename("file..txt")); +} + +void test_reserved_names() { + TEST_ASSERT_FALSE(FTPPath::isValidFilename("CON")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("PRN")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("AUX")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("NUL")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("COM1")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("LPT1")); + TEST_ASSERT_FALSE(FTPPath::isValidFilename("con")); // case insensitive + TEST_ASSERT_FALSE(FTPPath::isValidFilename("Com1")); // case insensitive + TEST_ASSERT_TRUE(FTPPath::isValidFilename("console")); // not exact match + TEST_ASSERT_TRUE(FTPPath::isValidFilename("file.txt")); +} + +void test_length_limits() { + String longFilename = String(256, 'a'); // 256 characters + TEST_ASSERT_FALSE(FTPPath::isValidFilename(longFilename)); + + String maxFilename = String(255, 'a'); // 255 characters + TEST_ASSERT_TRUE(FTPPath::isValidFilename(maxFilename)); +} + +void setup() { + UNITY_BEGIN(); + RUN_TEST(test_valid_filename); + RUN_TEST(test_invalid_filename); + RUN_TEST(test_path_with_invalid_filename); + RUN_TEST(test_path_traversal_protection); + RUN_TEST(test_reserved_names); + RUN_TEST(test_length_limits); + UNITY_END(); +} + +void loop() { + // Empty +} From 1412c7b09ec20541a788edfb57f296202c1ff71c Mon Sep 17 00:00:00 2001 From: Joe91 <1015.5@gmx.de> Date: Thu, 30 Apr 2026 19:32:34 +0200 Subject: [PATCH 07/18] String Co-authored-by: Copilot --- src/Commands/LIST.h | 22 ++++++++++++++++++---- src/Commands/MLSD.h | 12 ++++++++++-- src/Commands/NLST.h | 7 +++++-- src/FTPConnection.cpp | 5 +++++ 4 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/Commands/LIST.h b/src/Commands/LIST.h index 2601fa0..342ebc4 100644 --- a/src/Commands/LIST.h +++ b/src/Commands/LIST.h @@ -12,17 +12,31 @@ class LIST : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { + FTPPath listPath = WorkDirectory; + + // 1. Better Argument Parsing + // Look for the first argument that DOES NOT start with '-' + // This correctly skips 'LIST -la /sdcard' flags and finds the path + for (size_t i = 1; i < Line.size(); ++i) { + if (!Line[i].startsWith("-") && !Line[i].isEmpty()) { + listPath.changePath(Line[i]); + break; + } + } + if (!ConnectDataConnection()) { return; } - File dir = _Filesystem->open(WorkDirectory.getPath()); // + File dir = _Filesystem->open(listPath.getPath()); // if (!dir || !dir.isDirectory()) { CloseDataConnection(); - SendResponse(550, "Can't open directory " + WorkDirectory.getPath()); + SendResponse(550, "Can't open directory " + listPath.getPath()); return; } - int cnt = 0; - File f = dir.openNextFile(); + int cnt = 2; + data_println("drwxr-xr-x 1 owner group 0 Jan 01 1970 ."); + data_println("drwxr-xr-x 1 owner group 0 Jan 01 1970 .."); + File f = dir.openNextFile(); while (f) { String filename = f.name(); filename.remove(0, filename.lastIndexOf('/') + 1); diff --git a/src/Commands/MLSD.h b/src/Commands/MLSD.h index 9d6faba..a9e5420 100644 --- a/src/Commands/MLSD.h +++ b/src/Commands/MLSD.h @@ -14,16 +14,24 @@ class MLSD : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { + // Handle arguments: MLSD can take a path or flags + FTPPath listPath = WorkDirectory; + if (Line.size() > 1 && !Line[1].isEmpty() && Line[1] != Line[0] && !Line[1].startsWith("-")) { + // If argument exists, is not empty, is not the command itself, and doesn't start with -, treat it as a path + listPath.changePath(Line[1]); + } + // Flags are ignored for compatibility + if (!ConnectDataConnection()) { return; } - File root = _Filesystem->open(WorkDirectory.getPath(), "r"); + File root = _Filesystem->open(listPath.getPath(), "r"); if (!root || !root.isDirectory()) { root.close(); CloseDataConnection(); - SendResponse(550, "Can't open directory " + WorkDirectory.getPath()); + SendResponse(550, "Can't open directory " + listPath.getPath()); return; } diff --git a/src/Commands/NLST.h b/src/Commands/NLST.h index ca35d95..bc36e82 100644 --- a/src/Commands/NLST.h +++ b/src/Commands/NLST.h @@ -21,11 +21,14 @@ class NLST : public FTPCommand { SendResponse(550, "Can't open directory " + WorkDirectory.getPath()); return; } - int cnt = 0; - File f = dir.openNextFile(); + int cnt = 2; + data_println("."); + data_println(".."); + File f = dir.openNextFile(); while (f) { String filename = f.name(); filename.remove(0, filename.lastIndexOf('/') + 1); + data_println(filename); cnt++; f.close(); f = dir.openNextFile(); diff --git a/src/FTPConnection.cpp b/src/FTPConnection.cpp index c6b4244..22aeb5b 100644 --- a/src/FTPConnection.cpp +++ b/src/FTPConnection.cpp @@ -108,6 +108,7 @@ bool FTPConnection::handle() { logPrintlnD(_Line); #endif String command = _LineSplited[0]; + command.toUpperCase(); // This commands are always possible: if (command == "SYST") { @@ -151,6 +152,10 @@ bool FTPConnection::handle() { _Client.println("226 Data connection closed"); _Line = ""; return true; + } else if (command == "CLNT") { + _Client.println("200 Ok"); + _Line = ""; + return true; } // Logged in? From 4592284b639c686aa594439541fead7369ed51d2 Mon Sep 17 00:00:00 2001 From: Joe91 <1015.5@gmx.de> Date: Thu, 30 Apr 2026 22:35:43 +0200 Subject: [PATCH 08/18] some small tweaks and fixes --- src/Commands/LIST.h | 17 ++++++++++------- src/Commands/MLSD.h | 16 +++++++++++----- src/FTPConnection.cpp | 13 +++++++++++-- src/common.h | 25 +++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 14 deletions(-) diff --git a/src/Commands/LIST.h b/src/Commands/LIST.h index 342ebc4..eb34804 100644 --- a/src/Commands/LIST.h +++ b/src/Commands/LIST.h @@ -14,13 +14,16 @@ class LIST : public FTPCommand { void run(FTPPath &WorkDirectory, const std::vector &Line) override { FTPPath listPath = WorkDirectory; - // 1. Better Argument Parsing - // Look for the first argument that DOES NOT start with '-' - // This correctly skips 'LIST -la /sdcard' flags and finds the path - for (size_t i = 1; i < Line.size(); ++i) { - if (!Line[i].startsWith("-") && !Line[i].isEmpty()) { - listPath.changePath(Line[i]); - break; + // 1. Check if we have arguments + if (Line.size() > 1) { + String args = Line[1]; + args.trim(); // Modifies 'args' in place + + if (!args.isEmpty()) { + String path = ExtractPathFromOptions(args); + if (!path.isEmpty()) { + listPath.changePath(path); + } } } diff --git a/src/Commands/MLSD.h b/src/Commands/MLSD.h index a9e5420..a6a3ed0 100644 --- a/src/Commands/MLSD.h +++ b/src/Commands/MLSD.h @@ -14,13 +14,19 @@ class MLSD : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { - // Handle arguments: MLSD can take a path or flags FTPPath listPath = WorkDirectory; - if (Line.size() > 1 && !Line[1].isEmpty() && Line[1] != Line[0] && !Line[1].startsWith("-")) { - // If argument exists, is not empty, is not the command itself, and doesn't start with -, treat it as a path - listPath.changePath(Line[1]); + // 1. Check if we have arguments + if (Line.size() > 1) { + String args = Line[1]; + args.trim(); // Modifies 'args' in place + + if (!args.isEmpty()) { + String path = ExtractPathFromOptions(args); + if (!path.isEmpty()) { + listPath.changePath(path); + } + } } - // Flags are ignored for compatibility if (!ConnectDataConnection()) { return; diff --git a/src/FTPConnection.cpp b/src/FTPConnection.cpp index 22aeb5b..4140e91 100644 --- a/src/FTPConnection.cpp +++ b/src/FTPConnection.cpp @@ -77,9 +77,18 @@ FTPConnection::~FTPConnection() { bool FTPConnection::readUntilLineEnd() { while (_Client.available()) { char c = _Client.read(); + if (c == '\r') + continue; // Ignore carriage returns if (c == '\n') { - uint32_t index_separator = _Line.indexOf(' '); - _LineSplited = {_Line.substring(0, index_separator), _Line.substring(index_separator + 1, _Line.length())}; + int index_separator = _Line.indexOf(' '); + if (index_separator != -1) { + // Correctly split into Command and Arguments + _LineSplited = {_Line.substring(0, index_separator), _Line.substring(index_separator + 1)}; + } else { + // No space found? Arguments are empty. + _LineSplited = {_Line, ""}; + } + _Line = ""; // Reset buffer for next line return true; } if (c >= 32) { diff --git a/src/common.h b/src/common.h index 4b15edf..23b14ed 100644 --- a/src/common.h +++ b/src/common.h @@ -2,6 +2,7 @@ #define COMMON_H_ #include +#include #include #include @@ -20,4 +21,28 @@ template T Split(String str, char parser) { return str_array; } +inline String ExtractPathFromOptions(const String &args) { + if (args.isEmpty()) { + return String(); + } + std::vector tokens = Split>(args, ' '); + tokens.erase(std::remove_if(tokens.begin(), tokens.end(), + [](const String &s) { + return s.isEmpty(); + }), + tokens.end()); + + for (size_t i = 0; i < tokens.size(); ++i) { + if (!tokens[i].startsWith("-")) { + String path = tokens[i]; + for (size_t j = i + 1; j < tokens.size(); ++j) { + path += " "; + path += tokens[j]; + } + return path; + } + } + return String(); +} + #endif From d8784da26e5e85cb0913f9cb1ab588715c4778e4 Mon Sep 17 00:00:00 2001 From: Joe91 <1015.5@gmx.de> Date: Thu, 30 Apr 2026 22:42:43 +0200 Subject: [PATCH 09/18] Revert "Merge branch 'security_and_names'" This reverts commit 90fe1798b166f9714164f887e8f9a1d42b376278, reversing changes made to f0aa9b50eb99c3375e4055152f23181b5bfa7fc8. --- README.md | 19 +-------- src/Commands/CDUP.h | 3 +- src/Commands/CWD.h | 15 +------ src/Commands/DELE.h | 15 ++----- src/Commands/LIST.h | 15 ++----- src/Commands/MKD.h | 15 ++----- src/Commands/NLST.h | 15 ++----- src/Commands/PORT.h | 2 +- src/Commands/PWD.h | 3 +- src/Commands/RETR.h | 13 +----- src/Commands/RMD.h | 15 ++----- src/Commands/RNFR_RNTO.h | 27 +++--------- src/Commands/STOR.h | 15 ++----- src/Commands/TYPE.h | 7 ++- src/FTPCommand.h | 15 +++---- src/FTPPath.cpp | 43 ------------------- src/FTPPath.h | 1 - src/FTPResponseCodes.h | 46 -------------------- test/FTPPath/validation_tests.cpp | 71 ------------------------------- 19 files changed, 42 insertions(+), 313 deletions(-) delete mode 100644 src/FTPResponseCodes.h delete mode 100644 test/FTPPath/validation_tests.cpp diff --git a/README.md b/README.md index 316c7bb..f372065 100644 --- a/README.md +++ b/README.md @@ -3,14 +3,6 @@ This library will provide a simple and modern FTP server for your ESP32 or ESP8266 device. You can setup multiple users and mutliple filesystems (SD-Card, MMC-Card or/and SPIFFS). -## Security Features - -This library includes comprehensive input validation and security protections: -- **Filename validation**: Prevents illegal characters in filenames (?, *, :, ", <, >, |, \) -- **Parameter validation**: Validates command syntax and required parameters -- **Path traversal protection**: Helps prevent directory traversal attacks -- **Error handling**: Proper FTP response codes for security-related errors - ## Examples In the example folder you can find a very simple usage of the FTP server. You just need to setup the users, add the filesystems which you want to use, and call the handle function in the loop. @@ -23,7 +15,6 @@ Currently all kind of simple commands are known to the server: * DELE * LISST * MKD -* PASV * PORT * PWD * RETR @@ -42,12 +33,4 @@ Currently all kind of simple commands are known to the server: Some commands are still missing, if you need them create a ticket :) -**Security Improvements Completed:** -- Input validation for all critical commands (DELE, MKD, RETR, RMD, RNFR, RNTO, STOR) -- Filename validation with illegal character detection -- Proper error responses with standard FTP codes - -**Still missing:** -- Additional FTP commands: FEAT, MDTM, SIZE, REST -- Rate limiting and connection throttling -- Enhanced logging capabilities +Currently just the active mode is supported. For the passive mode you need to wait until version 1.0.0. diff --git a/src/Commands/CDUP.h b/src/Commands/CDUP.h index 68a8a54..a88aba7 100644 --- a/src/Commands/CDUP.h +++ b/src/Commands/CDUP.h @@ -4,7 +4,6 @@ #include #include "../FTPCommand.h" -#include "../FTPResponseCodes.h" class CDUP : public FTPCommand { public: @@ -13,7 +12,7 @@ class CDUP : public FTPCommand { void run(FTPPath &WorkDirectory, const std::vector &Line) override { WorkDirectory.goPathUp(); - SendResponse(FTPResponse::FILE_ACTION_OK, "Ok. Current directory is " + WorkDirectory.getPath()); + SendResponse(250, "Ok. Current directory is " + WorkDirectory.getPath()); } }; diff --git a/src/Commands/CWD.h b/src/Commands/CWD.h index 5fd991a..5071d1c 100644 --- a/src/Commands/CWD.h +++ b/src/Commands/CWD.h @@ -4,7 +4,6 @@ #include #include "../FTPCommand.h" -#include "../FTPResponseCodes.h" class CWD : public FTPCommand { public: @@ -12,28 +11,18 @@ class CWD : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { - if (Line.size() < 2) { - SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); - return; - } - FTPPath path = WorkDirectory; if (Line[1] == "..") { path.goPathUp(); } else { - // Validate path for illegal characters - if (!FTPPath::isValidFilename(Line[1])) { - SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename or path"); - return; - } path.changePath(Line[1]); } File dir = _Filesystem->open(path.getPath()); if (dir.isDirectory()) { WorkDirectory = path; - SendResponse(FTPResponse::FILE_ACTION_OK, "Ok. Current directory is " + WorkDirectory.getPath()); + SendResponse(250, "Ok. Current directory is " + WorkDirectory.getPath()); } else { - SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "Directory does not exist"); + SendResponse(550, "Directory does not exist"); } } }; diff --git a/src/Commands/DELE.h b/src/Commands/DELE.h index da240cd..c8ec30c 100644 --- a/src/Commands/DELE.h +++ b/src/Commands/DELE.h @@ -4,7 +4,6 @@ #include #include "../FTPCommand.h" -#include "../FTPResponseCodes.h" class DELE : public FTPCommand { public: @@ -12,23 +11,15 @@ class DELE : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { - if (Line.size() < 2) { - SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); - return; - } - if (!FTPPath::isValidFilename(Line[1])) { - SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename"); - return; - } String filepath = WorkDirectory.getFilePath(Line[1]); if (!_Filesystem->exists(filepath)) { - SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "File " + filepath + " not found"); + SendResponse(550, "File " + filepath + " not found"); return; } if (_Filesystem->remove(filepath)) { - SendResponse(FTPResponse::FILE_ACTION_OK, " Deleted \"" + filepath + "\""); + SendResponse(250, " Deleted \"" + filepath + "\""); } else { - SendResponse(FTPResponse::FILE_ACTION_ABORTED, "Can't delete \"" + filepath + "\""); + SendResponse(450, "Can't delete \"" + filepath + "\""); } } }; diff --git a/src/Commands/LIST.h b/src/Commands/LIST.h index 84c4582..2601fa0 100644 --- a/src/Commands/LIST.h +++ b/src/Commands/LIST.h @@ -4,7 +4,6 @@ #include #include "../FTPCommand.h" -#include "../FTPResponseCodes.h" #include "../common.h" class LIST : public FTPCommand { @@ -13,21 +12,13 @@ class LIST : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { - // Validate optional path parameter - if (Line.size() > 1 && !FTPPath::isValidFilename(Line[1])) { - SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename or path"); - return; - } - if (!ConnectDataConnection()) { return; } - - String listPath = (Line.size() > 1) ? WorkDirectory.getFilePath(Line[1]) : WorkDirectory.getPath(); - File dir = _Filesystem->open(listPath); // + File dir = _Filesystem->open(WorkDirectory.getPath()); // if (!dir || !dir.isDirectory()) { CloseDataConnection(); - SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "Can't open directory " + WorkDirectory.getPath()); + SendResponse(550, "Can't open directory " + WorkDirectory.getPath()); return; } int cnt = 0; @@ -52,7 +43,7 @@ class LIST : public FTPCommand { f = dir.openNextFile(); } CloseDataConnection(); - SendResponse(FTPResponse::TRANSFER_COMPLETE, String(cnt) + " matches total"); + SendResponse(226, String(cnt) + " matches total"); } }; diff --git a/src/Commands/MKD.h b/src/Commands/MKD.h index 363c58d..e1c1765 100644 --- a/src/Commands/MKD.h +++ b/src/Commands/MKD.h @@ -4,7 +4,6 @@ #include #include "../FTPCommand.h" -#include "../FTPResponseCodes.h" class MKD : public FTPCommand { public: @@ -12,23 +11,15 @@ class MKD : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { - if (Line.size() < 2) { - SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); - return; - } - if (!FTPPath::isValidFilename(Line[1])) { - SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename"); - return; - } String filepath = WorkDirectory.getFilePath(Line[1]); if (_Filesystem->exists(filepath)) { - SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "Can't create \"" + filepath + "\", Directory exists"); + SendResponse(521, "Can't create \"" + filepath + "\", Directory exists"); return; } if (_Filesystem->mkdir(filepath)) { - SendResponse(FTPResponse::PATHNAME_CREATED, "\"" + filepath + "\" created"); + SendResponse(257, "\"" + filepath + "\" created"); } else { - SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "Can't create \"" + filepath + "\""); + SendResponse(550, "Can't create \"" + filepath + "\""); } } }; diff --git a/src/Commands/NLST.h b/src/Commands/NLST.h index e484290..ca35d95 100644 --- a/src/Commands/NLST.h +++ b/src/Commands/NLST.h @@ -4,7 +4,6 @@ #include #include "../FTPCommand.h" -#include "../FTPResponseCodes.h" #include "../common.h" class NLST : public FTPCommand { @@ -13,21 +12,13 @@ class NLST : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { - // Validate optional path parameter - if (Line.size() > 1 && !FTPPath::isValidFilename(Line[1])) { - SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename or path"); - return; - } - if (!ConnectDataConnection()) { return; } - - String listPath = (Line.size() > 1) ? WorkDirectory.getFilePath(Line[1]) : WorkDirectory.getPath(); - File dir = _Filesystem->open(listPath); // + File dir = _Filesystem->open(WorkDirectory.getPath()); // if (!dir || !dir.isDirectory()) { CloseDataConnection(); - SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "Can't open directory " + WorkDirectory.getPath()); + SendResponse(550, "Can't open directory " + WorkDirectory.getPath()); return; } int cnt = 0; @@ -40,7 +31,7 @@ class NLST : public FTPCommand { f = dir.openNextFile(); } CloseDataConnection(); - SendResponse(FTPResponse::TRANSFER_COMPLETE, String(cnt) + " matches total"); + SendResponse(226, String(cnt) + " matches total"); } }; diff --git a/src/Commands/PORT.h b/src/Commands/PORT.h index 7d634b2..f047df1 100644 --- a/src/Commands/PORT.h +++ b/src/Commands/PORT.h @@ -25,7 +25,7 @@ class PORT : public FTPCommand { (*_DataAddress)[i] = connection_details[i].toInt(); } *_DataPort = connection_details[4].toInt() * 256 + connection_details[5].toInt(); - SendResponse(FTPResponse::COMMAND_OK, "PORT command successful"); + SendResponse(200, "PORT command successful"); } }; diff --git a/src/Commands/PWD.h b/src/Commands/PWD.h index bd04651..9aa8dfc 100644 --- a/src/Commands/PWD.h +++ b/src/Commands/PWD.h @@ -4,7 +4,6 @@ #include #include "../FTPCommand.h" -#include "../FTPResponseCodes.h" class PWD : public FTPCommand { public: @@ -12,7 +11,7 @@ class PWD : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { - SendResponse(FTPResponse::PATHNAME_CREATED, "\"" + WorkDirectory.getPath() + "\" is your current directory"); + SendResponse(257, "\"" + WorkDirectory.getPath() + "\" is your current directory"); } }; diff --git a/src/Commands/RETR.h b/src/Commands/RETR.h index 9e92ac7..53075b4 100644 --- a/src/Commands/RETR.h +++ b/src/Commands/RETR.h @@ -4,7 +4,6 @@ #include #include "../FTPCommand.h" -#include "../FTPResponseCodes.h" #include "../common.h" #define FTP_BUF_SIZE 4096 @@ -18,14 +17,6 @@ class RETR : public FTPCommandTransfer { if (trasferInProgress()) { return; } - if (Line.size() < 2) { - SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); - return; - } - if (!FTPPath::isValidFilename(Line[1])) { - SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename"); - return; - } if (!ConnectDataConnection()) { return; } @@ -33,7 +24,7 @@ class RETR : public FTPCommandTransfer { _file = _Filesystem->open(path); if (!_file || _file.isDirectory()) { CloseDataConnection(); - SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "Can't open " + path); + SendResponse(550, "Can't open " + path); return; } workOnData(); @@ -47,7 +38,7 @@ class RETR : public FTPCommandTransfer { return; } CloseDataConnection(); - SendResponse(FTPResponse::TRANSFER_COMPLETE, "File successfully transferred"); + SendResponse(226, "File successfully transferred"); _file.close(); } diff --git a/src/Commands/RMD.h b/src/Commands/RMD.h index a7e2ada..dad6f52 100644 --- a/src/Commands/RMD.h +++ b/src/Commands/RMD.h @@ -4,7 +4,6 @@ #include #include "../FTPCommand.h" -#include "../FTPResponseCodes.h" class RMD : public FTPCommand { public: @@ -12,23 +11,15 @@ class RMD : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { - if (Line.size() < 2) { - SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); - return; - } - if (!FTPPath::isValidFilename(Line[1])) { - SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename"); - return; - } String filepath = WorkDirectory.getFilePath(Line[1]); if (!_Filesystem->exists(filepath)) { - SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "Folder " + filepath + " not found"); + SendResponse(550, "Folder " + filepath + " not found"); return; } if (_Filesystem->rmdir(filepath)) { - SendResponse(FTPResponse::FILE_ACTION_OK, " Deleted \"" + filepath + "\""); + SendResponse(250, " Deleted \"" + filepath + "\""); } else { - SendResponse(FTPResponse::FILE_ACTION_ABORTED, "Can't delete \"" + filepath + "\""); + SendResponse(450, "Can't delete \"" + filepath + "\""); } } }; diff --git a/src/Commands/RNFR_RNTO.h b/src/Commands/RNFR_RNTO.h index 034b168..6a66e9d 100644 --- a/src/Commands/RNFR_RNTO.h +++ b/src/Commands/RNFR_RNTO.h @@ -4,7 +4,6 @@ #include #include "../FTPCommand.h" -#include "../FTPResponseCodes.h" class RNFR_RNTO : public FTPCommand { public: @@ -20,44 +19,30 @@ class RNFR_RNTO : public FTPCommand { } void from(const FTPPath &WorkDirectory, const std::vector &Line) { - if (Line.size() < 2) { - SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); - return; - } String filepath = WorkDirectory.getFilePath(Line[1]); if (!_Filesystem->exists(filepath)) { - SendResponse(FTPResponse::FILE_ACTION_NOT_TAKEN, "File " + Line[1] + " not found"); + SendResponse(550, "File " + Line[1] + " not found"); return; } _fromSet = true; _from = filepath; - SendResponse(FTPResponse::FILE_ACTION_PENDING, "RNFR accepted - file exists, ready for destination"); + SendResponse(350, "RNFR accepted - file exists, ready for destination"); } void to(const FTPPath &WorkDirectory, const std::vector &Line) { - if (Line.size() < 2) { - SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); - return; - } if (!_fromSet) { - SendResponse(FTPResponse::BAD_SEQUENCE, "Need RNFR before RNTO"); - return; - } - if (!FTPPath::isValidFilename(Line[1])) { - SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename"); - _fromSet = false; - _from = ""; + SendResponse(503, "Need RNFR before RNTO"); return; } String filepath = WorkDirectory.getFilePath(Line[1]); if (_Filesystem->exists(filepath)) { - SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "File " + Line[1] + " already exists"); + SendResponse(553, "File " + Line[1] + " already exists"); return; } if (_Filesystem->rename(_from, filepath)) { - SendResponse(FTPResponse::FILE_ACTION_OK, "File successfully renamed or moved"); + SendResponse(250, "File successfully renamed or moved"); } else { - SendResponse(FTPResponse::FILE_ACTION_ABORTED_LOCAL_ERROR, "Rename/move failure"); + SendResponse(451, "Rename/move failure"); } _fromSet = false; _from = ""; diff --git a/src/Commands/STOR.h b/src/Commands/STOR.h index d80c51b..6f3e022 100644 --- a/src/Commands/STOR.h +++ b/src/Commands/STOR.h @@ -4,7 +4,6 @@ #include #include "../FTPCommand.h" -#include "../FTPResponseCodes.h" #include "../common.h" #define FTP_BUF_SIZE 4096 @@ -18,21 +17,13 @@ class STOR : public FTPCommandTransfer { if (trasferInProgress()) { return; } - if (Line.size() < 2) { - SendResponse(FTPResponse::SYNTAX_ERROR_PARAMS, "Syntax error in parameters"); - return; - } - if (!FTPPath::isValidFilename(Line[1])) { - SendResponse(FTPResponse::FILE_NAME_NOT_ALLOWED, "Illegal filename"); - return; - } if (!ConnectDataConnection()) { return; } _ftpFsFilePath = WorkDirectory.getFilePath(Line[1]); _file = _Filesystem->open(_ftpFsFilePath, "w"); if (!_file) { - SendResponse(FTPResponse::FILE_ACTION_ABORTED_LOCAL_ERROR, "Can't open/create " + _ftpFsFilePath); + SendResponse(451, "Can't open/create " + _ftpFsFilePath); CloseDataConnection(); return; } @@ -48,14 +39,14 @@ class STOR : public FTPCommandTransfer { _file.close(); this->_Filesystem->remove(_ftpFsFilePath.c_str()); - SendResponse(FTPResponse::EXCEEDED_STORAGE, "Error occured while STORing"); + SendResponse(552, "Error occured while STORing"); CloseDataConnection(); } return; } - SendResponse(FTPResponse::TRANSFER_COMPLETE, "File successfully transferred"); + SendResponse(226, "File successfully transferred"); CloseDataConnection(); _file.close(); } diff --git a/src/Commands/TYPE.h b/src/Commands/TYPE.h index c5224b9..522e2ab 100644 --- a/src/Commands/TYPE.h +++ b/src/Commands/TYPE.h @@ -4,7 +4,6 @@ #include #include "../FTPCommand.h" -#include "../FTPResponseCodes.h" class TYPE : public FTPCommand { public: @@ -13,13 +12,13 @@ class TYPE : public FTPCommand { void run(FTPPath &WorkDirectory, const std::vector &Line) override { if (Line[1] == "A") { - SendResponse(FTPResponse::COMMAND_OK, "TYPE is now ASCII"); + SendResponse(200, "TYPE is now ASCII"); return; } else if (Line[1] == "I") { - SendResponse(FTPResponse::COMMAND_OK, "TYPE is now 8-bit binary"); + SendResponse(200, "TYPE is now 8-bit binary"); return; } - SendResponse(FTPResponse::COMMAND_NOT_IMPLEMENTED_FOR_PARAMETER, "Unknown TYPE"); + SendResponse(504, "Unknow TYPE"); } }; diff --git a/src/FTPCommand.h b/src/FTPCommand.h index c05ab56..9c8edbc 100644 --- a/src/FTPCommand.h +++ b/src/FTPCommand.h @@ -8,7 +8,6 @@ #include "FTPFilesystem.h" #include "FTPPath.h" -#include "FTPResponseCodes.h" class FTPCommand { public: @@ -43,7 +42,7 @@ class FTPCommand { } if (_PassiveMode != 0 && *_PassiveMode) { if (_PassiveServer == 0 || *_PassiveServer == 0) { - SendResponse(FTPResponse::NO_DATA_CONNECTION, "No passive server"); + SendResponse(425, "No passive server"); return false; } WiFiServer *server = *_PassiveServer; @@ -56,28 +55,28 @@ class FTPCommand { } if (!server->hasClient()) { StopPassiveServer(); - SendResponse(FTPResponse::NO_DATA_CONNECTION, "No data connection"); + SendResponse(425, "No data connection"); return false; } WiFiClient client = server->accept(); if (!client) { StopPassiveServer(); - SendResponse(FTPResponse::NO_DATA_CONNECTION, "No data connection"); + SendResponse(425, "No data connection"); return false; } *_DataConnection = client; StopPassiveServer(); *_PassiveMode = false; - SendResponse(FTPResponse::DATA_CONNECTION_OPEN, "Accepted data connection"); + SendResponse(150, "Accepted data connection"); return true; } _DataConnection->connect(*_DataAddress, *_DataPort); if (!_DataConnection->connected()) { _DataConnection->stop(); - SendResponse(FTPResponse::NO_DATA_CONNECTION, "No data connection"); + SendResponse(425, "No data connection"); return false; } - SendResponse(FTPResponse::DATA_CONNECTION_OPEN, "Accepted data connection"); + SendResponse(150, "Accepted data connection"); return true; } @@ -165,7 +164,7 @@ class FTPCommandTransfer : public FTPCommand { void abort() { if (_file) { CloseDataConnection(); - SendResponse(FTPResponse::CONNECTION_CLOSED, "Transfer aborted"); + SendResponse(426, "Transfer aborted"); _file.close(); } } diff --git a/src/FTPPath.cpp b/src/FTPPath.cpp index 76395d9..37c6973 100644 --- a/src/FTPPath.cpp +++ b/src/FTPPath.cpp @@ -63,46 +63,3 @@ String FTPPath::createPath(const std::list &path) { } return new_path; } - -bool FTPPath::isValidFilename(const String &filepath) { - if (filepath.isEmpty()) { - return false; - } - - // Check for path traversal attempts - if (filepath.indexOf("../") != -1 || filepath.startsWith("..")) { - return false; - } - - int lastSlash = filepath.lastIndexOf('/'); - String filename = (lastSlash >= 0) ? filepath.substring(lastSlash + 1) : filepath; - - if (filename.isEmpty()) { - return false; - } - - // Check for invalid characters - const char invalid_chars[] = {'?', '*', ':', '"', '<', '>', '|', '\\'}; - for (char c : invalid_chars) { - if (filename.indexOf(c) != -1) { - return false; - } - } - - // Check for Windows reserved names (case-insensitive) - String upperFilename = filename; - upperFilename.toUpperCase(); - const char *reserved_names[] = {"CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9"}; - for (const char *reserved : reserved_names) { - if (upperFilename == reserved) { - return false; - } - } - - // Check for reasonable length limits - if (filename.length() > 255) { - return false; - } - - return true; -} diff --git a/src/FTPPath.h b/src/FTPPath.h index 57f1c65..4bfccdd 100644 --- a/src/FTPPath.h +++ b/src/FTPPath.h @@ -18,7 +18,6 @@ class FTPPath { static std::list splitPath(String path); static String createPath(const std::list &path); - static bool isValidFilename(const String &filename); private: std::list _Path; diff --git a/src/FTPResponseCodes.h b/src/FTPResponseCodes.h deleted file mode 100644 index dc59ff6..0000000 --- a/src/FTPResponseCodes.h +++ /dev/null @@ -1,46 +0,0 @@ -#ifndef FTP_RESPONSE_CODES_H_ -#define FTP_RESPONSE_CODES_H_ - -// FTP Response Code Constants -namespace FTPResponse { -// Success codes -const int DATA_CONNECTION_OPEN = 150; -const int COMMAND_OK = 200; -const int SYSTEM_STATUS = 211; -const int DIRECTORY_STATUS = 212; -const int FILE_STATUS = 213; -const int HELP_MESSAGE = 214; -const int SYSTEM_TYPE = 215; -const int READY = 220; -const int CLOSING = 221; -const int TRANSFER_COMPLETE = 226; -const int PASSIVE_MODE = 227; -const int LOGGED_IN = 230; -const int FILE_ACTION_OK = 250; -const int PATHNAME_CREATED = 257; -const int USER_OK = 331; -const int NEED_PASSWORD = 331; -const int FILE_ACTION_PENDING = 350; - -// Error codes -const int SYNTAX_ERROR = 500; -const int SYNTAX_ERROR_PARAMS = 501; -const int COMMAND_NOT_IMPLEMENTED = 502; -const int BAD_SEQUENCE = 503; -const int COMMAND_NOT_IMPLEMENTED_FOR_PARAMETER = 504; -const int NOT_LOGGED_IN = 530; -const int NEED_ACCOUNT = 532; -const int FILE_ACTION_NOT_TAKEN = 550; -const int PAGE_TYPE_UNKNOWN = 551; -const int EXCEEDED_STORAGE = 552; -const int FILE_NAME_NOT_ALLOWED = 553; -const int TRANSFER_ABORTED = 426; -const int NO_DATA_CONNECTION = 425; -const int CANNOT_OPEN_CONNECTION = 425; -const int CONNECTION_CLOSED = 426; -const int FILE_ACTION_ABORTED = 450; -const int FILE_ACTION_ABORTED_LOCAL_ERROR = 451; -const int INSUFFICIENT_STORAGE = 452; -} // namespace FTPResponse - -#endif diff --git a/test/FTPPath/validation_tests.cpp b/test/FTPPath/validation_tests.cpp deleted file mode 100644 index 5fcceed..0000000 --- a/test/FTPPath/validation_tests.cpp +++ /dev/null @@ -1,71 +0,0 @@ -#include "FTPPath.h" -#include - -void test_valid_filename() { - TEST_ASSERT_TRUE(FTPPath::isValidFilename("file.txt")); - TEST_ASSERT_TRUE(FTPPath::isValidFilename("document.pdf")); - TEST_ASSERT_TRUE(FTPPath::isValidFilename("12345")); - TEST_ASSERT_TRUE(FTPPath::isValidFilename("file_with_underscores.doc")); - TEST_ASSERT_TRUE(FTPPath::isValidFilename("file-with-dashes.txt")); -} - -void test_invalid_filename() { - TEST_ASSERT_FALSE(FTPPath::isValidFilename("")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("file?.txt")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("file*.txt")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("file:name.txt")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("file\"name.txt")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("file.txt")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("file|name.txt")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("file\\name.txt")); -} - -void test_path_with_invalid_filename() { - TEST_ASSERT_FALSE(FTPPath::isValidFilename("/path/file?.txt")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("folder/file*.txt")); - TEST_ASSERT_TRUE(FTPPath::isValidFilename("/folder/file.txt")); -} - -void test_path_traversal_protection() { - TEST_ASSERT_FALSE(FTPPath::isValidFilename("../file.txt")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("../../../etc/passwd")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("..")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("/path/../file.txt")); - TEST_ASSERT_TRUE(FTPPath::isValidFilename("file..txt")); -} - -void test_reserved_names() { - TEST_ASSERT_FALSE(FTPPath::isValidFilename("CON")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("PRN")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("AUX")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("NUL")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("COM1")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("LPT1")); - TEST_ASSERT_FALSE(FTPPath::isValidFilename("con")); // case insensitive - TEST_ASSERT_FALSE(FTPPath::isValidFilename("Com1")); // case insensitive - TEST_ASSERT_TRUE(FTPPath::isValidFilename("console")); // not exact match - TEST_ASSERT_TRUE(FTPPath::isValidFilename("file.txt")); -} - -void test_length_limits() { - String longFilename = String(256, 'a'); // 256 characters - TEST_ASSERT_FALSE(FTPPath::isValidFilename(longFilename)); - - String maxFilename = String(255, 'a'); // 255 characters - TEST_ASSERT_TRUE(FTPPath::isValidFilename(maxFilename)); -} - -void setup() { - UNITY_BEGIN(); - RUN_TEST(test_valid_filename); - RUN_TEST(test_invalid_filename); - RUN_TEST(test_path_with_invalid_filename); - RUN_TEST(test_path_traversal_protection); - RUN_TEST(test_reserved_names); - RUN_TEST(test_length_limits); - UNITY_END(); -} - -void loop() { - // Empty -} From 8a8458c9c7086b2b46abf2730936de8699dfd267 Mon Sep 17 00:00:00 2001 From: Joe91 <1015.5@gmx.de> Date: Thu, 30 Apr 2026 23:15:11 +0200 Subject: [PATCH 10/18] add ftp-return-names --- src/Commands/CDUP.h | 3 ++- src/Commands/CWD.h | 5 +++-- src/Commands/DELE.h | 7 +++--- src/Commands/LIST.h | 5 +++-- src/Commands/MKD.h | 7 +++--- src/Commands/MLSD.h | 5 +++-- src/Commands/NLST.h | 5 +++-- src/Commands/PASV.h | 5 +++-- src/Commands/PORT.h | 3 ++- src/Commands/PWD.h | 2 +- src/Commands/RETR.h | 5 +++-- src/Commands/RMD.h | 7 +++--- src/Commands/RNFR_RNTO.h | 13 ++++++----- src/Commands/STAT.h | 5 +++-- src/Commands/STOR.h | 6 ++--- src/Commands/TYPE.h | 6 ++--- src/FTPCommand.h | 19 ++++++++-------- src/FTPResponseCodes.h | 47 ++++++++++++++++++++++++++++++++++++++++ 18 files changed, 108 insertions(+), 47 deletions(-) create mode 100644 src/FTPResponseCodes.h diff --git a/src/Commands/CDUP.h b/src/Commands/CDUP.h index a88aba7..da8f210 100644 --- a/src/Commands/CDUP.h +++ b/src/Commands/CDUP.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" class CDUP : public FTPCommand { public: @@ -12,7 +13,7 @@ class CDUP : public FTPCommand { void run(FTPPath &WorkDirectory, const std::vector &Line) override { WorkDirectory.goPathUp(); - SendResponse(250, "Ok. Current directory is " + WorkDirectory.getPath()); + SendResponse(FtpCodes::COMMAND_OK, "Ok. Current directory is " + WorkDirectory.getPath()); } }; diff --git a/src/Commands/CWD.h b/src/Commands/CWD.h index 5071d1c..29641e3 100644 --- a/src/Commands/CWD.h +++ b/src/Commands/CWD.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" class CWD : public FTPCommand { public: @@ -20,9 +21,9 @@ class CWD : public FTPCommand { File dir = _Filesystem->open(path.getPath()); if (dir.isDirectory()) { WorkDirectory = path; - SendResponse(250, "Ok. Current directory is " + WorkDirectory.getPath()); + SendResponse(FtpCodes::COMMAND_OK, "Ok. Current directory is " + WorkDirectory.getPath()); } else { - SendResponse(550, "Directory does not exist"); + SendResponse(FtpCodes::FILE_ACTION_NOT_TAKEN, "Directory does not exist"); } } }; diff --git a/src/Commands/DELE.h b/src/Commands/DELE.h index c8ec30c..57a15a0 100644 --- a/src/Commands/DELE.h +++ b/src/Commands/DELE.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" class DELE : public FTPCommand { public: @@ -13,13 +14,13 @@ class DELE : public FTPCommand { void run(FTPPath &WorkDirectory, const std::vector &Line) override { String filepath = WorkDirectory.getFilePath(Line[1]); if (!_Filesystem->exists(filepath)) { - SendResponse(550, "File " + filepath + " not found"); + SendResponse(FtpCodes::FILE_NOT_FOUND, "File " + filepath + " not found"); return; } if (_Filesystem->remove(filepath)) { - SendResponse(250, " Deleted \"" + filepath + "\""); + SendResponse(FtpCodes::COMMAND_OK, " Deleted \"" + filepath + "\""); } else { - SendResponse(450, "Can't delete \"" + filepath + "\""); + SendResponse(FtpCodes::FILE_ACTION_ABORTED, "Can't delete \"" + filepath + "\""); } } }; diff --git a/src/Commands/LIST.h b/src/Commands/LIST.h index eb34804..f8b9486 100644 --- a/src/Commands/LIST.h +++ b/src/Commands/LIST.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" #include "../common.h" class LIST : public FTPCommand { @@ -33,7 +34,7 @@ class LIST : public FTPCommand { File dir = _Filesystem->open(listPath.getPath()); // if (!dir || !dir.isDirectory()) { CloseDataConnection(); - SendResponse(550, "Can't open directory " + listPath.getPath()); + SendResponse(FtpCodes::FILE_NOT_FOUND, "Can't open directory " + listPath.getPath()); return; } int cnt = 2; @@ -60,7 +61,7 @@ class LIST : public FTPCommand { f = dir.openNextFile(); } CloseDataConnection(); - SendResponse(226, String(cnt) + " matches total"); + SendResponse(FtpCodes::TRANSFER_COMPLETE, String(cnt) + " matches total"); } }; diff --git a/src/Commands/MKD.h b/src/Commands/MKD.h index e1c1765..ca2ccda 100644 --- a/src/Commands/MKD.h +++ b/src/Commands/MKD.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" class MKD : public FTPCommand { public: @@ -13,13 +14,13 @@ class MKD : public FTPCommand { void run(FTPPath &WorkDirectory, const std::vector &Line) override { String filepath = WorkDirectory.getFilePath(Line[1]); if (_Filesystem->exists(filepath)) { - SendResponse(521, "Can't create \"" + filepath + "\", Directory exists"); + SendResponse(FtpCodes::FILE_ACTION_NOT_TAKEN, "Can't create \"" + filepath + "\", Directory exists"); return; } if (_Filesystem->mkdir(filepath)) { - SendResponse(257, "\"" + filepath + "\" created"); + SendResponse(FtpCodes::PATHNAME_CREATED, "\"" + filepath + "\" created"); } else { - SendResponse(550, "Can't create \"" + filepath + "\""); + SendResponse(FtpCodes::FILE_ACTION_NOT_TAKEN, "Can't create \"" + filepath + "\""); } } }; diff --git a/src/Commands/MLSD.h b/src/Commands/MLSD.h index a6a3ed0..c56eabf 100644 --- a/src/Commands/MLSD.h +++ b/src/Commands/MLSD.h @@ -5,6 +5,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" #include "../common.h" #include @@ -37,7 +38,7 @@ class MLSD : public FTPCommand { if (!root || !root.isDirectory()) { root.close(); CloseDataConnection(); - SendResponse(550, "Can't open directory " + listPath.getPath()); + SendResponse(FtpCodes::FILE_NOT_FOUND, "Can't open directory " + listPath.getPath()); return; } @@ -81,7 +82,7 @@ class MLSD : public FTPCommand { root.close(); CloseDataConnection(); - SendResponse(226, String(cnt) + " matches total"); + SendResponse(FtpCodes::TRANSFER_COMPLETE, String(cnt) + " matches total"); } }; diff --git a/src/Commands/NLST.h b/src/Commands/NLST.h index bc36e82..7bcff5b 100644 --- a/src/Commands/NLST.h +++ b/src/Commands/NLST.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" #include "../common.h" class NLST : public FTPCommand { @@ -18,7 +19,7 @@ class NLST : public FTPCommand { File dir = _Filesystem->open(WorkDirectory.getPath()); // if (!dir || !dir.isDirectory()) { CloseDataConnection(); - SendResponse(550, "Can't open directory " + WorkDirectory.getPath()); + SendResponse(FtpCodes::FILE_NOT_FOUND, "Can't open directory " + WorkDirectory.getPath()); return; } int cnt = 2; @@ -34,7 +35,7 @@ class NLST : public FTPCommand { f = dir.openNextFile(); } CloseDataConnection(); - SendResponse(226, String(cnt) + " matches total"); + SendResponse(FtpCodes::TRANSFER_COMPLETE, String(cnt) + " matches total"); } }; diff --git a/src/Commands/PASV.h b/src/Commands/PASV.h index 86e4ee6..04e446f 100644 --- a/src/Commands/PASV.h +++ b/src/Commands/PASV.h @@ -10,6 +10,7 @@ #endif #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" #include "../common.h" class PASV : public FTPCommand { @@ -28,7 +29,7 @@ class PASV : public FTPCommand { if (_PassiveMode != 0) { *_PassiveMode = false; } - SendResponse(425, "No local IP address for passive mode"); + SendResponse(FtpCodes::NO_DATA_CONNECTION, "No local IP address for passive mode"); return; } int port = 20000 + random(0, 1000); @@ -43,7 +44,7 @@ class PASV : public FTPCommand { int p1 = port / 256; int p2 = port % 256; String response = "Entering Passive Mode (" + String(localIP[0]) + "," + String(localIP[1]) + "," + String(localIP[2]) + "," + String(localIP[3]) + "," + String(p1) + "," + String(p2) + ")"; - SendResponse(227, response); + SendResponse(FtpCodes::ENTERING_PASV_MODE, response); } }; diff --git a/src/Commands/PORT.h b/src/Commands/PORT.h index f047df1..90cbf09 100644 --- a/src/Commands/PORT.h +++ b/src/Commands/PORT.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" #include "../common.h" class PORT : public FTPCommand { @@ -25,7 +26,7 @@ class PORT : public FTPCommand { (*_DataAddress)[i] = connection_details[i].toInt(); } *_DataPort = connection_details[4].toInt() * 256 + connection_details[5].toInt(); - SendResponse(200, "PORT command successful"); + SendResponse(FtpCodes::COMMAND_OK, "PORT command successful"); } }; diff --git a/src/Commands/PWD.h b/src/Commands/PWD.h index 9aa8dfc..ee3d406 100644 --- a/src/Commands/PWD.h +++ b/src/Commands/PWD.h @@ -11,7 +11,7 @@ class PWD : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { - SendResponse(257, "\"" + WorkDirectory.getPath() + "\" is your current directory"); + SendResponse(FtpCodes::PATHNAME_CREATED, "\"" + WorkDirectory.getPath() + "\" is your current directory"); } }; diff --git a/src/Commands/RETR.h b/src/Commands/RETR.h index 53075b4..2f4c081 100644 --- a/src/Commands/RETR.h +++ b/src/Commands/RETR.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" #include "../common.h" #define FTP_BUF_SIZE 4096 @@ -24,7 +25,7 @@ class RETR : public FTPCommandTransfer { _file = _Filesystem->open(path); if (!_file || _file.isDirectory()) { CloseDataConnection(); - SendResponse(550, "Can't open " + path); + SendResponse(FtpCodes::FILE_NOT_FOUND, "Can't open " + path); return; } workOnData(); @@ -38,7 +39,7 @@ class RETR : public FTPCommandTransfer { return; } CloseDataConnection(); - SendResponse(226, "File successfully transferred"); + SendResponse(FtpCodes::TRANSFER_COMPLETE, "File successfully transferred"); _file.close(); } diff --git a/src/Commands/RMD.h b/src/Commands/RMD.h index dad6f52..5970247 100644 --- a/src/Commands/RMD.h +++ b/src/Commands/RMD.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" class RMD : public FTPCommand { public: @@ -13,13 +14,13 @@ class RMD : public FTPCommand { void run(FTPPath &WorkDirectory, const std::vector &Line) override { String filepath = WorkDirectory.getFilePath(Line[1]); if (!_Filesystem->exists(filepath)) { - SendResponse(550, "Folder " + filepath + " not found"); + SendResponse(FtpCodes::FILE_NOT_FOUND, "Folder " + filepath + " not found"); return; } if (_Filesystem->rmdir(filepath)) { - SendResponse(250, " Deleted \"" + filepath + "\""); + SendResponse(FtpCodes::FILE_ACTION_OK, " Deleted \"" + filepath + "\""); } else { - SendResponse(450, "Can't delete \"" + filepath + "\""); + SendResponse(FtpCodes::FILE_ACTION_ABORTED, "Can't delete \"" + filepath + "\""); } } }; diff --git a/src/Commands/RNFR_RNTO.h b/src/Commands/RNFR_RNTO.h index 6a66e9d..a27b8fa 100644 --- a/src/Commands/RNFR_RNTO.h +++ b/src/Commands/RNFR_RNTO.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" class RNFR_RNTO : public FTPCommand { public: @@ -21,28 +22,28 @@ class RNFR_RNTO : public FTPCommand { void from(const FTPPath &WorkDirectory, const std::vector &Line) { String filepath = WorkDirectory.getFilePath(Line[1]); if (!_Filesystem->exists(filepath)) { - SendResponse(550, "File " + Line[1] + " not found"); + SendResponse(FtpCodes::FILE_NOT_FOUND, "File " + Line[1] + " not found"); return; } _fromSet = true; _from = filepath; - SendResponse(350, "RNFR accepted - file exists, ready for destination"); + SendResponse(FtpCodes::FILE_ACTION_PENDING, "RNFR accepted - file exists, ready for destination"); } void to(const FTPPath &WorkDirectory, const std::vector &Line) { if (!_fromSet) { - SendResponse(503, "Need RNFR before RNTO"); + SendResponse(FtpCodes::BAD_SEQUENCE, "Need RNFR before RNTO"); return; } String filepath = WorkDirectory.getFilePath(Line[1]); if (_Filesystem->exists(filepath)) { - SendResponse(553, "File " + Line[1] + " already exists"); + SendResponse(FtpCodes::FILE_NAME_NOT_ALLOWED, "File " + Line[1] + " already exists"); return; } if (_Filesystem->rename(_from, filepath)) { - SendResponse(250, "File successfully renamed or moved"); + SendResponse(FtpCodes::FILE_ACTION_OK, "File successfully renamed or moved"); } else { - SendResponse(451, "Rename/move failure"); + SendResponse(FtpCodes::FILE_ACTION_ABORTED_LOCAL_ERROR, "Rename/move failure"); } _fromSet = false; _from = ""; diff --git a/src/Commands/STAT.h b/src/Commands/STAT.h index c726bb7..2b8e51d 100644 --- a/src/Commands/STAT.h +++ b/src/Commands/STAT.h @@ -4,6 +4,7 @@ #include #include "../FTPCommand.h" +#include "../FTPResponseCodes.h" #include "../common.h" class STAT : public FTPCommand { @@ -14,7 +15,7 @@ class STAT : public FTPCommand { void run(FTPPath &WorkDirectory, const std::vector &Line) override { File dir = _Filesystem->open(WorkDirectory.getPath()); // if (!dir || !dir.isDirectory()) { - SendResponse(550, "Can't open directory " + WorkDirectory.getPath()); + SendResponse(FtpCodes::FILE_NOT_FOUND, "Can't open directory " + WorkDirectory.getPath()); return; } int cnt = 0; @@ -38,7 +39,7 @@ class STAT : public FTPCommand { f.close(); f = dir.openNextFile(); } - SendResponse(226, String(cnt) + " matches total"); + SendResponse(FtpCodes::TRANSFER_COMPLETE, String(cnt) + " matches total"); } }; diff --git a/src/Commands/STOR.h b/src/Commands/STOR.h index 6f3e022..18d4375 100644 --- a/src/Commands/STOR.h +++ b/src/Commands/STOR.h @@ -23,7 +23,7 @@ class STOR : public FTPCommandTransfer { _ftpFsFilePath = WorkDirectory.getFilePath(Line[1]); _file = _Filesystem->open(_ftpFsFilePath, "w"); if (!_file) { - SendResponse(451, "Can't open/create " + _ftpFsFilePath); + SendResponse(FtpCodes::FILE_ACTION_ABORTED_LOCAL_ERROR, "Can't open/create " + _ftpFsFilePath); CloseDataConnection(); return; } @@ -39,14 +39,14 @@ class STOR : public FTPCommandTransfer { _file.close(); this->_Filesystem->remove(_ftpFsFilePath.c_str()); - SendResponse(552, "Error occured while STORing"); + SendResponse(FtpCodes::EXCEEDED_STORAGE, "Error occured while STORing"); CloseDataConnection(); } return; } - SendResponse(226, "File successfully transferred"); + SendResponse(FtpCodes::TRANSFER_COMPLETE, "File successfully transferred"); CloseDataConnection(); _file.close(); } diff --git a/src/Commands/TYPE.h b/src/Commands/TYPE.h index 522e2ab..60ab12f 100644 --- a/src/Commands/TYPE.h +++ b/src/Commands/TYPE.h @@ -12,13 +12,13 @@ class TYPE : public FTPCommand { void run(FTPPath &WorkDirectory, const std::vector &Line) override { if (Line[1] == "A") { - SendResponse(200, "TYPE is now ASCII"); + SendResponse(FtpCodes::COMMAND_OK, "TYPE is now ASCII"); return; } else if (Line[1] == "I") { - SendResponse(200, "TYPE is now 8-bit binary"); + SendResponse(FtpCodes::COMMAND_OK, "TYPE is now 8-bit binary"); return; } - SendResponse(504, "Unknow TYPE"); + SendResponse(FtpCodes::COMMAND_NOT_IMPLEMENTED_FOR_PARAMETER, "Unknown TYPE"); } }; diff --git a/src/FTPCommand.h b/src/FTPCommand.h index 9c8edbc..a8753e4 100644 --- a/src/FTPCommand.h +++ b/src/FTPCommand.h @@ -8,6 +8,7 @@ #include "FTPFilesystem.h" #include "FTPPath.h" +#include "FTPResponseCodes.h" class FTPCommand { public: @@ -42,7 +43,7 @@ class FTPCommand { } if (_PassiveMode != 0 && *_PassiveMode) { if (_PassiveServer == 0 || *_PassiveServer == 0) { - SendResponse(425, "No passive server"); + SendResponse(FtpCodes::NO_DATA_CONNECTION, "No passive server"); return false; } WiFiServer *server = *_PassiveServer; @@ -55,28 +56,28 @@ class FTPCommand { } if (!server->hasClient()) { StopPassiveServer(); - SendResponse(425, "No data connection"); + SendResponse(FtpCodes::NO_DATA_CONNECTION, "No data connection"); return false; } WiFiClient client = server->accept(); if (!client) { StopPassiveServer(); - SendResponse(425, "No data connection"); + SendResponse(FtpCodes::NO_DATA_CONNECTION, "No data connection"); return false; } *_DataConnection = client; StopPassiveServer(); *_PassiveMode = false; - SendResponse(150, "Accepted data connection"); + SendResponse(FtpCodes::DATA_CONNECTION_OPEN, "Accepted data connection"); return true; } _DataConnection->connect(*_DataAddress, *_DataPort); if (!_DataConnection->connected()) { _DataConnection->stop(); - SendResponse(425, "No data connection"); + SendResponse(FtpCodes::NO_DATA_CONNECTION, "No data connection"); return false; } - SendResponse(150, "Accepted data connection"); + SendResponse(FtpCodes::DATA_CONNECTION_OPEN, "Accepted data connection"); return true; } @@ -105,9 +106,9 @@ class FTPCommand { if ((_DataConnection != 0) && (_DataConnection->available() > 0)) { return _DataConnection->readBytes(c, l); } - // Brief wait for initial data (max 100ms in 1ms intervals) + // Brief wait for initial data (max 500ms in 1ms intervals) if (_DataConnection != 0) { - for (int i = 0; i < 100; i++) { + for (int i = 0; i < 500; i++) { delay(1); if (_DataConnection->available() > 0) { return _DataConnection->readBytes(c, l); @@ -164,7 +165,7 @@ class FTPCommandTransfer : public FTPCommand { void abort() { if (_file) { CloseDataConnection(); - SendResponse(426, "Transfer aborted"); + SendResponse(FtpCodes::CONNECTION_CLOSED, "Transfer aborted"); _file.close(); } } diff --git a/src/FTPResponseCodes.h b/src/FTPResponseCodes.h new file mode 100644 index 0000000..e50df33 --- /dev/null +++ b/src/FTPResponseCodes.h @@ -0,0 +1,47 @@ +#ifndef FTP_RESPONSE_CODES_H_ +#define FTP_RESPONSE_CODES_H_ + +// FTP Response Code Constants +namespace FtpCodes { +// Success codes +const int DATA_CONNECTION_OPEN = 150; +const int COMMAND_OK = 200; +const int SYSTEM_STATUS = 211; +const int DIRECTORY_STATUS = 212; +const int FILE_STATUS = 213; +const int HELP_MESSAGE = 214; +const int SYSTEM_TYPE = 215; +const int READY = 220; +const int CLOSING = 221; +const int TRANSFER_COMPLETE = 226; +const int ENTERING_PASV_MODE = 227; +const int LOGGED_IN = 230; +const int FILE_ACTION_OK = 250; +const int PATHNAME_CREATED = 257; +const int USER_OK = 331; +const int NEED_PASSWORD = 331; +const int FILE_ACTION_PENDING = 350; + +// Error codes +const int SYNTAX_ERROR = 500; +const int SYNTAX_ERROR_PARAMS = 501; +const int COMMAND_NOT_IMPLEMENTED = 502; +const int BAD_SEQUENCE = 503; +const int COMMAND_NOT_IMPLEMENTED_FOR_PARAMETER = 504; +const int NOT_LOGGED_IN = 530; +const int NEED_ACCOUNT = 532; +const int FILE_ACTION_NOT_TAKEN = 550; +const int FILE_NOT_FOUND = 550; +const int PAGE_TYPE_UNKNOWN = 551; +const int EXCEEDED_STORAGE = 552; +const int FILE_NAME_NOT_ALLOWED = 553; +const int TRANSFER_ABORTED = 426; +const int NO_DATA_CONNECTION = 425; +const int CANNOT_OPEN_CONNECTION = 425; +const int CONNECTION_CLOSED = 426; +const int FILE_ACTION_ABORTED = 450; +const int FILE_ACTION_ABORTED_LOCAL_ERROR = 451; +const int INSUFFICIENT_STORAGE = 452; +} // namespace FtpCodes + +#endif From 9d7ecb3a2b9aa828ee28c0143d43607906393e39 Mon Sep 17 00:00:00 2001 From: Joe91 <1015.5@gmx.de> Date: Thu, 7 May 2026 17:10:34 +0200 Subject: [PATCH 11/18] initial commit --- README.md | 13 ++++++++---- src/Commands/LIST.h | 1 + src/Commands/MLSD.h | 1 + src/Commands/STOR.h | 2 +- src/FTPPath.cpp | 49 +++++++++++++++++++++++++++++++++++++++++---- src/FTPPath.h | 19 ++++++++++++++++++ 6 files changed, 76 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index f372065..f961a09 100644 --- a/README.md +++ b/README.md @@ -11,26 +11,31 @@ In the example folder you can find a very simple usage of the FTP server. You ju Currently all kind of simple commands are known to the server: * CDUP +* CLNT * CWD * DELE +* FEAT * LISST * MKD +* MLSD +* NLST +* OPTS +* PASV * PORT * PWD * RETR * RMD * RNFR * RNTO +* STAT * STOR * TYPE +* USER +* PASS * SYST * QUIT * ABOR -* NLST -* STAT ## What is still missing / TODO Some commands are still missing, if you need them create a ticket :) - -Currently just the active mode is supported. For the passive mode you need to wait until version 1.0.0. diff --git a/src/Commands/LIST.h b/src/Commands/LIST.h index f8b9486..ed698d6 100644 --- a/src/Commands/LIST.h +++ b/src/Commands/LIST.h @@ -55,6 +55,7 @@ class LIST : public FTPCommand { for (int i = 0; i < fill_cnt; i++) { data_print(" "); } + filename = listPath.reparse(filename); data_println(filesize + " Jan 01 1970 " + filename); cnt++; f.close(); diff --git a/src/Commands/MLSD.h b/src/Commands/MLSD.h index c56eabf..23e047d 100644 --- a/src/Commands/MLSD.h +++ b/src/Commands/MLSD.h @@ -74,6 +74,7 @@ class MLSD : public FTPCommand { data_print(" "); String filename = f.name(); filename.remove(0, filename.lastIndexOf('/') + 1); + filename = listPath.reparse(filename); data_println(filename); cnt++; diff --git a/src/Commands/STOR.h b/src/Commands/STOR.h index 18d4375..a5be0bc 100644 --- a/src/Commands/STOR.h +++ b/src/Commands/STOR.h @@ -35,7 +35,7 @@ class STOR : public FTPCommandTransfer { int nb = data_read(buffer, FTP_BUF_SIZE); if (nb > 0) { const auto wb = _file.write(buffer, nb); - if (wb != static_cast(nb)) { + if (wb != static_cast::type>(nb)) { _file.close(); this->_Filesystem->remove(_ftpFsFilePath.c_str()); diff --git a/src/FTPPath.cpp b/src/FTPPath.cpp index 37c6973..57dae2a 100644 --- a/src/FTPPath.cpp +++ b/src/FTPPath.cpp @@ -30,13 +30,14 @@ String FTPPath::getPath() const { } String FTPPath::getFilePath(String filename) const { - if (*filename.begin() == '/') { - return filename; + String sane_filename = sanitize(filename); + if (*sane_filename.begin() == '/') { + return sane_filename; } if (_Path.size() == 0) { - return "/" + filename; + return "/" + sane_filename; } - return getPath() + "/" + filename; + return getPath() + "/" + sane_filename; } std::list FTPPath::splitPath(String path) { @@ -63,3 +64,43 @@ String FTPPath::createPath(const std::list &path) { } return new_path; } + +String FTPPath::sanitize(String input) const { + String output = ""; + // Pre-allocate memory to prevent heap fragmentation + output.reserve(input.length()); + + for (size_t i = 0; i < input.length(); i++) { + unsigned char c = input[i]; + // Check for illegal chars, percent sign, or non-printable control chars + const String illegal_chars = ":*?\"<>|%"; + if (illegal_chars.indexOf(c) != -1 || c < 32) { + output += '%'; + output += to_hex(c >> 4); + output += to_hex(c & 0x0F); + } else { + output += (char)c; + } + } + return output; +} + +String FTPPath::reparse(String input) const { + String output = ""; + output.reserve(input.length()); + + for (size_t i = 0; i < input.length(); i++) { + if (input[i] == '%' && i + 2 < input.length()) { + int high = from_hex(input[i + 1]); + int low = from_hex(input[i + 2]); + + if (high != -1 && low != -1) { + output += (char)((high << 4) | low); + i += 2; // Skip the two hex characters + continue; + } + } + output += input[i]; + } + return output; +} diff --git a/src/FTPPath.h b/src/FTPPath.h index 4bfccdd..503026c 100644 --- a/src/FTPPath.h +++ b/src/FTPPath.h @@ -19,7 +19,26 @@ class FTPPath { static std::list splitPath(String path); static String createPath(const std::list &path); + String sanitize(String input) const; + String reparse(String input) const; + private: + // Helper to convert nibble to hex character + char to_hex(unsigned char v) const { + return v < 10 ? '0' + v : 'A' + (v - 10); + } + + // Helper to convert hex character to nibble + int from_hex(char c) const { + if (c >= '0' && c <= '9') + return c - '0'; + if (c >= 'A' && c <= 'F') + return c - 'A' + 10; + if (c >= 'a' && c <= 'f') + return c - 'a' + 10; + return -1; + } + std::list _Path; }; From 494778fe79aaccfff2fe88d025ca6f80c40b10a8 Mon Sep 17 00:00:00 2001 From: Joe91 <1015.5@gmx.de> Date: Thu, 7 May 2026 21:48:45 +0200 Subject: [PATCH 12/18] add define for sanitization --- src/FTPPath.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/FTPPath.cpp b/src/FTPPath.cpp index 57dae2a..3daf688 100644 --- a/src/FTPPath.cpp +++ b/src/FTPPath.cpp @@ -66,6 +66,9 @@ String FTPPath::createPath(const std::list &path) { } String FTPPath::sanitize(String input) const { +#ifndef ENABLE_FTP_SANITIZATION + return input; +#else String output = ""; // Pre-allocate memory to prevent heap fragmentation output.reserve(input.length()); @@ -83,9 +86,13 @@ String FTPPath::sanitize(String input) const { } } return output; +#endif } String FTPPath::reparse(String input) const { +#ifndef ENABLE_FTP_SANITIZATION + return input; +#else String output = ""; output.reserve(input.length()); @@ -103,4 +110,5 @@ String FTPPath::reparse(String input) const { output += input[i]; } return output; +#endif } From 5effb4c898cc161db18f94a2c877deaff0bfe130 Mon Sep 17 00:00:00 2001 From: Joe91 <1015.5@gmx.de> Date: Thu, 7 May 2026 22:27:44 +0200 Subject: [PATCH 13/18] fix folders as well --- src/Commands/CDUP.h | 2 +- src/Commands/CWD.h | 2 +- src/Commands/LIST.h | 2 +- src/Commands/MLSD.h | 2 +- src/Commands/NLST.h | 2 +- src/Commands/PWD.h | 2 +- src/Commands/STAT.h | 2 +- src/FTPPath.cpp | 5 +++++ src/FTPPath.h | 1 + 9 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/Commands/CDUP.h b/src/Commands/CDUP.h index da8f210..9016ed4 100644 --- a/src/Commands/CDUP.h +++ b/src/Commands/CDUP.h @@ -13,7 +13,7 @@ class CDUP : public FTPCommand { void run(FTPPath &WorkDirectory, const std::vector &Line) override { WorkDirectory.goPathUp(); - SendResponse(FtpCodes::COMMAND_OK, "Ok. Current directory is " + WorkDirectory.getPath()); + SendResponse(FtpCodes::COMMAND_OK, "Ok. Current directory is " + WorkDirectory.getClearPath()); } }; diff --git a/src/Commands/CWD.h b/src/Commands/CWD.h index 29641e3..e8f8420 100644 --- a/src/Commands/CWD.h +++ b/src/Commands/CWD.h @@ -21,7 +21,7 @@ class CWD : public FTPCommand { File dir = _Filesystem->open(path.getPath()); if (dir.isDirectory()) { WorkDirectory = path; - SendResponse(FtpCodes::COMMAND_OK, "Ok. Current directory is " + WorkDirectory.getPath()); + SendResponse(FtpCodes::COMMAND_OK, "Ok. Current directory is " + WorkDirectory.getClearPath()); } else { SendResponse(FtpCodes::FILE_ACTION_NOT_TAKEN, "Directory does not exist"); } diff --git a/src/Commands/LIST.h b/src/Commands/LIST.h index ed698d6..334464e 100644 --- a/src/Commands/LIST.h +++ b/src/Commands/LIST.h @@ -34,7 +34,7 @@ class LIST : public FTPCommand { File dir = _Filesystem->open(listPath.getPath()); // if (!dir || !dir.isDirectory()) { CloseDataConnection(); - SendResponse(FtpCodes::FILE_NOT_FOUND, "Can't open directory " + listPath.getPath()); + SendResponse(FtpCodes::FILE_NOT_FOUND, "Can't open directory " + listPath.getClearPath()); return; } int cnt = 2; diff --git a/src/Commands/MLSD.h b/src/Commands/MLSD.h index 23e047d..32aa52b 100644 --- a/src/Commands/MLSD.h +++ b/src/Commands/MLSD.h @@ -38,7 +38,7 @@ class MLSD : public FTPCommand { if (!root || !root.isDirectory()) { root.close(); CloseDataConnection(); - SendResponse(FtpCodes::FILE_NOT_FOUND, "Can't open directory " + listPath.getPath()); + SendResponse(FtpCodes::FILE_NOT_FOUND, "Can't open directory " + listPath.getClearPath()); return; } diff --git a/src/Commands/NLST.h b/src/Commands/NLST.h index 7bcff5b..450db66 100644 --- a/src/Commands/NLST.h +++ b/src/Commands/NLST.h @@ -19,7 +19,7 @@ class NLST : public FTPCommand { File dir = _Filesystem->open(WorkDirectory.getPath()); // if (!dir || !dir.isDirectory()) { CloseDataConnection(); - SendResponse(FtpCodes::FILE_NOT_FOUND, "Can't open directory " + WorkDirectory.getPath()); + SendResponse(FtpCodes::FILE_NOT_FOUND, "Can't open directory " + WorkDirectory.getClearPath()); return; } int cnt = 2; diff --git a/src/Commands/PWD.h b/src/Commands/PWD.h index ee3d406..9cadb3e 100644 --- a/src/Commands/PWD.h +++ b/src/Commands/PWD.h @@ -11,7 +11,7 @@ class PWD : public FTPCommand { } void run(FTPPath &WorkDirectory, const std::vector &Line) override { - SendResponse(FtpCodes::PATHNAME_CREATED, "\"" + WorkDirectory.getPath() + "\" is your current directory"); + SendResponse(FtpCodes::PATHNAME_CREATED, "\"" + WorkDirectory.getClearPath() + "\" is your current directory"); } }; diff --git a/src/Commands/STAT.h b/src/Commands/STAT.h index 2b8e51d..abd6491 100644 --- a/src/Commands/STAT.h +++ b/src/Commands/STAT.h @@ -15,7 +15,7 @@ class STAT : public FTPCommand { void run(FTPPath &WorkDirectory, const std::vector &Line) override { File dir = _Filesystem->open(WorkDirectory.getPath()); // if (!dir || !dir.isDirectory()) { - SendResponse(FtpCodes::FILE_NOT_FOUND, "Can't open directory " + WorkDirectory.getPath()); + SendResponse(FtpCodes::FILE_NOT_FOUND, "Can't open directory " + WorkDirectory.getClearPath()); return; } int cnt = 0; diff --git a/src/FTPPath.cpp b/src/FTPPath.cpp index 3daf688..038e83d 100644 --- a/src/FTPPath.cpp +++ b/src/FTPPath.cpp @@ -12,6 +12,7 @@ FTPPath::~FTPPath() { } void FTPPath::changePath(String path) { + path = sanitize(path); std::list p = splitPath(path); if (!path.isEmpty() && path[0] == '/') { _Path.assign(p.begin(), p.end()); @@ -29,6 +30,10 @@ String FTPPath::getPath() const { return createPath(_Path); } +String FTPPath::getClearPath() const { + return reparse(createPath(_Path)); +} + String FTPPath::getFilePath(String filename) const { String sane_filename = sanitize(filename); if (*sane_filename.begin() == '/') { diff --git a/src/FTPPath.h b/src/FTPPath.h index 503026c..d53ba38 100644 --- a/src/FTPPath.h +++ b/src/FTPPath.h @@ -14,6 +14,7 @@ class FTPPath { void goPathUp(); String getPath() const; + String getClearPath() const; String getFilePath(String filename) const; static std::list splitPath(String path); From 7347e0047879c2d494aea9dd6be002a078e63d9f Mon Sep 17 00:00:00 2001 From: Joe91 <1015.5@gmx.de> Date: Thu, 7 May 2026 22:50:37 +0200 Subject: [PATCH 14/18] two more fixes --- src/Commands/NLST.h | 2 +- src/Commands/STAT.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Commands/NLST.h b/src/Commands/NLST.h index 450db66..ac1762c 100644 --- a/src/Commands/NLST.h +++ b/src/Commands/NLST.h @@ -29,7 +29,7 @@ class NLST : public FTPCommand { while (f) { String filename = f.name(); filename.remove(0, filename.lastIndexOf('/') + 1); - data_println(filename); + data_println(WorkDirectory.reparse(filename)); cnt++; f.close(); f = dir.openNextFile(); diff --git a/src/Commands/STAT.h b/src/Commands/STAT.h index abd6491..2e4baef 100644 --- a/src/Commands/STAT.h +++ b/src/Commands/STAT.h @@ -34,7 +34,7 @@ class STAT : public FTPCommand { for (int i = 0; i < fill_cnt; i++) { data_print(" "); } - data_println(filesize + " Jan 01 1970 " + filename); + data_println(filesize + " Jan 01 1970 " + WorkDirectory.reparse(filename)); cnt++; f.close(); f = dir.openNextFile(); From c36f1ddf8ac5dcab9db0ed2153d826450ed93384 Mon Sep 17 00:00:00 2001 From: Joe91 <1015.5@gmx.de> Date: Tue, 12 May 2026 20:14:48 +0200 Subject: [PATCH 15/18] try to fix dependabot --- .github/workflows/dependabot.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/dependabot.yml b/.github/workflows/dependabot.yml index 21af65a..90061fb 100644 --- a/.github/workflows/dependabot.yml +++ b/.github/workflows/dependabot.yml @@ -13,7 +13,11 @@ jobs: steps: - name: Checkout uses: actions/checkout@v3 + with: + token: ${{ secrets.DEPENDABOT_PAT }} + - name: run PlatformIO Dependabot uses: peterus/platformio_dependabot@main with: github_token: ${{ secrets.DEPENDABOT_PAT }} + github_repo_path: ${{ github.repository }} From 9dee61973d7989addc7ec82ec623ff27ec4ee916 Mon Sep 17 00:00:00 2001 From: Joe91 <1015.5@gmx.de> Date: Tue, 12 May 2026 20:28:07 +0200 Subject: [PATCH 16/18] update readme and dependencies --- .github/workflows/dependabot.yml | 1 - README.md | 2 ++ platformio.ini | 4 ++-- src/FTPPath.h | 2 ++ 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/dependabot.yml b/.github/workflows/dependabot.yml index 90061fb..007c027 100644 --- a/.github/workflows/dependabot.yml +++ b/.github/workflows/dependabot.yml @@ -20,4 +20,3 @@ jobs: uses: peterus/platformio_dependabot@main with: github_token: ${{ secrets.DEPENDABOT_PAT }} - github_repo_path: ${{ github.repository }} diff --git a/README.md b/README.md index f961a09..3e8221c 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,13 @@ # ESP-FTP-Server-Lib +This is a fork form https://github.com/peterus/ESP-FTP-Server-Lib, since there seems to be no maintanance anymore. This library will provide a simple and modern FTP server for your ESP32 or ESP8266 device. You can setup multiple users and mutliple filesystems (SD-Card, MMC-Card or/and SPIFFS). ## Examples In the example folder you can find a very simple usage of the FTP server. You just need to setup the users, add the filesystems which you want to use, and call the handle function in the loop. +With the Compileflag -DENABLE_FTP_SANITIZATION you can enable support for special-characters like ":" or "?" by URL-Encodeing of Files. ## Known Commands to the server diff --git a/platformio.ini b/platformio.ini index 0df9b4e..bd653af 100644 --- a/platformio.ini +++ b/platformio.ini @@ -1,6 +1,6 @@ [env:ttgo-lora32] -platform = espressif32 @ 6.3.2 +platform = espressif32 @ 7.0.1 board = ttgo-lora32-v1 framework = arduino test_build_src = yes @@ -10,7 +10,7 @@ check_flags = check_skip_packages = yes [env:ttgo-ESP8266] -platform = espressif8266 @ 4.2.0 +platform = espressif8266 @ 4.2.1 board = esp_wroom_02 framework = arduino test_build_src = yes diff --git a/src/FTPPath.h b/src/FTPPath.h index d53ba38..5eb19db 100644 --- a/src/FTPPath.h +++ b/src/FTPPath.h @@ -24,6 +24,7 @@ class FTPPath { String reparse(String input) const; private: +#ifdef ENABLE_FTP_SANITIZATION // Helper to convert nibble to hex character char to_hex(unsigned char v) const { return v < 10 ? '0' + v : 'A' + (v - 10); @@ -39,6 +40,7 @@ class FTPPath { return c - 'a' + 10; return -1; } +#endif std::list _Path; }; From 0a5a8de1788ba0b06045785f1eab557d2d2d2ee8 Mon Sep 17 00:00:00 2001 From: Joe91 <1015.5@gmx.de> Date: Tue, 12 May 2026 20:39:01 +0200 Subject: [PATCH 17/18] add manual workflow-run as well --- .github/workflows/main.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index d8abd8a..96f4f47 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -3,6 +3,7 @@ name: Integration Tests on: merge_group: pull_request: + workflow_dispatch: jobs: build: From 517a2c90876659bfd6692d1f464784a05296e8d8 Mon Sep 17 00:00:00 2001 From: Joe91 <1015.5@gmx.de> Date: Tue, 19 May 2026 16:25:03 +0200 Subject: [PATCH 18/18] fix: only reparse parsed chars --- src/FTPPath.cpp | 14 ++++++++------ src/FTPPath.h | 5 +++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/FTPPath.cpp b/src/FTPPath.cpp index 038e83d..95c0d55 100644 --- a/src/FTPPath.cpp +++ b/src/FTPPath.cpp @@ -80,9 +80,8 @@ String FTPPath::sanitize(String input) const { for (size_t i = 0; i < input.length(); i++) { unsigned char c = input[i]; - // Check for illegal chars, percent sign, or non-printable control chars - const String illegal_chars = ":*?\"<>|%"; - if (illegal_chars.indexOf(c) != -1 || c < 32) { + // Check for illegal chars or percent sign + if (getInvalidChars().indexOf(c) != -1) { output += '%'; output += to_hex(c >> 4); output += to_hex(c & 0x0F); @@ -107,9 +106,12 @@ String FTPPath::reparse(String input) const { int low = from_hex(input[i + 2]); if (high != -1 && low != -1) { - output += (char)((high << 4) | low); - i += 2; // Skip the two hex characters - continue; + char c = (char)((high << 4) | low); + if (getInvalidChars().indexOf(c) != -1) { + output += c; + i += 2; // Skip the two hex characters + continue; + } } } output += input[i]; diff --git a/src/FTPPath.h b/src/FTPPath.h index 5eb19db..70f41ee 100644 --- a/src/FTPPath.h +++ b/src/FTPPath.h @@ -25,6 +25,11 @@ class FTPPath { private: #ifdef ENABLE_FTP_SANITIZATION + static const String &getInvalidChars() { + static const String invalidChars = ":*?\"<>|%"; + return invalidChars; + } + // Helper to convert nibble to hex character char to_hex(unsigned char v) const { return v < 10 ? '0' + v : 'A' + (v - 10);