Add a universal command hook (FtpResponse facade) + custom cooperative transfers

FtpServer.cpp

@@ -50,6 +50,7 @@
 #include <FtpServer.h>
 #include <ctype.h>
 #include <stdlib.h>
+#include <string.h>
 
 // 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,13 +444,30 @@ uint8_t FtpServer::handleFTP() {
 		} else if (transferStage == FTP_Mlsd)  // MLSD listing
 				{
 			if (!doMlsd()) {
-
 				transferStage = FTP_Close;
 			}
-		} else if (cmdStage > FTP_Client
+		} else if (transferStage == FTP_Custom) // caller-driven cooperative transfer
+				{
+			if (!doCustom()) {
+				transferStage = FTP_Close;
+			}
+		}
+
+		// 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;
 		}
@@ -508,6 +528,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 +625,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 
   //
@@ -1393,9 +1514,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;
 }
 
@@ -1496,8 +1615,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
@@ -1532,14 +1651,8 @@ bool FtpServer::doRetrieve()
     DEBUG_PRINT(F("WRITTEN --> "));
     DEBUG_PRINTLN(written);
 
-    if (written <= 0) {
-      DEBUG_PRINTLN(F("ERROR: data.write returned <= 0"));
-      closeTransfer();
-      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);
@@ -1550,6 +1663,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();
@@ -1566,15 +1687,23 @@ 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;
     }
 
     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);
 	  }
 
@@ -1607,8 +1736,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
@@ -1662,9 +1791,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;
 }
 
@@ -2195,16 +2323,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") );
@@ -2213,9 +2341,13 @@ void FtpServer::closeTransfer()
     client.println(F("226 File successfully transferred") );
 }
 
-void FtpServer::abortTransfer()
+void FtpServer::abortTransfer(const __FlashStringHelper* reply)
 {
-  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);
@@ -2225,7 +2357,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;