From 35b7369e3d538c50e5327c48c8d34dc3e6ef9298 Mon Sep 17 00:00:00 2001 From: srgg Date: Wed, 15 Jul 2026 23:58:44 -0600 Subject: [PATCH 1/3] feat: universal command hook (FtpResponse facade) + custom transfers Add a single extension point, setCommandHandler(FtpResponse&, command, parameter), invoked before the built-in dispatch for every command. The hook is void; its disposition is inferred from what it does with the FtpResponse capability facade: reply() -> command handled, built-in skipped rewriteCommand() -> library runs the rewritten command instead beginCustomTransfer -> caller-driven data transfer, built-in skipped (nothing) -> the original command runs FtpResponse is a narrow facade (friend of FtpServer) exposing only the hook-safe ops, so a hook cannot reach begin()/end()/handleFTP() or rebind itself. reply() being the handled-signal makes "reply then also pass" a non-issue by construction. rewriteCommand copies are bounded to the internal command[]/cmdLine[] buffers (strncpy/strnlen + alias-safe memmove). CustomTransfer streams cooperatively, one chunk per handleFTP() tick, so the control channel never blocks. onEnd() fires exactly once on every termination (done, ABOR, disconnect) through the abortTransfer() funnel, so resources never leak; it returns a custom final line or nullptr for the library default (226/426). --- FtpServer.cpp | 118 ++++++++++++++++++++++++++++++++++++++++++++++++-- FtpServer.h | 91 +++++++++++++++++++++++++++++++++++++- 2 files changed, 205 insertions(+), 4 deletions(-) diff --git a/FtpServer.cpp b/FtpServer.cpp index 65a2115..f97f4cf 100644 --- a/FtpServer.cpp +++ b/FtpServer.cpp @@ -50,6 +50,7 @@ #include #include #include +#include // Implementations for 8.3 helpers (only for SD on AVR) #if (STORAGE_TYPE == STORAGE_SD) @@ -289,7 +290,9 @@ void FtpServer::begin( const char * _welcomeMessage ) { void FtpServer::end() { if(client.connected()) { - disconnectClient(); + disconnectClient(); // -> abortTransfer -> finishCustom if a custom transfer is live + } else if (transferStage == FTP_Custom) { // no client to disconnect, but a custom transfer is still + finishCustom( CustomTransfer::TR_ABORTED ); // in flight: free it so onEnd runs on this server-side stop too } #if FTP_SERVER_NETWORK_TYPE == NETWORK_ESP32 // && !defined(ARDUINO_ARCH_RP2040) @@ -441,7 +444,11 @@ uint8_t FtpServer::handleFTP() { } else if (transferStage == FTP_Mlsd) // MLSD listing { if (!doMlsd()) { - + transferStage = FTP_Close; + } + } else if (transferStage == FTP_Custom) // caller-driven cooperative transfer + { + if (!doCustom()) { transferStage = FTP_Close; } } else if (cmdStage > FTP_Client @@ -508,6 +515,91 @@ void FtpServer::disconnectClient() } } +// --- FtpResponse: the narrow capability facade handed to a command hook (see FtpServer.h). +// Each method acts on the owning server's internals via friendship. --- +void FtpResponse::reply( const char * line ) +{ + server_.client.println( line ); + server_._replied = true; // marks the command handled (built-in skipped) +} + +void FtpResponse::rewriteCommand( const char * cmd, const char * param ) +{ + strncpy( server_.command, cmd, sizeof( server_.command ) - 1 ); + server_.command[ sizeof( server_.command ) - 1 ] = '\0'; + size_t n = param ? strnlen( param, sizeof( server_.cmdLine ) - 1 ) : 0; + memmove( server_.cmdLine, param ? param : "", n ); // memmove: param may point into cmdLine + server_.cmdLine[ n ] = '\0'; + server_.parameter = server_.cmdLine; +} + +bool FtpResponse::isAuthenticated() const +{ + return server_.cmdStage == FTP_Cmd; +} + +void FtpResponse::setAuthenticated( bool authenticated ) +{ + server_.cmdStage = authenticated ? FTP_Cmd : FTP_User; +} + +bool FtpResponse::beginCustomTransfer( const CustomTransfer * xfer, void * ctx ) +{ + if( ! server_.dataConnect( true ) ) // open data connection + send "150"; false on failure + return false; + server_._xfer = xfer; + server_._customCtx = ctx; + server_.transferStage = FTP_Custom; + server_.bytesTransfered = 0; // progress accumulator, same one the built-in uses + if( server_._transferCallback ) // mirror RETR: announce the transfer start (size unknown → 0) + server_._transferCallback( FTP_DOWNLOAD_START, xfer->name, 0 ); + // Mark THIS command handled. _replied is per-command (reset before each hook call), so a + // later command (e.g. ABOR) arriving while the transfer is still running is NOT swallowed — + // it falls through to its built-in. Keying off transferStage instead would block every + // command for the whole transfer. + server_._replied = true; + return true; +} + +// Push one chunk of the active custom transfer; false ends it (finishCustom already ran). +bool FtpServer::doCustom() +{ + int r = _xfer->sendChunk( _customCtx, data ); + if( r > 0 ) // bytes written this tick — report progress, continue + { + bytesTransfered += r; + if( FtpServer::_transferCallback ) + FtpServer::_transferCallback( FTP_DOWNLOAD, _xfer->name, bytesTransfered ); + return true; + } + if( r == 0 ) // yielded (nothing to send this tick); no progress + return true; + finishCustom( r == -1 ? CustomTransfer::TR_DONE : CustomTransfer::TR_ABORTED ); + return false; +} + +// End a custom transfer exactly once: onEnd() (caller cleanup + optional custom final line), +// then the final response (custom or default), close the data connection, clear state. +void FtpServer::finishCustom( CustomTransfer::TransferResult result ) +{ + const char * line = ( _xfer && _xfer->onEnd ) ? _xfer->onEnd( _customCtx, result ) : nullptr; + +#if defined(ESP8266) || defined(ESP32) + data.flush(); + delay( 20 ); // grace period to let TCP finish sending +#endif + data.stop(); + + client.println( line && *line ? line + : ( result == CustomTransfer::TR_DONE ? "226 Transfer complete" : "426 Transfer aborted" ) ); + if( FtpServer::_transferCallback ) // mirror RETR: report the terminal outcome + total bytes + FtpServer::_transferCallback( result == CustomTransfer::TR_DONE ? FTP_TRANSFER_STOP : FTP_TRANSFER_ERROR, + _xfer ? _xfer->name : nullptr, bytesTransfered ); + _xfer = nullptr; + _customCtx = nullptr; + transferStage = FTP_Close; +} + bool FtpServer::processCommand() { /////////////////////////////////////// @@ -520,6 +612,22 @@ bool FtpServer::processCommand() DEBUG_PRINT(F("Command is: ")); DEBUG_PRINTLN(command); + // Command hook: runs before the built-in dispatch. reply() marks the command as handled (the + // built-in is skipped); rewriteCommand() changes which command runs; doing nothing lets the + // original run. + _replied = false; + if( _commandHandler ) + { + FtpResponse res( *this ); + _commandHandler( res, command, parameter ); + } + + if( _replied ) + { + // _commandHandler processed this command + return true; + } + // // USER - User Identity // @@ -2215,7 +2323,11 @@ void FtpServer::closeTransfer() void FtpServer::abortTransfer() { - if( transferStage != FTP_Close ) + if( transferStage == FTP_Custom ) // caller-driven transfer: finishCustom owns onEnd + response + { + finishCustom( CustomTransfer::TR_ABORTED ); + } + else if( transferStage != FTP_Close ) { if (FtpServer::_transferCallback) { FtpServer::_transferCallback(FTP_TRANSFER_ERROR, getFileName(&file).c_str(), bytesTransfered); diff --git a/FtpServer.h b/FtpServer.h index c916866..19344e9 100644 --- a/FtpServer.h +++ b/FtpServer.h @@ -516,7 +516,8 @@ enum ftpTransfer { FTP_Close = 0, // In this stage, close data channel FTP_Store, // store file FTP_List, // list of files FTP_Nlst, // list of name of files - FTP_Mlsd }; // listing for machine processing + FTP_Mlsd, // listing for machine processing + FTP_Custom }; // caller-driven cooperative transfer (beginCustomTransfer) enum ftpDataConn { FTP_NoConn = 0,// No data connection FTP_Pasive, // Passive type @@ -545,6 +546,76 @@ enum FtpTransferOperation { FTP_UPLOAD_ERROR = 5 }; +class FtpServer; + +// A caller-driven data transfer, streamed cooperatively — one chunk per handleFTP() call — so +// the control channel stays responsive and nothing blocks. Use it for outputs with no standard +// FTP equivalent (a multi-file bundle, on-the-fly compression), started from a command hook via +// FtpResponse::beginCustomTransfer(). +struct CustomTransfer +{ + enum TransferResult { + TR_DONE, // transfer completed successfully + TR_ABORTED, // client aborted, connection lost, or a chunk reported an error + }; + + // Label reported to setTransferCallback on start/progress/end — a custom transfer has no + // `file`, so it supplies its own name (e.g. the download filename). May be nullptr. + const char * name; + + // Produce and send the next chunk into `data`, cooperatively (must NOT block). Return: + // >0 bytes written this tick — reported as progress; transfer continues + // 0 wrote nothing, not done — yield (e.g. socket full); retried next tick, no progress + // -1 done — TR_DONE + // <-1 error — TR_ABORTED + // The byte count drives the same progress callback the built-in RETR fires, so client + // progress and a host stall-watchdog keyed on setTransferCallback both work unchanged. + int (*sendChunk)( void * ctx, Client & data ); + + // Finalize — called EXACTLY once for every outcome (done, aborted, connection lost), so it + // owns cleanup (close files, free buffers). Return a custom final response line, or + // nullptr/"" for the library default (226 on success, 426 otherwise). The returned pointer + // must outlive the call (a literal, static, or a buffer owned by ctx). + const char * (*onEnd)( void * ctx, TransferResult result ); +}; + +// How a command hook reacts to the current command (Express-`res` style). A deliberately +// narrow handle: it exposes only the operations below, so a hook cannot begin()/end()/ +// handleFTP() or rebind itself. FtpServer builds one and passes it to the hook; the methods +// act on the server's internals through friendship. +class FtpResponse +{ +public: + // Send one FTP response line (e.g. "550 Read-only.") terminated by CRLF. Sending a reply + // from a command hook marks the command handled, so the built-in handler is skipped. + void reply( const char * line ); + + // Replace the command currently being processed; the library then dispatches the rewrite + // instead of the original (e.g., map "XLATEST" to "RETR " and reuse the built-in + // download). Copies are bound to the internal buffers, so cmd/param of any length are + // safe. Note: the rewrite is NOT re-filtered by the hook — apply your policy to the target + // verb, not just the incoming one. + void rewriteCommand( const char * cmd, const char * param ); + + // Whether the client has completed USER/PASS login. + bool isAuthenticated() const; + + // Force the login state, so a hook can implement its own authentication (a token, an IP + // allow-list, an extra factor) and have the rest of the session honor it. + void setAuthenticated( bool authenticated ); + + // Start a caller-driven data transfer from within a command hook: opens the data connection + // (sends "150"), then drives xfer->sendChunk once per handleFTP() call until it finishes and + // calls xfer->onEnd on any termination. `ctx` is passed to both callbacks. Returns false if + // the data connection could not be opened. Marks the command as handled. + bool beginCustomTransfer( const CustomTransfer * xfer, void * ctx ); + +private: + explicit FtpResponse( FtpServer & server ) : server_( server ) {} + FtpServer & server_; + friend class FtpServer; +}; + class FtpServer { public: @@ -569,10 +640,28 @@ class FtpServer _transferCallback = _transferCallbackParam; } + // Install a hook invoked for every command BEFORE the built-in dispatch. The hook is + // void; its intent is inferred from what it does with the FtpResponse: reply() takes the + // command over (the built-in is skipped), rewriteCommand() changes which command runs, + // and doing nothing lets the original run. The single extension point for rejecting, + // rewriting, implementing, or authorizing commands. + void setCommandHandler(void (*_commandHandlerParam)(FtpResponse& res, const char* command, const char* parameter) ) + { + _commandHandler = _commandHandlerParam; + } + private: // Use 32-bit sizes for callbacks to avoid truncation on platforms where "unsigned int" is 16-bit (AVR) void (*_callback)(FtpOperation ftpOperation, uint32_t freeSpace, uint32_t totalSpace){}; void (*_transferCallback)(FtpTransferOperation ftpOperation, const char* name, uint32_t transferredSize){}; + void (*_commandHandler)(FtpResponse& res, const char* command, const char* parameter){}; + bool _replied = false; // set by FtpResponse::reply() during a hook call; drives handled-vs-pass + friend class FtpResponse; + + const CustomTransfer * _xfer = nullptr; // active caller-driven transfer (FTP_Custom), or null + void * _customCtx = nullptr; + bool doCustom(); // push one chunk; false when the transfer ends + void finishCustom( CustomTransfer::TransferResult result ); // onEnd + final response + close, exactly once void iniVariables(); void clientConnected(); From 6ca6f9cd5d8649751d6e88fd97ac80b1a7d42035 Mon Sep 17 00:00:00 2001 From: srgg Date: Tue, 11 Aug 2026 16:29:42 -0600 Subject: [PATCH 2/3] fix: report every transfer outcome truthfully, and always report the end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An app pairs FTP_DOWNLOAD_START/FTP_UPLOAD_START with the terminal callback to release what it acquired for a transfer — a radio boost, a paused advertisement, a progress UI. Several endings never delivered that pairing, and several delivered the wrong one. Missing terminal callback: - closeTransfer() emitted it only inside `deltaT > 0 && bytesTransfered > 0`, a condition written to guard the throughput division. An empty file, or one that finished inside a single millisecond, ended with no notification at all. - dataConnected() answered 426 and closed the stage without any callback. - doStore()'s out-of-space path answered 552 and closed the file by hand: no callback, no dir close, no restart-position reset. Success reported for a failure — the client is told 226 and the app is told FTP_TRANSFER_STOP: - doRetrieve(): a zero-length socket write, and a peer that closed the data connection with bytes still to send (a truncated download). - doStore(): a peer that stayed connected but sent nothing for 5 s. - doRetrieve()'s REST seek failure additionally answered twice, 450 then 226. All of these now end through abortTransfer(), which closes the file and the dir, fires FTP_TRANSFER_ERROR, resets the restart position and replies once. It takes an optional reply so a caller with a more specific code than 426 keeps it — used by the 450 and 552 paths above. Separately, doRetrieve() dropped data on a short write: file.read() had already advanced the cursor by the full block, so bytes the socket did not accept were never sent and the client received a file with a hole in it and no error. The cursor is now rewound to the first unsent byte. --- FtpServer.cpp | 44 +++++++++++++++++++++++++------------------- FtpServer.h | 4 +++- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/FtpServer.cpp b/FtpServer.cpp index f97f4cf..08af9ab 100644 --- a/FtpServer.cpp +++ b/FtpServer.cpp @@ -1501,9 +1501,7 @@ bool FtpServer::dataConnected() { if( data.connected()) return true; - data.stop(); - client.println(F("426 Data connection closed. Transfer aborted") ); - transferStage = FTP_Close; + abortTransfer(F("426 Data connection closed. Transfer aborted")); return false; } @@ -1604,8 +1602,8 @@ bool FtpServer::doRetrieve() // Handle resume if REST was used if (restartPos > 0) { if (!file.seek(restartPos)) { - client.println(F("450 Cannot seek to restart position.")); - closeTransfer(); + DEBUG_PRINTLN(F("ERROR: cannot seek to restart position")); + abortTransfer(F("450 Cannot seek to restart position.")); return false; } bytesTransfered = restartPos; // Adjust the transferred bytes @@ -1642,7 +1640,7 @@ bool FtpServer::doRetrieve() if (written <= 0) { DEBUG_PRINTLN(F("ERROR: data.write returned <= 0")); - closeTransfer(); + abortTransfer(); return false; } @@ -1658,6 +1656,14 @@ bool FtpServer::doRetrieve() if (more > 0) written += more; } + // file.read() advanced the cursor by the full nb, so whatever went unsent must be re-read + // next round — otherwise those bytes vanish from the middle of the stream, silently. + if (written < nb && !file.seek(bytesTransfered + written)) { + DEBUG_PRINTLN(F("ERROR: cannot rewind after a short write")); + abortTransfer(); // the unsent bytes are unrecoverable — this is not a completed transfer + return false; + } + // Try to flush the socket where available (ESP-specific) #if defined(ESP8266) || defined(ESP32) data.flush(); @@ -1674,9 +1680,10 @@ bool FtpServer::doRetrieve() DEBUG_PRINT(F("DATA CONNECTED AFTER WRITE -> ")); DEBUG_PRINTLN(data.connected() ? 1 : 0); + // Reachable only with bytes still to send, so the peer left mid-file — closeTransfer() if (!data.connected()) { DEBUG_PRINTLN(F("Data socket closed by peer after write")); - closeTransfer(); + abortTransfer(); return false; } @@ -1715,8 +1722,8 @@ bool FtpServer::doStore() DEBUG_PRINT(F("No data received after ")); DEBUG_PRINT(waited); DEBUG_PRINTLN(F(" ms")); - // Decide to close transfer to avoid infinite loop and client timeout - closeTransfer(); + // Still connected but silent: a stalled upload. A peer ending a STOR closes the + abortTransfer(); return false; } // else continue and read available data below @@ -1770,9 +1777,8 @@ bool FtpServer::doStore() if( nb < 0 || rc == nb ) { return true; } - client.println(F("552 Probably insufficient storage space") ); - file.close(); - data.stop(); + + abortTransfer(F("552 Probably insufficient storage space")); return false; } @@ -2303,16 +2309,16 @@ void FtpServer::closeTransfer() data.stop(); + // Fires on every completed transfer, including an empty or sub-millisecond one. + if (FtpServer::_transferCallback) { + FtpServer::_transferCallback(FTP_TRANSFER_STOP, getFileName(&file).c_str(), bytesTransfered); + } + if( deltaT > 0 && bytesTransfered > 0 ) { DEBUG_PRINT( F(" Transfer completed in ") ); DEBUG_PRINT( deltaT ); DEBUG_PRINTLN( F(" ms, ") ); DEBUG_PRINT( bytesTransfered / deltaT ); DEBUG_PRINTLN( F(" kbytes/s") ); - if (FtpServer::_transferCallback) { - FtpServer::_transferCallback(FTP_TRANSFER_STOP, getFileName(&file).c_str(), bytesTransfered); - } - - client.println(F("226-File successfully transferred") ); client.print( F("226 ") ); client.print( deltaT ); client.print( F(" ms, ") ); client.print( bytesTransfered / deltaT ); client.println( F(" kbytes/s") ); @@ -2321,7 +2327,7 @@ void FtpServer::closeTransfer() client.println(F("226 File successfully transferred") ); } -void FtpServer::abortTransfer() +void FtpServer::abortTransfer(const __FlashStringHelper* reply) { if( transferStage == FTP_Custom ) // caller-driven transfer: finishCustom owns onEnd + response { @@ -2337,7 +2343,7 @@ void FtpServer::abortTransfer() #if STORAGE_TYPE != STORAGE_SPIFFS && STORAGE_TYPE != STORAGE_LITTLEFS && STORAGE_TYPE != STORAGE_SEEED_SD dir.close(); #endif - client.println(F("426 Transfer aborted") ); + client.println( reply ? reply : F("426 Transfer aborted") ); DEBUG_PRINTLN( F(" Transfer aborted!") ); transferStage = FTP_Close; diff --git a/FtpServer.h b/FtpServer.h index 19344e9..87d9acf 100644 --- a/FtpServer.h +++ b/FtpServer.h @@ -675,7 +675,9 @@ class FtpServer bool doList(); bool doMlsd(); void closeTransfer(); - void abortTransfer(); + // Ends a transfer as FAILED: closes the file, fires FTP_TRANSFER_ERROR, replies exactly once — + // `reply` replaces the default "426 Transfer aborted". Custom transfers: finishCustom() replies. + void abortTransfer(const __FlashStringHelper* reply = nullptr); bool makePath( char * fullName, char * param = nullptr ); bool makeExistsPath( char * path, char * param = nullptr ); bool openDir( FTP_DIR * pdir ); From d7c5cf0a2fbdb97dc6cd75f9899dafbe65637d13 Mon Sep 17 00:00:00 2001 From: srgg Date: Tue, 11 Aug 2026 16:30:29 -0600 Subject: [PATCH 3/3] feat: ride out a peer whose TCP window shuts, instead of ending the transfer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A zero-length socket write ended a download. It does not mean the peer is gone: the network client returns zero once its own retry budget expires, which on a memory-constrained board happens whenever the WiFi driver momentarily cannot allocate a transmit buffer. The window reopens seconds later. Downloads were being torn down for a condition that clears by itself — on the board this was first measured on, a 2.9 MB file died after 32 KB. doRetrieve() now treats a zero-length write as no progress rather than an ending: the file cursor is rewound so the same block is retried, the byte count and the download-progress callback stay put, and the transfer continues. Only rounds that actually sent something push the idle deadline out, so a peer that never comes back is ended by that deadline instead of running forever. The deadline could not do that job before. It was the tail of the if/else chain in handleFTP(), and a running transfer always took its own branch first — so it was unreachable during exactly the case that now needs bounding. It is now a check of its own, scoped to an idle command connection or a RETR: the other transfer types never refresh the deadline, and bounding them here would kill them mid-progress. --- FtpServer.cpp | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/FtpServer.cpp b/FtpServer.cpp index 08af9ab..b66a41e 100644 --- a/FtpServer.cpp +++ b/FtpServer.cpp @@ -451,10 +451,23 @@ uint8_t FtpServer::handleFTP() { if (!doCustom()) { transferStage = FTP_Close; } - } else if (cmdStage > FTP_Client + } + + // Out of the chain above, whose tail this was: a running transfer always took its own + // branch, so the deadline was never reached — and doRetrieve() now waits a stalled peer + // out instead of aborting, leaving nothing else to end it. RETR refreshes this deadline + // as it sends; the other types never do, so bounding them would kill them mid-progress. + const bool in_retrieve = (transferStage == FTP_Retrieve); + if (cmdStage > FTP_Client && (transferStage == FTP_Close || in_retrieve) && !((int32_t) (millisEndConnection - millis()) > 0)) { - DEBUG_PRINTLN(F("530 Timeout")); - client.println(F("530 Timeout")); + DEBUG_PRINTLN(F("Timeout")); + if (in_retrieve) { + // NOT closeTransfer(): that answers 226. abortTransfer() replies 426 and fires + // FTP_TRANSFER_ERROR, which releases what the app took. + abortTransfer(); + } else { + client.println(F("530 Timeout")); + } millisDelay = millis() + 200; // delay of 200 ms cmdStage = FTP_Stop; } @@ -1638,14 +1651,8 @@ bool FtpServer::doRetrieve() DEBUG_PRINT(F("WRITTEN --> ")); DEBUG_PRINTLN(written); - if (written <= 0) { - DEBUG_PRINTLN(F("ERROR: data.write returned <= 0")); - abortTransfer(); - return false; - } - // If partial write, try to send the remainder (best-effort) - if (written < nb) { + if (written > 0 && written < nb) { int16_t remaining = nb - written; DEBUG_PRINT(F("Partial write, attempting remainder -> ")); DEBUG_PRINTLN(remaining); @@ -1689,7 +1696,14 @@ bool FtpServer::doRetrieve() bytesTransfered += written; - if (FtpServer::_transferCallback) { + // Progress pushes the idle deadline out; a round that sent nothing deliberately does not — + // a zero write is often a transient shut window, and that deadline ends a peer really gone. + if (written > 0) { + millisEndConnection = millis() + 1000L * FTP_TIME_OUT; + } + + // Invoke callback on real progress: a stalled round must not look like a moving one to a watching app. + if (written > 0 && FtpServer::_transferCallback) { FtpServer::_transferCallback(FTP_DOWNLOAD, getFileName(&file).c_str(), bytesTransfered); }