More review changes

Co-authored-by: Copilot <copilot@github.com>

lib/EpdFont/scripts/build-sd-fonts.py

@@ -126,6 +126,8 @@ def build_family(family: dict, output_base: Path) -> tuple[str, bool, str]:
     """Build a single font family. Returns (name, success, message)."""
     name = family["name"]
     output_dir = output_base / name
+    if output_dir.exists():
+        shutil.rmtree(output_dir)
     output_dir.mkdir(parents=True, exist_ok=True)
 
     styles = family.get("styles", {})
@@ -258,7 +260,7 @@ def main():
 
     # Filter if --only specified
     if args.only:
-        only_names = set(args.only.split(","))
+        only_names = {token.strip() for token in args.only.split(",") if token.strip()}
         families = [f for f in families if f["name"] in only_names]
         missing = only_names - {f["name"] for f in families}
         if missing:

lib/Epub/Epub/converters/JpegToFramebufferConverter.cpp

@@ -8,6 +8,7 @@
 #include <Logging.h>
 
 #include <cstdlib>
+#include <limits>
 #include <memory>
 #include <new>
 
@@ -234,6 +235,17 @@ bool readJpegDimensionsFromHeader(const std::string& imagePath, ImageDimensions&
         LOG_ERR("JPG", "Invalid JPEG dimensions %ux%u: %s", width, height, imagePath.c_str());
         return false;
       }
+
+      constexpr int MAX_SOURCE_PIXELS = 3145728;  // Keep in sync with ImageToFramebufferDecoder contract.
+      const int widthInt = static_cast<int>(width);
+      const int heightInt = static_cast<int>(height);
+      if (width > static_cast<uint16_t>(std::numeric_limits<int16_t>::max()) ||
+          height > static_cast<uint16_t>(std::numeric_limits<int16_t>::max()) ||
+          widthInt * heightInt > MAX_SOURCE_PIXELS) {
+        LOG_ERR("JPG", "JPEG dimensions out of supported range %ux%u: %s", width, height, imagePath.c_str());
+        return false;
+      }
+
       out.width = static_cast<int16_t>(width);
       out.height = static_cast<int16_t>(height);
       return true;

lib/GfxRenderer/FontCacheManager.cpp

