diff --git a/examples/simple_room_server/MyMesh.cpp b/examples/simple_room_server/MyMesh.cpp index 48643172f..ceed8c15f 100644 --- a/examples/simple_room_server/MyMesh.cpp +++ b/examples/simple_room_server/MyMesh.cpp @@ -46,6 +46,7 @@ #include #include #include +#include #include #include #endif @@ -579,7 +580,11 @@ void MyMesh::processRoomWebBoard(const char* operation, JsonObjectConst input, json.reset(); json.raw("{\"error\":"); json.string(message); json.raw("}"); json.finish(); }; - mesh::RoomBoardIndex board; + // Metadata remains live through filesystem calls. Keep it off the 8 KiB + // loop task stack and fail before storage access if scratch allocation fails. + std::unique_ptr scratch(new (std::nothrow) mesh::RoomBoardIndex); + if (!scratch) { error("insufficient memory; information board retained"); return; } + mesh::RoomBoardIndex& board = *scratch; if (!mesh::loadRoomBoard(_fs, board)) { error("information board storage unavailable"); return; } if (strcmp(operation, "board.index") == 0) { if (!input["revision"].is() || !input["cursor"].is()) { error("invalid board cursor"); return; } @@ -602,25 +607,49 @@ void MyMesh::processRoomWebBoard(const char* operation, JsonObjectConst input, json.raw("],\"next\":"); json.number(next < board.count ? next : 255); json.raw("}"); json.finish(); return; } + if (strcmp(operation, "board.read") == 0) { + processRoomWebBoardRead(input, board, json); return; + } + processRoomWebBoardWrite(operation, input, board, json); +} + +void MyMesh::processRoomWebBoardRead(JsonObjectConst input, const mesh::RoomBoardIndex& board, + mesh::RoomJsonWriter& json) { + auto error = [&json](const char* message) { + json.reset(); + json.raw("{\"error\":"); json.string(message); json.raw("}"); json.finish(); + }; + if (!input["id"].is() || !input["version"].is()) { error("invalid article identity/version"); return; } + const uint8_t id = input["id"].as(); + const uint32_t version = input["version"].as(); + const mesh::RoomBoardArticle* article = nullptr; + for (uint8_t i = 0; i < board.count; ++i) if (board.articles[i].id == id) article = &board.articles[i]; + if (!input["offset"].is()) { error("invalid article offset"); return; } + const uint16_t offset = input["offset"].as(); + uint8_t chunk[mesh::ROOM_BOARD_READ_MAX_CHUNK]; + size_t length = 0; + const auto result = mesh::readRoomBoardArticle(_fs, id, version, offset, chunk, sizeof(chunk), length); + if (result != mesh::RoomBoardResult::Success || !article) { error("article changed or unavailable; reload index"); return; } + char encoded[4 * ((sizeof(chunk) + 2) / 3) + 1]; mesh::roomEncodeBase64(chunk, length, encoded); + json.raw("{\"version\":"); json.number(version); + json.raw(",\"offset\":"); json.number(offset); + json.raw(",\"next\":"); json.number(uint32_t(offset) + length); + json.raw(",\"total\":"); json.number(article->body_length); + json.raw(",\"data64\":"); json.string(encoded); json.raw("}"); json.finish(); +} + +void MyMesh::processRoomWebBoardWrite(const char* operation, JsonObjectConst input, + const mesh::RoomBoardIndex& board, + mesh::RoomJsonWriter& json) { + auto error = [&json](const char* message) { + json.reset(); + json.raw("{\"error\":"); json.string(message); json.raw("}"); json.finish(); + }; if (!input["id"].is() || !input["version"].is()) { error("invalid article identity/version"); return; } const uint8_t id = input["id"].as(); const uint32_t version = input["version"].as(); const mesh::RoomBoardArticle* article = nullptr; for (uint8_t i = 0; i < board.count; ++i) if (board.articles[i].id == id) article = &board.articles[i]; - if (strcmp(operation, "board.read") == 0) { - if (!input["offset"].is()) { error("invalid article offset"); return; } - const uint16_t offset = input["offset"].as(); - uint8_t chunk[mesh::ROOM_BOARD_READ_MAX_CHUNK]; - size_t length = 0; - const auto result = mesh::readRoomBoardArticle(_fs, id, version, offset, chunk, sizeof(chunk), length); - if (result != mesh::RoomBoardResult::Success || !article) { error("article changed or unavailable; reload index"); return; } - char encoded[4 * ((sizeof(chunk) + 2) / 3) + 1]; mesh::roomEncodeBase64(chunk, length, encoded); - json.raw("{\"version\":"); json.number(version); - json.raw(",\"offset\":"); json.number(offset); - json.raw(",\"next\":"); json.number(uint32_t(offset) + length); - json.raw(",\"total\":"); json.number(article->body_length); - json.raw(",\"data64\":"); json.string(encoded); json.raw("}"); json.finish(); return; - } mesh::RoomBoardResult result; if (strcmp(operation, "board.delete") == 0) { if (version == 0) { error("deletion requires the current article version"); return; } diff --git a/examples/simple_room_server/MyMesh.h b/examples/simple_room_server/MyMesh.h index c4e0e15b2..2f65c6aef 100644 --- a/examples/simple_room_server/MyMesh.h +++ b/examples/simple_room_server/MyMesh.h @@ -367,6 +367,11 @@ class MyMesh : public mesh::Mesh, public CommonCLICallbacks, mesh::RoomJsonWriter& json) __attribute__((noinline)); void processRoomWebBoard(const char* operation, JsonObjectConst input, mesh::RoomJsonWriter& json) __attribute__((noinline)); + void processRoomWebBoardRead(JsonObjectConst input, const mesh::RoomBoardIndex& board, + mesh::RoomJsonWriter& json) __attribute__((noinline)); + void processRoomWebBoardWrite(const char* operation, JsonObjectConst input, + const mesh::RoomBoardIndex& board, + mesh::RoomJsonWriter& json) __attribute__((noinline)); bool _unconfigured_setup_espnow_suspended = false; void suspendUnconfiguredSetupBridges(); bool startWebConfigImpl(bool force_ap, char* reply, bool automatic_setup); diff --git a/src/helpers/RoomBoardStore.h b/src/helpers/RoomBoardStore.h index b5fc02706..6662f07b6 100644 --- a/src/helpers/RoomBoardStore.h +++ b/src/helpers/RoomBoardStore.h @@ -42,6 +42,13 @@ static constexpr size_t STREAM_CHUNK = 64; static const uint8_t INDEX_MAGIC[4] = {'R', 'B', 'I', 1}; static const uint8_t ARTICLE_MAGIC[4] = {'R', 'B', 'A', 1}; +// Every field is an integer or character array. Clear the destination itself: +// assigning a value-initialized index can put a second whole index on the +// embedded caller's stack while filesystem recovery is still active. +inline void clearIndex(RoomBoardIndex& index) { + memset(static_cast(&index), 0, sizeof(index)); +} + inline uint32_t crc(uint32_t value, const uint8_t* data, size_t length) { while (length-- != 0) { value ^= *data++; @@ -206,7 +213,7 @@ bool readIndex(Filesystem* fs, const char* path, RoomBoardIndex& staged, for (size_t i = 9; i < sizeof(header); ++i) valid = valid && header[i] == 0; } uint32_t checksum = valid ? crc(UINT32_C(0xffffffff), header, sizeof(header)) : 0; - staged = RoomBoardIndex{}; + clearIndex(staged); if (valid) { staged.revision = get32(header + 4); staged.count = header[8]; } for (uint8_t i = 0; valid && i < staged.count; ++i) { uint8_t bytes[ENTRY_SIZE]; @@ -252,7 +259,7 @@ bool recover(Filesystem* fs, RoomBoardIndex& staged, bool require_cleanup) { if (present) { if (!readIndex(fs, ROOM_BOARD_BACKUP_PATH, staged) || !fs->rename(ROOM_BOARD_BACKUP_PATH, ROOM_BOARD_PRIMARY_PATH)) return false; - } else staged = RoomBoardIndex{}; + } else clearIndex(staged); const bool temp_removed = removeArtifact(fs, ROOM_BOARD_TEMP_PATH); return !require_cleanup || temp_removed; } @@ -295,10 +302,14 @@ RoomBoardResult commitIndex(Filesystem* fs, const RoomBoardIndex& next) { template bool loadRoomBoard(Filesystem* fs, RoomBoardIndex& index) { - RoomBoardIndex staged; - index = RoomBoardIndex{}; - if (!room_board_detail::recover(fs, staged, false)) return false; - index = staged; return true; + room_board_detail::clearIndex(index); + if (!room_board_detail::recover(fs, index, false)) { + // A corrupt body or a late I/O failure may follow valid metadata. Never + // return that partially populated index to a caller on failure. + room_board_detail::clearIndex(index); + return false; + } + return true; } // Main-loop single writer. The reader is called as reader(offset, buffer, diff --git a/test/fixtures/room_web_service/service.cpp b/test/fixtures/room_web_service/service.cpp index 0df24c71c..2ea930d90 100644 --- a/test/fixtures/room_web_service/service.cpp +++ b/test/fixtures/room_web_service/service.cpp @@ -1,6 +1,7 @@ #include "filesystem.h" #include #include +#include #include #include static bool deny_json_allocation = false; @@ -24,6 +25,11 @@ static void* trackedRealloc(void* ptr, size_t size) { #undef free #undef realloc #include +static bool deny_board_scratch_allocation = false; +void* operator new(size_t size, const std::nothrow_t&) noexcept { + if (deny_board_scratch_allocation) return nullptr; + try { return ::operator new(size); } catch (const std::bad_alloc&) { return nullptr; } +} #include #include #include @@ -172,6 +178,10 @@ struct MyMesh { mesh::RoomJsonWriter&) __attribute__((noinline)); void processRoomWebAdmin(const char*, JsonObjectConst, mesh::RoomJsonWriter&) __attribute__((noinline)); void processRoomWebBoard(const char*, JsonObjectConst, mesh::RoomJsonWriter&) __attribute__((noinline)); + void processRoomWebBoardRead(JsonObjectConst, const mesh::RoomBoardIndex&, + mesh::RoomJsonWriter&) __attribute__((noinline)); + void processRoomWebBoardWrite(const char*, JsonObjectConst, const mesh::RoomBoardIndex&, + mesh::RoomJsonWriter&) __attribute__((noinline)); void processRoomRequest(const uint8_t*, uint32_t, char*, char*, size_t); void serviceRoomQuotas(); bool storePost(const mesh::Identity&, const char*); @@ -1032,6 +1042,25 @@ int main() { assert(result(m, saveRequest(1, 0, "Reused", "new"), 36)["ok"] == true); error(m, request("board.read", "", ",\"id\":1,\"version\":2,\"offset\":0"), 37, "article changed"); ++scenarios; } + { + MyMesh m; enable(m); + assert(result(m, saveRequest(1, 0, "Retained", std::string(2048, 's')), 1)["ok"] == true); + std::map> before; + for (const auto& file : m.storage.files) before[file.first] = *file.second; + m.storage.trace.clear(); + deny_board_scratch_allocation = true; + error(m, request("board.index", "", ",\"revision\":0,\"cursor\":0"), 2, "insufficient memory"); + error(m, request("board.read", "", ",\"id\":1,\"version\":1,\"offset\":0"), 3, "insufficient memory"); + error(m, saveRequest(1, 1, "Changed", "new"), 4, "insufficient memory"); + error(m, request("board.delete", "admin-secret", ",\"boot\":71,\"id\":1,\"version\":1"), 5, "insufficient memory"); + deny_board_scratch_allocation = false; + assert(m.storage.trace.empty() && m.storage.files.size() == before.size()); + for (const auto& file : m.storage.files) assert(*file.second == before.at(file.first)); + const auto page = result(m, request("board.index", "", ",\"revision\":0,\"cursor\":0"), 6); + assert(page["revision"] == 1 && page["articles"][0]["length"] == 2048); + assert(result(m, request("board.read", "", ",\"id\":1,\"version\":1,\"offset\":0"), 7)["total"] == 2048); + ++scenarios; + } { MyMesh m; enable(m); uint32_t sequence = 1; for (const std::string& input : {std::string("{"), std::string("[]"), std::string("null"), std::string("{\"op\":{\"a\":{\"b\":{\"c\":{\"d\":1}}}}}"), diff --git a/test/test_room_board_store.py b/test/test_room_board_store.py index 68a4309b6..dded22670 100644 --- a/test/test_room_board_store.py +++ b/test/test_room_board_store.py @@ -95,7 +95,15 @@ static void checksum(std::vector& image) { static void preserveUnavailable(MemoryFS& fs) { const auto before = snapshot(fs); const auto directories = fs.directories; Index index; index.count = 7; index.revision = 99; + for (auto& article : index.articles) { + article.id = 7; article.bank = 1; article.body_length = 2000; article.version = 98; + memset(article.title, 'x', sizeof(article.title)); + } assert(!load(fs, index) && index.count == 0 && index.revision == 0); + for (const auto& article : index.articles) { + assert(article.id == 0 && article.bank == 0 && article.body_length == 0 && article.version == 0); + for (char value : article.title) assert(value == 0); + } assert(save(fs, 1, "Changed", NEW_BODY) == Result::Unavailable); assert(erase(fs, 1) == Result::Unavailable); uint8_t out[128]; size_t count = 99; diff --git a/test/test_room_web_service.py b/test/test_room_web_service.py index 0edeea986..48a9b6314 100644 --- a/test/test_room_web_service.py +++ b/test/test_room_web_service.py @@ -31,6 +31,7 @@ class RoomWebServiceTest(unittest.TestCase): "bool MyMesh::handleRoomWebCommand(", "static const char* roomWebString(", "bool MyMesh::processRoomWebMail(", "void MyMesh::processRoomWebAdmin(", "void MyMesh::processRoomWebBoard(", + "void MyMesh::processRoomWebBoardRead(", "void MyMesh::processRoomWebBoardWrite(", "void MyMesh::processRoomRequest(", "void MyMesh::serviceRoomQuotas(", "bool MyMesh::storePost(", "bool MyMesh::snapshotRoomHistory(", "bool MyMesh::executeClientPathCommand(", "bool MyMesh::setRoomClientPath(", @@ -111,7 +112,8 @@ class RoomWebServiceTest(unittest.TestCase): usage = "\n".join(path.read_text() for path in work.glob("*.su")) frames = {} for method, budget in (("processRoomRequest", 1024), ("processRoomWebAdmin", 1536), - ("processRoomWebBoard", 2048), ("processRoomWebMail", 3072)): + ("processRoomWebBoard", 384), ("processRoomWebBoardRead", 1152), + ("processRoomWebBoardWrite", 1536), ("processRoomWebMail", 3072)): sizes = re.findall(r"(?:void|bool) MyMesh::" + method + r"\([^\t\n]*\)\s*\t(\d+)\t[^\n]*", usage) self.assertEqual(len(sizes), 1, "missing or ambiguous stack frame: " + method)