From ac12f7d1c1c8e31c4800a734dd5f9894f1dee066 Mon Sep 17 00:00:00 2001 From: srgg Date: Tue, 11 Aug 2026 16:29:42 -0600 Subject: [PATCH 1/4] 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. (cherry picked from commit 6ca6f9cd5d8649751d6e88fd97ac80b1a7d42035) --- FtpServer.cpp | 50 ++++++++++++++++++++++++++++++-------------------- FtpServer.h | 5 ++++- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/FtpServer.cpp b/FtpServer.cpp index 65a2115..a444a7c 100644 --- a/FtpServer.cpp +++ b/FtpServer.cpp @@ -1393,9 +1393,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 +1494,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 @@ -1534,7 +1532,7 @@ bool FtpServer::doRetrieve() if (written <= 0) { DEBUG_PRINTLN(F("ERROR: data.write returned <= 0")); - closeTransfer(); + abortTransfer(); return false; } @@ -1550,6 +1548,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,9 +1572,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: the transfer failed. if (!data.connected()) { DEBUG_PRINTLN(F("Data socket closed by peer after write")); - closeTransfer(); + abortTransfer(); return false; } @@ -1607,8 +1614,9 @@ 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(); + // A peer that finished a STOR closes the data connection; one still connected and + // silent has stalled, so answering 226 would call an unfinished upload complete. + abortTransfer(); return false; } // else continue and read available data below @@ -1662,9 +1670,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 +2202,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,11 +2220,14 @@ void FtpServer::closeTransfer() client.println(F("226 File successfully transferred") ); } -void FtpServer::abortTransfer() +void FtpServer::abortTransfer(const __FlashStringHelper* reply) { if( transferStage != FTP_Close ) { - if (FtpServer::_transferCallback) { + // A listing has no file and no byte count of its own: reporting one would hand the + // application the name and total of whatever transfer ran before it. + const bool sending_file = ( transferStage == FTP_Retrieve || transferStage == FTP_Store ); + if (sending_file && FtpServer::_transferCallback) { FtpServer::_transferCallback(FTP_TRANSFER_ERROR, getFileName(&file).c_str(), bytesTransfered); } @@ -2225,7 +2235,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 c916866..6f8da4f 100644 --- a/FtpServer.h +++ b/FtpServer.h @@ -586,7 +586,10 @@ class FtpServer bool doList(); bool doMlsd(); void closeTransfer(); - void abortTransfer(); + // Ends a transfer as FAILED: closes the file and the directory, replies exactly once — + // `reply` replaces the default "426 Transfer aborted" — and fires FTP_TRANSFER_ERROR for the + // stages that carry a file, so a listing that loses its data connection reports no transfer. + 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 54796f850e5ea30cfc47c4399866a2f04c884a9c Mon Sep 17 00:00:00 2001 From: srgg Date: Tue, 11 Aug 2026 16:30:29 -0600 Subject: [PATCH 2/4] 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. (cherry picked from commit d7c5cf0a2fbdb97dc6cd75f9899dafbe65637d13) --- FtpServer.cpp | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/FtpServer.cpp b/FtpServer.cpp index a444a7c..cca088d 100644 --- a/FtpServer.cpp +++ b/FtpServer.cpp @@ -444,10 +444,23 @@ uint8_t FtpServer::handleFTP() { 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; } @@ -1530,14 +1543,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); @@ -1581,7 +1588,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); } From 20744d83a06b8adcaced7738de1bd9fdefecc5d8 Mon Sep 17 00:00:00 2001 From: srgg Date: Sat, 15 Aug 2026 00:38:00 -0600 Subject: [PATCH 3/4] fix: render a listing entry once, resume it if the peer takes part MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each entry went out as up to nine separate print() calls. A window that shuts mid-entry left half a line in the stream with the directory cursor already past it, so the rest of the listing was garbage. Each call also spends the socket's whole write budget again — ten seconds on ESP32 Arduino — so a peer that stopped reading held one handleFTP() call for ninety seconds. An entry is now rendered into a buffer sized from the limits this header already declares, and written once, the remainder pending until the peer takes it, as doRetrieve() does for a short write. A line too long keeps its CRLF, or it would merge into the next entry. Both paths write through one accountant, so neither sends without moving the deadline. --- FtpServer.cpp | 296 +++++++++++++++++++++----------------------------- FtpServer.h | 24 ++++ 2 files changed, 146 insertions(+), 174 deletions(-) diff --git a/FtpServer.cpp b/FtpServer.cpp index cca088d..f917201 100644 --- a/FtpServer.cpp +++ b/FtpServer.cpp @@ -880,6 +880,10 @@ bool FtpServer::processCommand() DEBUG_PRINTLN(F("Dir opened!!")); nbMatch = 0; + // Reset here rather than where a listing ends: a client that drops the data + // connection mid-entry ends one through neither closeTransfer() nor abortTransfer(), + // and the entry left pending would open the next listing. + listLineLen = listLineSent = 0; if( CommandIs( "LIST" )) transferStage = FTP_List; else if( CommandIs( "NLST" )) @@ -1536,7 +1540,7 @@ bool FtpServer::doRetrieve() { // write() may not send everything in one call on some clients; capture return int32_t written = 0; - written = data.write( buf, nb ); + written = writeData( (const uint8_t*) buf, nb ); DEBUG_PRINT(F("NB --> ")); DEBUG_PRINTLN(nb); @@ -1549,7 +1553,7 @@ bool FtpServer::doRetrieve() DEBUG_PRINT(F("Partial write, attempting remainder -> ")); DEBUG_PRINTLN(remaining); const uint8_t* p = (const uint8_t*)buf + written; - int32_t more = data.write(p, remaining); + int32_t more = writeData(p, remaining); DEBUG_PRINT(F("MORE WRITTEN -> ")); DEBUG_PRINTLN(more); if (more > 0) written += more; @@ -1588,12 +1592,6 @@ bool FtpServer::doRetrieve() bytesTransfered += written; - // 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); @@ -1689,59 +1687,17 @@ bool FtpServer::doStore() return false; } -void generateFileLine(FTP_CLIENT_NETWORK_CLASS* data, bool isDirectory, const char* fn, long fz, const char* time, const char* user, bool writeFilename = true) { - if( isDirectory ) { - // data->print( F("+/,\t") ); - // DEBUG_PRINT(F("+/,\t")); - - data->print( F("drwxrwsr-x\t2\t")); - data->print( user ); - data->print( F("\t") ); - data->print( long( 4096 ) ); - data->print( F("\t") ); - - DEBUG_PRINT( F("drwxrwsr-x\t2\t") ); - DEBUG_PRINT( user ); - DEBUG_PRINT( F("\t") ); - - DEBUG_PRINT( long( 4096 ) ); - DEBUG_PRINT( F("\t") ); - - data->print(time); - DEBUG_PRINT(time); - - data->print( F("\t") ); - if (writeFilename) data->println( fn ); - - DEBUG_PRINT( F("\t") ); - if (writeFilename) DEBUG_PRINTLN( fn ); - - } else { -// data.print( F("+r,s") ); -// DEBUG_PRINT(F("+r,s")); - - data->print( F("-rw-rw-r--\t1\t") ); - data->print( user ); - data->print( F("\t") ); - data->print( fz ); - data->print( F("\t") ); - - DEBUG_PRINT( F("-rw-rw-r--\t1\t") ); - DEBUG_PRINT( user ); - DEBUG_PRINT( F("\t") ); - DEBUG_PRINT( fz ); - DEBUG_PRINT( F("\t") ); - - data->print(time); - DEBUG_PRINT(time); - - data->print( F("\t") ); - if (writeFilename) data->println( fn ); - - DEBUG_PRINT( F("\t") ); - if (writeFilename) DEBUG_PRINTLN( fn ); - } - +// Renders one LIST entry into `out` and returns the length the whole line needs, which may +// exceed `outSize` — the caller decides what a line too long for its buffer becomes, and it +// cannot decide that from a length already clamped to the buffer. Building the line before +// any of it is sent is what lets a caller resume a partial write: nine separate print() +// calls could not, and each of them burns the socket's whole write budget again when the +// peer's window is shut. +size_t generateFileLine(char* out, size_t outSize, bool isDirectory, const char* fn, long fz, const char* time, const char* user) { + const int n = snprintf(out, outSize, "%s\t%s\t%ld\t%s\t%s\r\n", + isDirectory ? "drwxrwsr-x\t2" : "-rw-rw-r--\t1", + user, isDirectory ? 4096L : fz, time, fn); + return n < 0 ? 0 : (size_t) n; } #if defined(ESP32) || defined(ESP8266) || defined(ARDUINO_ARCH_RP2040) @@ -1793,11 +1749,69 @@ String makeDateTimeStrList(time_t ft, bool dateContracted = false) } // https://files.stairways.com/other/ftp-list-specs-info.txt -void generateFileLine(FTP_CLIENT_NETWORK_CLASS* data, bool isDirectory, const char* fn, long fz, time_t time, const char* user, bool writeFilename = true) { - generateFileLine(data, isDirectory, fn, fz, makeDateTimeStrList(time).c_str(), user, writeFilename); +size_t generateFileLine(char* out, size_t outSize, bool isDirectory, const char* fn, long fz, time_t time, const char* user) { + return generateFileLine(out, outSize, isDirectory, fn, fz, makeDateTimeStrList(time).c_str(), user); } #endif +// Renders one listing entry into listLine. Callers hand over the values their storage +// backend exposes; the wire format is the backend-independent part. +// Takes what snprintf() reported and returns the length to send. A line longer than the buffer +// is truncated by snprintf without its CRLF, and a listing entry with no line ending merges into +// the next one at the client, so the ending is restored over the last two bytes. +uint16_t FtpServer::finishListLine(int rendered) +{ + listLineSent = 0; + if( rendered <= 0 ) return 0; + if( (size_t) rendered < sizeof( listLine )) return (uint16_t) rendered; + listLine[ sizeof( listLine ) - 3 ] = '\r'; + listLine[ sizeof( listLine ) - 2 ] = '\n'; + listLine[ sizeof( listLine ) - 1 ] = '\0'; + return (uint16_t) ( sizeof( listLine ) - 1 ); +} + +// Renders one listing entry into listLine. Callers hand over the values their storage backend +// exposes; the wire format is the backend-independent part. +void FtpServer::buildListLine(bool isNlst, bool isDirectory, const char* fn, long fz, const char* time) +{ + const int n = isNlst ? snprintf( listLine, sizeof( listLine ), "%s\r\n", fn ) + : (int) generateFileLine( listLine, sizeof( listLine ), isDirectory, fn, fz, time, this->user ); + listLineLen = finishListLine( n ); + DEBUG_PRINT( listLine ); +} + +void FtpServer::buildListLine(bool isNlst, bool isDirectory, const char* fn, long fz, time_t time) +{ + buildListLine( isNlst, isDirectory, fn, fz, makeDateTimeStrList( time ).c_str()); +} + +void FtpServer::buildMlsdLine(bool isDirectory, const char* dtStr, long fz, const char* fn) +{ + const int n = snprintf( listLine, sizeof( listLine ), "Type=%s;Modify=%s;Size=%ld; %s\r\n", + isDirectory ? "dir" : "file", dtStr, fz, fn ); + listLineLen = finishListLine( n ); + DEBUG_PRINT( listLine ); +} + +size_t FtpServer::writeData(const uint8_t* p, size_t len) +{ + const size_t n = data.write( p, len ); + if( n > 0 ) millisEndConnection = millis() + 1000L * FTP_TIME_OUT; + return n; +} + +bool FtpServer::sendListLine() +{ + if( listLineLen == 0 ) return true; + listLineSent += (uint16_t) writeData((const uint8_t*) listLine + listLineSent, + listLineLen - listLineSent ); + if( listLineSent < listLineLen ) return false; + listLineLen = 0; + listLineSent = 0; + nbMatch ++; + return true; +} + bool FtpServer::doList() { if( ! dataConnected()) @@ -1808,6 +1822,10 @@ bool FtpServer::doList() return false; } + // An entry already rendered owns this round: its directory slot is gone, so the + // remainder has to go out before the cursor may move again. + if( listLineLen > 0 && ! sendListLine()) return true; + // Determine if current transfer is NLST (name list) so we only send filenames bool isNlst = (transferStage == FTP_Nlst); #if STORAGE_TYPE == STORAGE_SPIFFS @@ -1824,12 +1842,7 @@ bool FtpServer::doList() long fz = long( dir.fileSize()); if (fn[0]=='/') { fn.remove(0, fn.lastIndexOf("/")+1); } time_t time = dir.fileTime(); - if (isNlst) { - data.println(fn.c_str()); - DEBUG_PRINTLN(fn); - } else { - generateFileLine(&data, false, fn.c_str(), fz, time, this->user); - } + buildListLine( isNlst, false, fn.c_str(), fz, time ); #else long fz = long( fileDir.size()); const char* fnC = fileDir.name(); @@ -1841,16 +1854,11 @@ bool FtpServer::doList() } time_t time = fileDir.getLastWrite(); - if (isNlst) { - data.println(fn); - DEBUG_PRINTLN(fn); - } else { - generateFileLine(&data, false, fn, fz, time, this->user); - } + buildListLine( isNlst, false, fn, fz, time ); #endif - nbMatch ++; + sendListLine(); return true; } #elif STORAGE_TYPE == STORAGE_LITTLEFS || STORAGE_TYPE == STORAGE_SEEED_SD || STORAGE_TYPE == STORAGE_FFAT @@ -1897,21 +1905,14 @@ bool FtpServer::doList() // DEBUG_PRINT( F("\t") ); // DEBUG_PRINTLN( fileDir.name() ); #endif - if (isNlst) { - data.println(fn); - DEBUG_PRINTLN(fn); - } else { - #if defined(ESP8266) || defined(ARDUINO_ARCH_RP2040) - time_t time = dir.fileTime(); - generateFileLine(&data, dir.isDirectory(), fn, fz, time, this->user); - #elif defined(ESP32) - time_t time = fileDir.getLastWrite(); - generateFileLine(&data, fileDir.isDirectory(), fn, fz, time, this->user); - #else - generateFileLine(&data, fileDir.isDirectory(), fn, fz, "Jan 01 00:00", this->user); - #endif - } - nbMatch ++; + #if defined(ESP8266) || defined(ARDUINO_ARCH_RP2040) + buildListLine( isNlst, dir.isDirectory(), fn, fz, dir.fileTime()); + #elif defined(ESP32) + buildListLine( isNlst, fileDir.isDirectory(), fn, fz, fileDir.getLastWrite()); + #else + buildListLine( isNlst, fileDir.isDirectory(), fn, fz, "Jan 01 00:00" ); + #endif + sendListLine(); return true; } #elif STORAGE_TYPE == STORAGE_SD || STORAGE_TYPE == STORAGE_SD_MMC @@ -1924,22 +1925,12 @@ bool FtpServer::doList() #if STORAGE_TYPE == STORAGE_SD_MMC time_t time = fileDir.getLastWrite(); - if (isNlst) { - data.println(fn.c_str()); - DEBUG_PRINTLN(fn); - } else { - generateFileLine(&data, fileDir.isDirectory(), fn.c_str(), long( fileDir.size()), time, this->user); - } + buildListLine( isNlst, fileDir.isDirectory(), fn.c_str(), long( fileDir.size()), time ); #else - if (isNlst) { - data.println(fn.c_str()); - DEBUG_PRINTLN(fn); - } else { - generateFileLine(&data, fileDir.isDirectory(), fn.c_str(), long( fileDir.size()), "Jan 01 00:00", this->user); - } + buildListLine( isNlst, fileDir.isDirectory(), fn.c_str(), long( fileDir.size()), "Jan 01 00:00" ); #endif - nbMatch ++; + sendListLine(); return true; } @@ -1950,31 +1941,19 @@ bool FtpServer::doList() String fn = dir.fileName(); if (fn[0]=='/') { fn.remove(0, fn.lastIndexOf("/")+1); } - if (isNlst) { - data.println(fn.c_str()); - DEBUG_PRINTLN(fn); - } else { - generateFileLine(&data, dir.isDir(), fn.c_str(), long( dir.fileSize()), "Jan 01 00:00", this->user); - } + buildListLine( isNlst, dir.isDir(), fn.c_str(), long( dir.fileSize()), "Jan 01 00:00" ); - nbMatch ++; + sendListLine(); return true; } #else if( file.openNext( &dir, FTP_FILE_READ_ONLY )) { - // For storages using file.printName, only send name in NLST mode - if (isNlst) { - file.printName(&data); - data.println(); - } else { - generateFileLine(&data, file.isDir(), "", long( fileSize( file )), "Jan 01 00:00", this->user, false); - - file.printName( & data ); - data.println(); - } + char nameBuf[ FTP_CWD_SIZE ]; + file.getName( nameBuf, sizeof( nameBuf )); + buildListLine( isNlst, file.isDir(), nameBuf, long( fileSize( file )), "Jan 01 00:00" ); file.close(); - nbMatch ++; + sendListLine(); return true; } #endif @@ -1999,6 +1978,10 @@ bool FtpServer::doMlsd() DEBUG_PRINTLN(F("Not connected!!")); return false; } + // An entry already rendered owns this round: its directory slot is gone, so the + // remainder has to go out before the cursor may move again. + if( listLineLen > 0 && ! sendListLine()) return true; + DEBUG_PRINTLN(F("Connected!!")); #if STORAGE_TYPE == STORAGE_SPIFFS @@ -2038,21 +2021,8 @@ bool FtpServer::doMlsd() long fz = fileDir.size(); #endif - data.print( F("Type=") ); - - data.print( F("file") ); - data.print( F(";Modify=") ); data.print(dtStr);// data.print( makeDateTimeStr( dtStr, time, time) ); - data.print( F(";Size=") ); data.print( fz ); - data.print( F("; ") ); data.println( fn ); - - DEBUG_PRINT( F("Type=") ); - DEBUG_PRINT( F("file") ); - - DEBUG_PRINT( F(";Modify=") ); DEBUG_PRINT(dtStr); //DEBUG_PRINT( makeDateTimeStr( dtStr, time, time) ); - DEBUG_PRINT( F(";Size=") ); DEBUG_PRINT( fz ); - DEBUG_PRINT( F("; ") ); DEBUG_PRINTLN( fn ); - - nbMatch ++; + buildMlsdLine( false, dtStr, fz, fn.c_str()); + sendListLine(); return true; } #elif STORAGE_TYPE == STORAGE_LITTLEFS || STORAGE_TYPE == STORAGE_SEEED_SD || STORAGE_TYPE == STORAGE_FFAT @@ -2102,14 +2072,13 @@ bool FtpServer::doMlsd() #endif #if defined(ESP8266) || defined(ARDUINO_ARCH_RP2040) time_t time = dir.fileTime(); - generateFileLine(&data, dir.isDirectory(), fn, fz, time, this->user); + buildListLine( false, dir.isDirectory(), fn, fz, time ); #elif defined(ESP32) - time_t time = fileDir.getLastWrite(); - generateFileLine(&data, fileDir.isDirectory(), fn, fz, time, this->user); + buildListLine( false, fileDir.isDirectory(), fn, fz, fileDir.getLastWrite()); #else - generateFileLine(&data, fileDir.isDirectory(), fn, fz, "Jan 01 00:00", this->user); + buildListLine( false, fileDir.isDirectory(), fn, fz, "Jan 01 00:00" ); #endif - nbMatch ++; + sendListLine(); return true; } #elif STORAGE_TYPE == STORAGE_SD || STORAGE_TYPE == STORAGE_SD_MMC @@ -2132,21 +2101,8 @@ bool FtpServer::doMlsd() - data.print( F("Type=") ); - - data.print( ( fileDir.isDirectory() ? F("dir") : F("file")) ); - data.print( F(";Modify=") ); data.print(dtStr);// data.print( makeDateTimeStr( dtStr, time, time) ); - data.print( F(";Size=") ); data.print( fz ); - data.print( F("; ") ); data.println( fn ); - - DEBUG_PRINT( F("Type=") ); - DEBUG_PRINT( ( fileDir.isDirectory() ? F("dir") : F("file")) ); - - DEBUG_PRINT( F(";Modify=") ); DEBUG_PRINT(dtStr); //DEBUG_PRINT( makeDateTimeStr( dtStr, time, time) ); - DEBUG_PRINT( F(";Size=") ); DEBUG_PRINT( fz ); - DEBUG_PRINT( F("; ") ); DEBUG_PRINTLN( fn ); - - nbMatch ++; + buildMlsdLine( fileDir.isDirectory(), dtStr, fz, fn.c_str()); + sendListLine(); return true; } @@ -2154,11 +2110,10 @@ bool FtpServer::doMlsd() if( dir.nextFile()) { char dtStr[ 15 ]; - data.print( F("Type=") ); data.print( ( dir.isDir() ? F("dir") : F("file")) ); - data.print( F(";Modify=") ); data.print( makeDateTimeStr( dtStr, dir.fileModDate(), dir.fileModTime()) ); - data.print( F(";Size=") ); data.print( long( dir.fileSize()) ); - data.print( F("; ") ); data.println( dir.fileName() ); - nbMatch ++; + String fn = dir.fileName(); + buildMlsdLine( dir.isDir(), makeDateTimeStr( dtStr, dir.fileModDate(), dir.fileModTime()), + long( dir.fileSize()), fn.c_str()); + sendListLine(); return true; } #else @@ -2171,18 +2126,11 @@ bool FtpServer::doMlsd() DEBUG_PRINTLN(gfmt); if( gfmt ) { - data.print( F("Type=") ); data.print( ( file.isDir() ? F("dir") : F("file")) ); - data.print( F(";Modify=") ); data.print( makeDateTimeStr( dtStr, filelwd, filelwt ) ); - data.print( F(";Size=") ); data.print( long( fileSize( file )) ); data.print( F("; ") ); - file.printName( & data ); - data.println(); - - DEBUG_PRINT( F("Type=") ); DEBUG_PRINT( ( file.isDir() ? F("dir") : F("file")) ); - DEBUG_PRINT( F(";Modify=") ); DEBUG_PRINT( makeDateTimeStr( dtStr, filelwd, filelwt ) ); - DEBUG_PRINT( F(";Size=") ); DEBUG_PRINT( long( fileSize( file )) ); DEBUG_PRINT( F("; ") ); -// DEBUG_PRINT(file.name()); - DEBUG_PRINTLN(); - nbMatch ++; + char nameBuf[ FTP_CWD_SIZE ]; + file.getName( nameBuf, sizeof( nameBuf )); + buildMlsdLine( file.isDir(), makeDateTimeStr( dtStr, filelwd, filelwt ), + long( fileSize( file )), nameBuf ); + sendListLine(); } file.close(); return gfmt; diff --git a/FtpServer.h b/FtpServer.h index 6f8da4f..e546459 100644 --- a/FtpServer.h +++ b/FtpServer.h @@ -502,6 +502,10 @@ #define FTP_CWD_SIZE FF_MAX_LFN+8 // max size of a directory name #define FTP_FIL_SIZE FF_MAX_LFN // max size of a file name #define FTP_CRED_SIZE 16 // max size of username and password +// One rendered listing entry: the two names at the limits above, the widest a long prints, +// the date makeDateTimeStrList() builds in its own char[25], the "Type=…;Size=…; " prefix, +// CRLF and the terminator. +#define FTP_LIST_LINE_SIZE (FTP_FIL_SIZE + FTP_CRED_SIZE + 64) #define FTP_NULLIP() IPAddress(0,0,0,0) enum ftpCmd { FTP_Stop = 0, // In this stage, stop any connection @@ -585,6 +589,18 @@ class FtpServer bool doStore(); bool doList(); bool doMlsd(); + // Sends what is still pending of listLine. True once the whole entry is away — only then + // is it counted in nbMatch, and any byte accepted pushes the idle deadline out, so a + // listing rides out a shut window exactly as a retrieve does. + bool sendListLine(); + // The one path from this server to the data socket. Returns what the socket took, and a + // non-zero take is what pushes the idle deadline out — so no transfer path can send bytes + // without the deadline noticing, or move the deadline without sending any. + size_t writeData(const uint8_t* p, size_t len); + void buildListLine(bool isNlst, bool isDirectory, const char* fn, long fz, const char* time); + void buildListLine(bool isNlst, bool isDirectory, const char* fn, long fz, time_t time); + void buildMlsdLine(bool isDirectory, const char* dtStr, long fz, const char* fn); + uint16_t finishListLine(int rendered); void closeTransfer(); // Ends a transfer as FAILED: closes the file and the directory, replies exactly once — // `reply` replaces the default "426 Transfer aborted" — and fires FTP_TRANSFER_ERROR for the @@ -834,6 +850,14 @@ class FtpServer uint16_t iCL; // pointer to cmdLine next incoming char uint16_t nbMatch; + // One listing entry, rendered whole before any of it is sent. The directory cursor has + // already moved past the entry by then, so a line the peer only half accepts has to be + // resumed from here — re-reading it is impossible, and a half line left in the stream + // corrupts every entry after it. + char listLine[ FTP_LIST_LINE_SIZE ]; + uint16_t listLineLen = 0; // rendered length; 0 = no entry pending + uint16_t listLineSent = 0; // how much of it the socket has taken + uint32_t millisDelay, // millisEndConnection, // millisBeginTrans, // store time of beginning of a transaction From ff82e52a19532b7317afc6fc393ef8d7a78d79bf Mon Sep 17 00:00:00 2001 From: srgg Date: Thu, 27 Aug 2026 01:21:42 -0600 Subject: [PATCH 4/4] fix(FtpServer): guard FF_MAX_LFN to prevent redefinition with framework FatFs SimpleFTPServer and framework FatFs both define FF_MAX_LFN. Without the guard, including both headers causes a redefinition error under -Werror. --- FtpServer.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/FtpServer.h b/FtpServer.h index e546459..c4b381e 100644 --- a/FtpServer.h +++ b/FtpServer.h @@ -497,7 +497,10 @@ #define FTP_DATA_PORT_DFLT 20 // Default data port in active mode #define FTP_DATA_PORT_PASV 50009 // Data port in passive mode +#ifndef FF_MAX_LFN #define FF_MAX_LFN 255 // max size of a long file name +#endif + #define FTP_CMD_SIZE FF_MAX_LFN+8 // max size of a command #define FTP_CWD_SIZE FF_MAX_LFN+8 // max size of a directory name #define FTP_FIL_SIZE FF_MAX_LFN // max size of a file name