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.
This commit is contained in:
+118
-31
@@ -88,6 +88,9 @@ bool renameFile(const char *pathFrom, const char *pathTo)
|
||||
#endif
|
||||
}
|
||||
|
||||
#include <cstring>
|
||||
#include <new>
|
||||
#include <stdexcept>
|
||||
#include <vector>
|
||||
|
||||
/**
|
||||
@@ -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<meshtastic_FileInfo> &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<uint32_t>(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<meshtastic_FileInfo> getFiles(const char *dirname, uint8_t levels)
|
||||
std::vector<meshtastic_FileInfo> getFiles(const char *dirname, uint8_t levels, size_t maxCount, bool *wasLimited)
|
||||
{
|
||||
std::vector<meshtastic_FileInfo> 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<meshtastic_FileInfo> subDirFilenames = getFiles(filepath, levels - 1);
|
||||
filenames.insert(filenames.end(), subDirFilenames.begin(), subDirFilenames.end());
|
||||
file.close();
|
||||
}
|
||||
} else {
|
||||
meshtastic_FileInfo fileInfo = {"", static_cast<uint32_t>(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;
|
||||
}
|
||||
|
||||
+1
-1
@@ -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<meshtastic_FileInfo> getFiles(const char *dirname, uint8_t levels);
|
||||
std::vector<meshtastic_FileInfo> 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();
|
||||
+31
-8
@@ -40,6 +40,17 @@
|
||||
#include "Throttle.h"
|
||||
#include <RTC.h>
|
||||
|
||||
namespace
|
||||
{
|
||||
constexpr uint8_t FILES_MANIFEST_LEVELS = 3;
|
||||
constexpr size_t FILES_MANIFEST_MAX_COUNT = 64;
|
||||
|
||||
void releaseFilesManifest(std::vector<meshtastic_FileInfo> &filesManifest)
|
||||
{
|
||||
std::vector<meshtastic_FileInfo>().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<unsigned>(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 {
|
||||
|
||||
Reference in New Issue
Block a user