From 9302c9c47ce75b584bb8a4ce00dcf6ecfc93569a Mon Sep 17 00:00:00 2001 From: James Hutchinson Date: Thu, 29 Oct 2026 04:21:53 -0700 Subject: [PATCH 1/2] Fail visibly on a full module list, and hash any size with SHA-1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A boot that registers a 33rd top-level module now logs it and gets false back instead of losing the module silently, and SHA-1 hashes input of any length instead of silently skipping the digest past 503 bytes. KPI: 256lights | Desktop:2221KB | tick:7/10/7/1/7/7/163/7/7/8us(FPS:142857/100000/142857/1000000/142857/142857/6134/142857/142857/125000) | src:345(88766) | test:261(61559) | functions-over:345 Commit: src +64/-54 lines | test +69/-0 lines | duplicated lines 3268 -> 3268 (+0) **Core** - Scheduler::addModule returns bool: false, with a "no room" log line, when the 32 top-level slots are full: the bool `addChild` returns and the log line `Control` writes. - sha1 streams whole 64-byte blocks from the input through `detail::sha1Block`, the FIPS 180-4 compression step, and pads only the tail: no size limit, and stack use falls from 832 to 448 bytes. - Scheduler.h and Sha1.h leave the docgen warning list: `@xref`s from `tick`, `prepareTree` and `setControl` to the sections they explain, and Sha1's appendix one line per paragraph, said once. **Tests** - unit_Scheduler_capacity: the 33rd add is refused and the first 32 stay in order. - unit_sha1: the FIPS 180-4 vectors, the 55, 56 and 64-byte padding boundaries, and one million 'a'. - build_desktop: PASS, no warnings. check_code: PASS, counts equal to the committed ones. test_desktop: PASS, 2,848 test cases, 179,557 assertions. - check_esp32_built --firmware esp32s3-n16r8: PASS, rebuilt with these sources. - check_specs: PASS. check_docgen: PASS, 0 errors, warnings 2050 → 2045. - check_prose: NegatedHeading 6→8 and SelfReference 17→18 against the committed counts with no change of ours, the same rise in the other branch, which changes no prose (Vale 3.25.1); left for the maintainer. **Reviews** - Fable review: the KPI line and triggered checks listed, done; "512 bytes" corrected to 503, done; `` included, done; all 32 slots checked, done; the WebSocket-only sentence said once, done. - Fable review, round two: `addChild` returns a bool and logs nothing, so the message names each precedent, done; a negative clause in Sha1.h cut, done; a public constant for the 32-slot limit, declined: it grows the API for one test, and the test documents the limit. - check_code: the block lambda counted into `sha1` and raised complex function 293→294, so the FIPS 180-4 compression step is its own function, `detail::sha1Block`, done. Co-Authored-By: Claude Opus 5.5 --- src/core/module/Scheduler.cpp | 11 ++- src/core/module/Scheduler.h | 10 +-- src/core/system/Sha1.h | 97 ++++++++++++---------- test/CMakeLists.txt | 2 + test/unit/core/unit_Scheduler_capacity.cpp | 31 +++++++ test/unit/core/unit_sha1.cpp | 36 ++++++++ 6 files changed, 133 insertions(+), 54 deletions(-) create mode 100644 test/unit/core/unit_Scheduler_capacity.cpp create mode 100644 test/unit/core/unit_sha1.cpp diff --git a/src/core/module/Scheduler.cpp b/src/core/module/Scheduler.cpp index 2479b55d..cca29760 100644 --- a/src/core/module/Scheduler.cpp +++ b/src/core/module/Scheduler.cpp @@ -6,14 +6,19 @@ #include "core/util/JsonUtil.h" // mm::json::parseBool for the "enabled" pseudo-control #include "platform/platform.h" -#include // std::snprintf in freeName +#include // std::snprintf in freeName, std::printf in addModule #include // std::strcmp in firstInTree namespace mm { -void Scheduler::addModule(MoonModule* mod) { - if (!mod || moduleCount_ >= modules_.size()) return; +bool Scheduler::addModule(MoonModule* mod) { + if (!mod) return false; + if (moduleCount_ >= modules_.size()) { + std::printf("scheduler: no room for module %s, %u top-level modules is the limit\n", mod->name(), static_cast(modules_.size())); + return false; + } modules_[moduleCount_++] = mod; + return true; } void Scheduler::setup() { diff --git a/src/core/module/Scheduler.h b/src/core/module/Scheduler.h index 5e3e1ed0..57d5513b 100644 --- a/src/core/module/Scheduler.h +++ b/src/core/module/Scheduler.h @@ -50,11 +50,11 @@ class Scheduler { /// Tell every module in the tree that `owner`'s list rows changed, so one resolving from that list re-resolves. void notifyListChanged(const MoonModule& owner); - /// Register a top-level module, which the boot then walks in declared order. - void addModule(MoonModule* mod); + /// Register a top-level module, which the boot then walks in declared order; false, with a log line, when the array is full. + bool addModule(MoonModule* mod); /// Run the boot phases over every registered module. void setup(); - /// One pass: tick every top-level module, then pace and publish the timing. + /// One pass: tick every top-level module, then pace and publish the timing: @xref{three-cadences-cover-every-module}. void tick() MM_NONBLOCKING; /// Release every module, in reverse of the order they were added. void release(); @@ -62,7 +62,7 @@ class Scheduler { /// Milliseconds since setup, which is the clock every effect animates against. uint32_t elapsed() const; - /// Rebuild derived state across the whole tree, immediately, on the calling thread. + /// Rebuild derived state across the whole tree, immediately, on the calling thread: @xref{rebuilding-derived-state}. void prepareTree(); /// Ask for a rebuild at the next frame boundary, which is safe from any task. @@ -127,7 +127,7 @@ class Scheduler { ReadOnly, ///< tried to write a display-only control }; - /// Set one control by module and control name, applying the whole control-change reaction. + /// Set one control by module and control name, applying the whole control-change reaction: @xref{driving-one-control-from-anywhere}. SetControlResult setControl(const char* moduleName, const char* controlName, const char* valueJson, bool live = false); /// The reaction to a control that changed: visibility rebuilt, the module told, and the tree re-prepared when the control reshapes it. diff --git a/src/core/system/Sha1.h b/src/core/system/Sha1.h index f0ea9f80..f7b5f639 100644 --- a/src/core/system/Sha1.h +++ b/src/core/system/Sha1.h @@ -9,60 +9,65 @@ namespace mm { /// /// @technical /// -/// This is not a general-purpose crypto primitive. -/// SHA-1 is broken for security, and it appears here solely because the WebSocket standard requires it for the accept-key derivation. +/// SHA-1 is broken for security. /// -/// It sits out of class so it is not bound to one translation unit, and in the namespace so a caller needs no qualifier. -/// It is inline so any future unit that wants SHA-1 includes this header without a separate implementation file, the function being small enough that duplicating it costs little. +/// An inline free function in the namespace, so any unit includes it with no implementation file and calls it unqualified, at the cost of a small copy per unit. + +namespace detail { + +/// One 64-byte block of the FIPS 180-4 compression function, folded into the five hash words `h`. +inline void sha1Block(uint32_t h[5], const uint8_t* p) { + uint32_t w[80]; + for (int i = 0; i < 16; i++) { + w[i] = (static_cast(p[i * 4]) << 24) | + (static_cast(p[i * 4 + 1]) << 16) | + (static_cast(p[i * 4 + 2]) << 8) | + static_cast(p[i * 4 + 3]); + } + for (int i = 16; i < 80; i++) { + uint32_t v = w[i-3] ^ w[i-8] ^ w[i-14] ^ w[i-16]; + w[i] = (v << 1) | (v >> 31); + } + + uint32_t a = h[0], b = h[1], c = h[2], d = h[3], e = h[4]; + for (int i = 0; i < 80; i++) { + uint32_t f, k; + if (i < 20) { f = (b & c) | (~b & d); k = 0x5A827999; } + else if (i < 40) { f = b ^ c ^ d; k = 0x6ED9EBA1; } + else if (i < 60) { f = (b & c) | (b & d) | (c & d); k = 0x8F1BBCDC; } + else { f = b ^ c ^ d; k = 0xCA62C1D6; } + uint32_t temp = ((a << 5) | (a >> 27)) + f + e + k + w[i]; + e = d; d = c; c = (b << 30) | (b >> 2); b = a; a = temp; + } + h[0] += a; h[1] += b; h[2] += c; h[3] += d; h[4] += e; +} + +} // namespace detail /// Hash `len` bytes at `data` into `out`, which must hold 20 bytes. inline void sha1(const uint8_t* data, size_t len, uint8_t out[20]) { - uint32_t h0 = 0x67452301, h1 = 0xEFCDAB89, h2 = 0x98BADCFE, - h3 = 0x10325476, h4 = 0xC3D2E1F0; + uint32_t h[5] = {0x67452301, 0xEFCDAB89, 0x98BADCFE, 0x10325476, 0xC3D2E1F0}; - // Pad message - size_t padLen = ((len + 8) / 64 + 1) * 64; - uint8_t padded[512] = {}; - if (padLen > sizeof(padded)) return; // input too large for our buffer - std::memcpy(padded, data, len); - padded[len] = 0x80; - uint64_t bitLen = static_cast(len) * 8; - for (int i = 0; i < 8; i++) { - padded[padLen - 1 - i] = static_cast(bitLen >> (i * 8)); - } + // Whole blocks stream straight from the input; only the tail is padded, into one or two blocks. + const size_t full = len / 64; + for (size_t i = 0; i < full; i++) detail::sha1Block(h, data + i * 64); - for (size_t offset = 0; offset < padLen; offset += 64) { - uint32_t w[80]; - for (int i = 0; i < 16; i++) { - w[i] = (static_cast(padded[offset + i * 4]) << 24) | - (static_cast(padded[offset + i * 4 + 1]) << 16) | - (static_cast(padded[offset + i * 4 + 2]) << 8) | - static_cast(padded[offset + i * 4 + 3]); - } - for (int i = 16; i < 80; i++) { - uint32_t v = w[i-3] ^ w[i-8] ^ w[i-14] ^ w[i-16]; - w[i] = (v << 1) | (v >> 31); - } + const size_t rest = len - full * 64; + uint8_t tail[128] = {}; + if (rest) std::memcpy(tail, data + full * 64, rest); + tail[rest] = 0x80; + const size_t tailLen = rest + 9 <= 64 ? 64 : 128; // the 0x80 and the 8-byte length must fit + const uint64_t bitLen = static_cast(len) * 8; + for (int i = 0; i < 8; i++) tail[tailLen - 1 - i] = static_cast(bitLen >> (i * 8)); + for (size_t off = 0; off < tailLen; off += 64) detail::sha1Block(h, tail + off); - uint32_t a = h0, b = h1, c = h2, d = h3, e = h4; - for (int i = 0; i < 80; i++) { - uint32_t f, k; - if (i < 20) { f = (b & c) | (~b & d); k = 0x5A827999; } - else if (i < 40) { f = b ^ c ^ d; k = 0x6ED9EBA1; } - else if (i < 60) { f = (b & c) | (b & d) | (c & d); k = 0x8F1BBCDC; } - else { f = b ^ c ^ d; k = 0xCA62C1D6; } - uint32_t temp = ((a << 5) | (a >> 27)) + f + e + k + w[i]; - e = d; d = c; c = (b << 30) | (b >> 2); b = a; a = temp; - } - h0 += a; h1 += b; h2 += c; h3 += d; h4 += e; + // The digest is the five words, big-endian. + for (int i = 0; i < 5; i++) { + out[i * 4] = static_cast(h[i] >> 24); + out[i * 4 + 1] = static_cast(h[i] >> 16); + out[i * 4 + 2] = static_cast(h[i] >> 8); + out[i * 4 + 3] = static_cast(h[i]); } - - auto store32 = [](uint8_t* p, uint32_t v) { - p[0] = static_cast(v >> 24); p[1] = static_cast(v >> 16); - p[2] = static_cast(v >> 8); p[3] = static_cast(v); - }; - store32(out, h0); store32(out + 4, h1); store32(out + 8, h2); - store32(out + 12, h3); store32(out + 16, h4); } } // namespace mm diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e3c70047..b4e795f1 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -110,10 +110,12 @@ add_executable(mm_tests unit/core/unit_OscService.cpp unit/core/unit_Scheduler_prepareScope.cpp unit/core/unit_Scheduler_unique_names.cpp + unit/core/unit_Scheduler_capacity.cpp unit/core/unit_MoonStatsModule.cpp unit/core/unit_MoonTalkModule.cpp unit/core/unit_InstallationId.cpp unit/core/unit_MoonCloudModule.cpp + unit/core/unit_sha1.cpp unit/core/unit_sha256.cpp unit/core/unit_MoonStatsReport.cpp unit/core/unit_SystemModule.cpp diff --git a/test/unit/core/unit_Scheduler_capacity.cpp b/test/unit/core/unit_Scheduler_capacity.cpp new file mode 100644 index 00000000..4afc6a1f --- /dev/null +++ b/test/unit/core/unit_Scheduler_capacity.cpp @@ -0,0 +1,31 @@ +/// @module Scheduler + +#include "doctest.h" +#include "core/module/Scheduler.h" +#include "core/module/MoonModule.h" + +// Pins Scheduler::addModule at its limit. A boot that wires past the top-level array must learn of it, so the add answers false and logs a line rather than dropping the module silently. + +namespace { +struct Stub : public mm::MoonModule {}; +} // namespace + +// The 33rd add is refused and answers false, while the first 32 are accepted and stay in order. +TEST_CASE("Scheduler::addModule refuses a module past the limit and says so") { + mm::Scheduler s; + Stub* mods[33]; + for (int i = 0; i < 33; i++) { + mods[i] = new Stub(); + mods[i]->setName("M"); + const bool added = s.addModule(mods[i]); + CHECK(added == (i < 32)); + } + for (int i = 0; i < 32; i++) CHECK(s.module(i) == mods[i]); + CHECK(s.module(32) == nullptr); + + CHECK_FALSE(s.addModule(nullptr)); + for (int i = 0; i < 33; i++) { + if (i < 32) s.deleteTree(mods[i]); + else delete mods[i]; + } +} diff --git a/test/unit/core/unit_sha1.cpp b/test/unit/core/unit_sha1.cpp new file mode 100644 index 00000000..06b6cb1a --- /dev/null +++ b/test/unit/core/unit_sha1.cpp @@ -0,0 +1,36 @@ +/// @module sha1 + +#include "doctest.h" +#include "core/system/Sha1.h" + +#include +#include + +namespace { +std::string hex(const std::string& in) { + uint8_t d[20]; + mm::sha1(reinterpret_cast(in.data()), in.size(), d); + char buf[41]; + for (int i = 0; i < 20; i++) std::snprintf(buf + i * 2, 3, "%02x", d[i]); + return std::string(buf); +} +} // namespace + +// The digest matches the published FIPS 180-4 vectors: the empty string, "abc", and the two-block example. +TEST_CASE("sha1 matches the published test vectors") { + CHECK(hex("") == "da39a3ee5e6b4b0d3255bfef95601890afd80709"); + CHECK(hex("abc") == "a9993e364706816aba3e25717850c26c9cd0d89d"); + CHECK(hex("abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq") == "84983e441c3bd26ebaae4aa1f95129e5e54670f1"); +} + +// A message of 55, 56 and 64 bytes hashes to the known digest: 55 leaves room for the length, 56 forces a second padding block, 64 is a whole block with all the padding after it. +TEST_CASE("sha1 pads correctly around the block boundary") { + CHECK(hex(std::string(55, 'a')) == "c1c8bbdc22796e28c0e15163d20899b65621d65a"); + CHECK(hex(std::string(56, 'a')) == "c2db330f6083854c99d4b5bfb6e8f29f201be699"); + CHECK(hex(std::string(64, 'a')) == "0098ba824b5c16427bd7a1122a5a442a25ec644d"); +} + +// One million 'a' hashes to the published digest, far past the 503 bytes the old fixed buffer could pad. +TEST_CASE("sha1 hashes an input of any size") { + CHECK(hex(std::string(1000000, 'a')) == "34aa973cd4c4daa4f61eeb2bdbad27316534016f"); +} From 4f8d372a013cc8290c8838bab76ee5dee60e6487 Mon Sep 17 00:00:00 2001 From: James Hutchinson Date: Thu, 29 Oct 2026 06:32:06 -0700 Subject: [PATCH 2/2] Pin the line a full module list prints The boot ignores addModule's answer, so the printed line is what makes a full list visible, and the test now reads it. **Tests** - unit_Scheduler_capacity: stdout is captured around the refused add alone, and the test checks the line names the module that had no room and the limit; removing the printf now fails it. The capture uses dup and dup2, _dup and _dup2 under MSVC. **Reviews** - Copilot: the capacity test never asserted the diagnostic, so removing the printf left it passing; done, the test reads the printed line. --- test/unit/core/unit_Scheduler_capacity.cpp | 74 ++++++++++++++++++++-- 1 file changed, 70 insertions(+), 4 deletions(-) diff --git a/test/unit/core/unit_Scheduler_capacity.cpp b/test/unit/core/unit_Scheduler_capacity.cpp index 4afc6a1f..16105ec4 100644 --- a/test/unit/core/unit_Scheduler_capacity.cpp +++ b/test/unit/core/unit_Scheduler_capacity.cpp @@ -4,22 +4,88 @@ #include "core/module/Scheduler.h" #include "core/module/MoonModule.h" +#include +#include +#ifdef _WIN32 +#include +#else +#include +#endif + // Pins Scheduler::addModule at its limit. A boot that wires past the top-level array must learn of it, so the add answers false and logs a line rather than dropping the module silently. namespace { struct Stub : public mm::MoonModule {}; + +#ifdef _WIN32 +int dupFd(int fd) { return _dup(fd); } +int dup2Fd(int from, int to) { return _dup2(from, to); } +int closeFd(int fd) { return _close(fd); } +int fdOf(std::FILE* f) { return _fileno(f); } +#else +int dupFd(int fd) { return dup(fd); } +int dup2Fd(int from, int to) { return dup2(from, to); } +int closeFd(int fd) { return close(fd); } +int fdOf(std::FILE* f) { return fileno(f); } +#endif + +// What a span of code prints to stdout: stdout points at a temporary file from construction until text(), then points back. +struct StdoutCapture { + std::FILE* file = std::tmpfile(); + int saved = -1; + + StdoutCapture() { + if (!file) return; + std::fflush(stdout); + saved = dupFd(fdOf(stdout)); + dup2Fd(fdOf(file), fdOf(stdout)); + } + std::string text() { + restore(); + std::string out; + if (!file) return out; + std::rewind(file); + char buf[256]; + for (size_t n; (n = std::fread(buf, 1, sizeof buf, file)) > 0;) out.append(buf, n); + return out; + } + ~StdoutCapture() { + restore(); + if (file) std::fclose(file); + } + +private: + void restore() { + if (saved < 0) return; + std::fflush(stdout); + dup2Fd(saved, fdOf(stdout)); + closeFd(saved); + saved = -1; + } +}; } // namespace -// The 33rd add is refused and answers false, while the first 32 are accepted and stay in order. +// The 33rd add is refused, answers false and prints which module had no room and the limit, while the first 32 are accepted and stay in order. TEST_CASE("Scheduler::addModule refuses a module past the limit and says so") { mm::Scheduler s; Stub* mods[33]; for (int i = 0; i < 33; i++) { mods[i] = new Stub(); - mods[i]->setName("M"); - const bool added = s.addModule(mods[i]); - CHECK(added == (i < 32)); + mods[i]->setName(i < 32 ? "M" : "Extra"); } + for (int i = 0; i < 32; i++) CHECK(s.addModule(mods[i])); + + bool added = true; + std::string said; + { + StdoutCapture out; // around the refused add alone, so no check's own output lands in it + added = s.addModule(mods[32]); + said = out.text(); + } + CHECK_FALSE(added); + CHECK(said.find("no room for module Extra") != std::string::npos); + CHECK(said.find("32 top-level modules is the limit") != std::string::npos); + for (int i = 0; i < 32; i++) CHECK(s.module(i) == mods[i]); CHECK(s.module(32) == nullptr);