From b93239bb0a7a2b642fab90de2d5e62c7195a8ec5 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Wed, 24 Jun 2026 12:38:46 -0500 Subject: [PATCH] fix(phone-api): skip manifest scan for node-info-only requests; bound getFiles (#10778) Forward-port of #10754 and #10757 from master (2.7) into develop, so the develop->master 2.8 promotion (#10777) doesn't drop them. #10754: PhoneAPI no longer walks the filesystem to build the file manifest on node-info-only config requests (SPECIAL_NONCE_ONLY_NODES), which never consume it. getFiles() is now bounded (default 64 entries, depth 3) via collectFiles(), takes an optional wasLimited out-param, and reserves capacity with a bad_alloc/ length_error fallback. The manifest vector is freed via swap (releaseFilesManifest). #10757: getFiles()/collectFiles() now guard against empty file names returned by the Adafruit LittleFS nRF52 glue (issue 4395). Ported by hand rather than cherry-picked: master had reflowed FSCommon.cpp to a different brace style (every line conflicted), #10754 already subsumes #10757, and develop carries a MESHTASTIC_EXCLUDE_FILES_MANIFEST path (nRF54L15) that master lacks. The exclude path is preserved and now also short-circuits + frees the manifest. Verified: native Docker suite 448/448, clang-format clean. --- src/FSCommon.cpp | 149 +++++++++++++++++++++++++++++++++--------- src/FSCommon.h | 2 +- src/mesh/PhoneAPI.cpp | 39 ++++++++--- 3 files changed, 150 insertions(+), 40 deletions(-) diff --git a/src/FSCommon.cpp b/src/FSCommon.cpp index a229dcbfc..7b0595aa9 100644 --- a/src/FSCommon.cpp +++ b/src/FSCommon.cpp @@ -88,6 +88,9 @@ bool renameFile(const char *pathFrom, const char *pathTo) #endif } +#include +#include +#include #include /** @@ -119,6 +122,93 @@ bool fsFormat() #endif } +#ifdef FSCom +namespace +{ +bool pathEndsWithDot(const char *path) +{ + if (!path) + return false; + + size_t length = strlen(path); + return length > 0 && path[length - 1] == '.'; +} + +bool copyFilePath(char *dest, size_t destSize, const char *path, bool *wasLimited) +{ + if (!path || destSize == 0) { + if (wasLimited) + *wasLimited = true; + return false; + } + + if (strlcpy(dest, path, destSize) >= destSize) { + if (wasLimited) + *wasLimited = true; + return false; + } + + return true; +} + +void collectFiles(const char *dirname, uint8_t levels, size_t maxCount, std::vector &filenames, + bool *wasLimited) +{ + if (!dirname) + return; + + File root = FSCom.open(dirname, FILE_O_READ); + if (!root) + return; + if (!root.isDirectory()) { + root.close(); + return; + } + + File file = root.openNextFile(); + // file.name()[0] check is a workaround for a bug in the Adafruit LittleFS nrf52 glue (see issue 4395) + while (file && file.name()[0]) { + if (filenames.size() >= maxCount) { + if (wasLimited) + *wasLimited = true; + file.close(); + break; + } + const char *fileName = file.name(); + if (file.isDirectory() && !pathEndsWithDot(fileName)) { + char pathBuffer[sizeof(((meshtastic_FileInfo *)nullptr)->file_name)] = {}; +#ifdef ARCH_ESP32 + const char *subDirPath = file.path(); +#else + const char *subDirPath = fileName; +#endif + bool hasSubDirPath = copyFilePath(pathBuffer, sizeof(pathBuffer), subDirPath, wasLimited); + file.close(); + + if (levels && hasSubDirPath) { + collectFiles(pathBuffer, levels - 1, maxCount, filenames, wasLimited); + } else if (wasLimited) { + *wasLimited = true; + } + } else { + meshtastic_FileInfo fileInfo = {"", static_cast(file.size())}; +#ifdef ARCH_ESP32 + bool hasFilePath = copyFilePath(fileInfo.file_name, sizeof(fileInfo.file_name), file.path(), wasLimited); +#else + bool hasFilePath = copyFilePath(fileInfo.file_name, sizeof(fileInfo.file_name), file.name(), wasLimited); +#endif + if (hasFilePath && !pathEndsWithDot(fileInfo.file_name)) { + filenames.push_back(fileInfo); + } + file.close(); + } + file = root.openNextFile(); + } + root.close(); +} +} // namespace +#endif + /** * @brief Get the list of files in a directory. * @@ -127,43 +217,40 @@ bool fsFormat() * * @param dirname The name of the directory. * @param levels The number of levels of subdirectories to list. - * @return A vector of strings containing the full path of each file in the directory. + * @param maxCount The maximum number of files to collect before truncating the walk. + * @param wasLimited Optional out-param, set to true if the listing was truncated (by maxCount or low memory). + * @return A vector of meshtastic_FileInfo for each file in the directory. */ -std::vector getFiles(const char *dirname, uint8_t levels) +std::vector getFiles(const char *dirname, uint8_t levels, size_t maxCount, bool *wasLimited) { std::vector filenames = {}; + if (wasLimited) + *wasLimited = false; #ifdef FSCom - File root = FSCom.open(dirname, FILE_O_READ); - if (!root) - return filenames; - if (!root.isDirectory()) - return filenames; - - File file = root.openNextFile(); - while (file) { -#ifdef ARCH_ESP32 - const char *filepath = file.path(); -#else - const char *filepath = file.name(); -#endif - if (file.isDirectory() && !String(file.name()).endsWith(".")) { - if (levels) { - std::vector subDirFilenames = getFiles(filepath, levels - 1); - filenames.insert(filenames.end(), subDirFilenames.begin(), subDirFilenames.end()); - file.close(); - } - } else { - meshtastic_FileInfo fileInfo = {"", static_cast(file.size())}; - strncpy(fileInfo.file_name, filepath, sizeof(fileInfo.file_name) - 1); - fileInfo.file_name[sizeof(fileInfo.file_name) - 1] = '\0'; - if (!String(fileInfo.file_name).endsWith(".")) { - filenames.push_back(fileInfo); - } - file.close(); +#if defined(__cpp_exceptions) || defined(__EXCEPTIONS) + size_t reservedCount = maxCount; + while (reservedCount > 0) { + try { + filenames.reserve(reservedCount); + break; + } catch (const std::bad_alloc &) { + reservedCount /= 2; + } catch (const std::length_error &) { + reservedCount /= 2; } - file = root.openNextFile(); } - root.close(); + if (reservedCount == 0) { + if (wasLimited) + *wasLimited = true; + return filenames; + } + if (reservedCount < maxCount) { + if (wasLimited) + *wasLimited = true; + maxCount = reservedCount; + } +#endif + collectFiles(dirname, levels, maxCount, filenames, wasLimited); #endif return filenames; } diff --git a/src/FSCommon.h b/src/FSCommon.h index 2f86928af..bccbc600b 100644 --- a/src/FSCommon.h +++ b/src/FSCommon.h @@ -61,7 +61,7 @@ void fsListFiles(); bool copyFile(const char *from, const char *to); bool renameFile(const char *pathFrom, const char *pathTo); bool fsFormat(); -std::vector getFiles(const char *dirname, uint8_t levels); +std::vector getFiles(const char *dirname, uint8_t levels, size_t maxCount = 64, bool *wasLimited = nullptr); void listDir(const char *dirname, uint8_t levels, bool del = false); void rmDir(const char *dirname); void setupSDCard(); \ No newline at end of file diff --git a/src/mesh/PhoneAPI.cpp b/src/mesh/PhoneAPI.cpp index 5ee553bdc..884cbfae1 100644 --- a/src/mesh/PhoneAPI.cpp +++ b/src/mesh/PhoneAPI.cpp @@ -40,6 +40,17 @@ #include "Throttle.h" #include +namespace +{ +constexpr uint8_t FILES_MANIFEST_LEVELS = 3; +constexpr size_t FILES_MANIFEST_MAX_COUNT = 64; + +void releaseFilesManifest(std::vector &filesManifest) +{ + std::vector().swap(filesManifest); +} +} // namespace + // Flag to indicate a heartbeat was received and we should send queue status bool heartbeatReceived = false; @@ -298,18 +309,31 @@ void PhoneAPI::handleStartConfig() state = STATE_SEND_MY_INFO; } pauseBluetoothLogging = true; - spiLock->lock(); #if defined(MESHTASTIC_EXCLUDE_FILES_MANIFEST) // Skip the recursive FS walk. Used by platforms whose Zephyr LittleFS // backend can't safely traverse a deep tree (e.g. nRF54L15) and platforms // that don't support OTA browsing — the manifest is only consumed by // companion apps for those flows. - filesManifest.clear(); + releaseFilesManifest(filesManifest); #else - filesManifest = getFiles("/", 10); + // Manifest is never read on the node-info-only path (STATE_SEND_FILEMANIFEST + // short-circuits to sendConfigComplete), so skip the SPI lock + FS walk. + if (config_nonce != SPECIAL_NONCE_ONLY_NODES) { + bool filesManifestLimited = false; + { + concurrency::LockGuard guard(spiLock); + filesManifest = getFiles("/", FILES_MANIFEST_LEVELS, FILES_MANIFEST_MAX_COUNT, &filesManifestLimited); + } + if (filesManifestLimited) { + LOG_WARN("Got %zu files in manifest (limited to %zu entries/depth %u)", filesManifest.size(), + FILES_MANIFEST_MAX_COUNT, static_cast(FILES_MANIFEST_LEVELS)); + } else { + LOG_DEBUG("Got %zu files in manifest", filesManifest.size()); + } + } else { + releaseFilesManifest(filesManifest); + } #endif - spiLock->unlock(); - LOG_DEBUG("Got %d files in manifest", filesManifest.size()); LOG_INFO("Start API client config millis=%u", millis()); // Protect against concurrent BLE callbacks: they run in NimBLE's FreeRTOS task and also touch nodeInfoQueue. @@ -377,8 +401,7 @@ void PhoneAPI::close() replayPhase = REPLAY_PHASE_IDLE; } packetForPhone = NULL; - filesManifest.clear(); - filesManifest.shrink_to_fit(); + releaseFilesManifest(filesManifest); lastPortNumToRadio.clear(); fromRadioNum = 0; config_nonce = 0; @@ -943,7 +966,7 @@ size_t PhoneAPI::getFromRadio(uint8_t *buf) // ONLY_NODES variants skip the manifest. if (config_state == filesManifest.size() || config_nonce == SPECIAL_NONCE_ONLY_NODES) { config_state = 0; - filesManifest.clear(); + releaseFilesManifest(filesManifest); // Skip to complete packet sendConfigComplete(); } else {