@@ -109,8 +109,16 @@ void FontCacheManager::PrewarmScope::endScanAndPrewarm() {
 
   manager_->prewarmCache(manager_->scanFontId_, manager_->scanText_.c_str(), styleMask);
 
-  // Keep reserved capacity to avoid repeated alloc/free churn between pages.
+  constexpr size_t BASE_SCAN_TEXT_CAP = 2048;
+  constexpr size_t MAX_SCAN_TEXT_CAP = 16384;
+
+  // Keep reserved capacity for typical pages, but trim pathological outliers.
   manager_->scanText_.clear();
+  if (manager_->scanText_.capacity() > MAX_SCAN_TEXT_CAP) {
+    std::string trimmed;
+    trimmed.reserve(BASE_SCAN_TEXT_CAP);
+    manager_->scanText_.swap(trimmed);
+  }
 }
 
 FontCacheManager::PrewarmScope::~PrewarmScope() {

src/FontInstaller.h

@@ -19,7 +19,8 @@ class FontInstaller {
 
   explicit FontInstaller(SdCardFontRegistry& registry);
 
-  static constexpr size_t MAX_FAMILY_NAME_LEN = 64;
+  // Must fit CrossPointSettings::sdFontFamilyName[32] including NUL.
+  static constexpr size_t MAX_FAMILY_NAME_LEN = 31;
 
   /// Validate a family name: alphanumeric + hyphen + underscore only, no path traversal.
   static bool isValidFamilyName(const char* name);

src/RecentBooksStore.cpp

@@ -106,8 +106,9 @@ bool RecentBooksStore::setReaderOverrides(const std::string& path, const int8_t
   if (it == recentBooks.end()) {
     return false;
   }
-  return setReaderOverrides(path, embeddedStyleOverride, imageRenderingOverride, fontFamilyOverride,
+  const std::string sdOverride = (fontFamilyOverride >= 0) ? std::string() : it->sdFontFamilyOverride;
-                            it->sdFontFamilyOverride, fontSizeOverride, it->bionicReadingOverride);
+  return setReaderOverrides(path, embeddedStyleOverride, imageRenderingOverride, fontFamilyOverride, sdOverride,
+                            fontSizeOverride, it->bionicReadingOverride);
 }
 
 bool RecentBooksStore::setReaderOverrides(const std::string& path, const int8_t embeddedStyleOverride,
@@ -141,8 +142,9 @@ bool RecentBooksStore::setReaderOverrides(const std::string& path, const int8_t
   if (it == recentBooks.end()) {
     return false;
   }
-  return setReaderOverrides(path, embeddedStyleOverride, imageRenderingOverride, fontFamilyOverride,
+  const std::string sdOverride = (fontFamilyOverride >= 0) ? std::string() : it->sdFontFamilyOverride;
-                            it->sdFontFamilyOverride, fontSizeOverride, bionicReadingOverride);
+  return setReaderOverrides(path, embeddedStyleOverride, imageRenderingOverride, fontFamilyOverride, sdOverride,
+                            fontSizeOverride, bionicReadingOverride);
 }
 
 bool RecentBooksStore::setReaderOverrides(const std::string& path, const int8_t embeddedStyleOverride,

src/activities/reader/EpubReaderActivity.cpp

@@ -1043,16 +1043,26 @@ void EpubReaderActivity::applyBookReaderOverrides(const int8_t embeddedStyleOver
     return;
   }
 
+  // Built-in and SD font overrides are mutually exclusive; explicit built-in wins.
+  int8_t normalizedFontFamilyOverride = fontFamilyOverride;
+  std::string normalizedSdFontFamilyOverride = sdFontFamilyOverride;
+  if (normalizedFontFamilyOverride >= 0) {
+    normalizedSdFontFamilyOverride.clear();
+  } else if (!normalizedSdFontFamilyOverride.empty()) {
+    normalizedFontFamilyOverride = -1;
+  }
+
   if (bookEmbeddedStyleOverride == embeddedStyleOverride && bookImageRenderingOverride == imageRenderingOverride &&
-      bookFontFamilyOverride == fontFamilyOverride && bookSdFontFamilyOverride == sdFontFamilyOverride &&
+      bookFontFamilyOverride == normalizedFontFamilyOverride &&
-      bookFontSizeOverride == fontSizeOverride && bookBionicReadingOverride == bionicReadingOverride) {
+      bookSdFontFamilyOverride == normalizedSdFontFamilyOverride && bookFontSizeOverride == fontSizeOverride &&
+      bookBionicReadingOverride == bionicReadingOverride) {
     return;
   }
 
   bookEmbeddedStyleOverride = embeddedStyleOverride;
   bookImageRenderingOverride = imageRenderingOverride;
-  bookFontFamilyOverride = fontFamilyOverride;
+  bookFontFamilyOverride = normalizedFontFamilyOverride;
-  bookSdFontFamilyOverride = sdFontFamilyOverride;
+  bookSdFontFamilyOverride = normalizedSdFontFamilyOverride;
   bookFontSizeOverride = fontSizeOverride;
   bookBionicReadingOverride = bionicReadingOverride;
   RECENT_BOOKS.setReaderOverrides(epub->getPath(), bookEmbeddedStyleOverride, bookImageRenderingOverride,
@@ -1753,6 +1763,12 @@ bool EpubReaderActivity::drawCurrentPageToBuffer(const std::string& filePath, Gf
   if (effectiveFontId == 0 && currentBook.fontFamilyOverride >= 0) {
     effectiveFontId = CrossPointSettings::getBuiltinReaderFontId(effectiveFontFamily, effectiveFontSize);
   }
+  if (effectiveFontId == 0 && currentBook.fontSizeOverride >= 0 && SETTINGS.sdFontFamilyName[0] != '\0') {
+    effectiveFontId = resolveSdCardFontId(SETTINGS.sdFontFamilyName, effectiveFontSize);
+  }
+  if (effectiveFontId == 0 && currentBook.fontSizeOverride >= 0) {
+    effectiveFontId = CrossPointSettings::getBuiltinReaderFontId(SETTINGS.fontFamily, effectiveFontSize);
+  }
   if (effectiveFontId == 0) {
     effectiveFontId = SETTINGS.getReaderFontId();
   }

src/activities/reader/EpubReaderMenuActivity.cpp

@@ -17,7 +17,12 @@ namespace {
 // though the per-book override list itself is built-in only.
 std::string defaultFontFamilyLabel(const SettingInfo& item) {
   if (SETTINGS.sdFontFamilyName[0] != '\0') {
-    return std::string(SETTINGS.sdFontFamilyName);
+    const auto& families = sdFontSystem.registry().getFamilies();
+    const auto it = std::find_if(families.begin(), families.end(),
+                                 [](const auto& family) { return family.name == SETTINGS.sdFontFamilyName; });
+    if (it != families.end()) {
+      return std::string(SETTINGS.sdFontFamilyName);
+    }
   }
   // Built-in: enumValues[0] is STR_DEFAULT_VALUE, [1..] are built-in families
   // in CrossPointSettings::FONT_FAMILY order.

src/network/CrossPointWebServer.cpp

@@ -1464,6 +1464,7 @@ void CrossPointWebServer::handleFontUploadData() {
       String family = server->arg("family");
       fontUpload.valid = false;
       fontUpload.magicChecked = false;
+      fontUpload.headerBytesReceived = 0;
       fontUpload.bytesWritten = 0;
       fontUpload.bufferPos = 0;
 
@@ -1477,6 +1478,10 @@ void CrossPointWebServer::handleFontUploadData() {
         LOG_ERR("WEB", "Not a .cpfont file: %s", filename.c_str());
         break;
       }
+      if (filename.indexOf('/') >= 0 || filename.indexOf('\\') >= 0 || filename.indexOf("..") >= 0) {
+        LOG_ERR("WEB", "Invalid font filename: %s", filename.c_str());
+        break;
+      }
 
       fontUpload.familyName = family.c_str();
 
@@ -1504,13 +1509,22 @@ void CrossPointWebServer::handleFontUploadData() {
       if (!fontUpload.valid) break;
       esp_task_wdt_reset();
 
-      if (!fontUpload.magicChecked && up.currentSize >= 8) {
+      if (!fontUpload.magicChecked) {
-        if (memcmp(up.buf, "CPFONT\0\0", 8) != 0) {
+        size_t needed = 8 - fontUpload.headerBytesReceived;
-          LOG_ERR("WEB", "Invalid .cpfont magic bytes");
+        size_t take = (up.currentSize < needed) ? up.currentSize : needed;
-          fontUpload.valid = false;
+        if (take > 0) {
-          break;
+          memcpy(fontUpload.header + fontUpload.headerBytesReceived, up.buf, take);
+          fontUpload.headerBytesReceived += take;
+        }
+        if (fontUpload.headerBytesReceived == 8) {
+          if (memcmp(fontUpload.header, "CPFONT\0\0", 8) != 0) {
+            LOG_ERR("WEB", "Invalid .cpfont magic bytes");
+            fontUpload.valid = false;
+            fontUpload.file.close();
+            return;
+          }
+          fontUpload.magicChecked = true;
         }
-        fontUpload.magicChecked = true;
       }
 
       size_t remaining = up.currentSize;
@@ -1524,8 +1538,16 @@ void CrossPointWebServer::handleFontUploadData() {
         remaining -= chunk;
 
         if (fontUpload.bufferPos >= FontUploadState::BUFFER_SIZE) {
-          fontUpload.file.write(fontUpload.buffer.data(), fontUpload.bufferPos);
+          const size_t expected = fontUpload.bufferPos;
-          fontUpload.bytesWritten += fontUpload.bufferPos;
+          const size_t written = fontUpload.file.write(fontUpload.buffer.data(), expected);
+          fontUpload.bytesWritten += written;
+          if (written != expected) {
+            LOG_ERR("WEB", "Failed writing uploaded font chunk (%u/%u bytes)", static_cast<unsigned>(written),
+                    static_cast<unsigned>(expected));
+            fontUpload.valid = false;
+            fontUpload.file.close();
+            return;
+          }
           fontUpload.bufferPos = 0;
           esp_task_wdt_reset();
         }
@@ -1534,9 +1556,19 @@ void CrossPointWebServer::handleFontUploadData() {
     }
 
     case UPLOAD_FILE_END: {
+      if (fontUpload.valid && !fontUpload.magicChecked) {
+        LOG_ERR("WEB", "Invalid .cpfont upload: header not fully received");
+        fontUpload.valid = false;
+      }
       if (fontUpload.valid && fontUpload.bufferPos > 0) {
-        fontUpload.file.write(fontUpload.buffer.data(), fontUpload.bufferPos);
+        const size_t expected = fontUpload.bufferPos;
-        fontUpload.bytesWritten += fontUpload.bufferPos;
+        const size_t written = fontUpload.file.write(fontUpload.buffer.data(), expected);
+        fontUpload.bytesWritten += written;
+        if (written != expected) {
+          LOG_ERR("WEB", "Failed flushing uploaded font chunk (%u/%u bytes)", static_cast<unsigned>(written),
+                  static_cast<unsigned>(expected));
+          fontUpload.valid = false;
+        }
         fontUpload.bufferPos = 0;
       }
       fontUpload.file.close();

src/network/CrossPointWebServer.h

@@ -123,6 +123,8 @@ class CrossPointWebServer {
     std::string filePath;
     bool valid = false;
     bool magicChecked = false;
+    uint8_t header[8] = {0};
+    size_t headerBytesReceived = 0;
     size_t bytesWritten = 0;
     static constexpr size_t BUFFER_SIZE = 4096;
     std::vector<uint8_t> buffer;

src/network/html/FontsPage.html

@@ -278,7 +278,12 @@
         info.textContent = 'Pick one or more .cpfont files.';
         return;
       }
-      const family = sanitizeFamily(familyFromFilename(files[0].name));
+      const families = new Set(files.map(f => sanitizeFamily(familyFromFilename(f.name))));
+      if (families.size > 1) {
+        info.textContent = 'Picked files contain multiple families — please select files from a single family.';
+        return;
+      }
+      const family = families.values().next().value;
       info.textContent = files.length + ' file' + (files.length === 1 ? '' : 's') +
         ' → family "' + family + '"';
     });