Fail visibly on a full module list, and hash any size with SHA-1

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 <cstdio>   // std::snprintf in freeName
+#include <cstdio>   // std::snprintf in freeName, std::printf in addModule
 #include <cstring>  // 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<unsigned>(modules_.size()));
+        return false;
+    }
     modules_[moduleCount_++] = mod;
+    return true;
 }
 
 void Scheduler::setup() {

src/core/module/Scheduler.h

@@ -50,19 +50,19 @@ 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();
 
     /// 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.

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<uint32_t>(p[i * 4]) << 24) |
+               (static_cast<uint32_t>(p[i * 4 + 1]) << 16) |
+               (static_cast<uint32_t>(p[i * 4 + 2]) << 8) |
+                static_cast<uint32_t>(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<uint64_t>(len) * 8;
-    for (int i = 0; i < 8; i++) {
-        padded[padLen - 1 - i] = static_cast<uint8_t>(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<uint32_t>(padded[offset + i * 4]) << 24) |
-                   (static_cast<uint32_t>(padded[offset + i * 4 + 1]) << 16) |
-                   (static_cast<uint32_t>(padded[offset + i * 4 + 2]) << 8) |
-                    static_cast<uint32_t>(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<uint64_t>(len) * 8;
+    for (int i = 0; i < 8; i++) tail[tailLen - 1 - i] = static_cast<uint8_t>(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<uint8_t>(h[i] >> 24);
+        out[i * 4 + 1] = static_cast<uint8_t>(h[i] >> 16);
+        out[i * 4 + 2] = static_cast<uint8_t>(h[i] >> 8);
+        out[i * 4 + 3] = static_cast<uint8_t>(h[i]);
     }
-
-    auto store32 = [](uint8_t* p, uint32_t v) {
-        p[0] = static_cast<uint8_t>(v >> 24); p[1] = static_cast<uint8_t>(v >> 16);
-        p[2] = static_cast<uint8_t>(v >> 8);  p[3] = static_cast<uint8_t>(v);
-    };
-    store32(out, h0); store32(out + 4, h1); store32(out + 8, h2);
-    store32(out + 12, h3); store32(out + 16, h4);
 }
 
 } // namespace mm

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

test/unit/core/unit_Scheduler_capacity.cpp

@@ -0,0 +1,97 @@
+/// @module Scheduler
+
+#include "doctest.h"
+#include "core/module/Scheduler.h"
+#include "core/module/MoonModule.h"
+
+#include <cstdio>
+#include <string>
+#ifdef _WIN32
+#include <io.h>
+#else
+#include <unistd.h>
+#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, 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(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);
+
+    CHECK_FALSE(s.addModule(nullptr));
+    for (int i = 0; i < 33; i++) {
+        if (i < 32) s.deleteTree(mods[i]);
+        else delete mods[i];
+    }
+}

test/unit/core/unit_sha1.cpp

@@ -0,0 +1,36 @@
+/// @module sha1
+
+#include "doctest.h"
+#include "core/system/Sha1.h"
+
+#include <cstdio>
+#include <string>
+
+namespace {
+std::string hex(const std::string& in) {
+    uint8_t d[20];
+    mm::sha1(reinterpret_cast<const uint8_t*>(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");
+}