From cc9ff5bae9b0a3da84017cb7ec41e93b65f0b218 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Sat, 2 May 2026 18:42:45 +0200 Subject: [PATCH] More review changes Co-authored-by: Copilot --- lib/EpdFont/scripts/build-sd-fonts.py | 4 +- .../converters/JpegToFramebufferConverter.cpp | 12 +++++ lib/GfxRenderer/FontCacheManager.cpp | 10 +++- src/FontInstaller.h | 3 +- src/RecentBooksStore.cpp | 10 ++-- src/activities/reader/EpubReaderActivity.cpp | 24 +++++++-- .../reader/EpubReaderMenuActivity.cpp | 7 ++- src/network/CrossPointWebServer.cpp | 52 +++++++++++++++---- src/network/CrossPointWebServer.h | 2 + src/network/html/FontsPage.html | 7 ++- 10 files changed, 108 insertions(+), 23 deletions(-) diff --git a/lib/EpdFont/scripts/build-sd-fonts.py b/lib/EpdFont/scripts/build-sd-fonts.py index a6d126da..275fae28 100644 --- a/lib/EpdFont/scripts/build-sd-fonts.py +++ b/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: diff --git a/lib/Epub/Epub/converters/JpegToFramebufferConverter.cpp b/lib/Epub/Epub/converters/JpegToFramebufferConverter.cpp index da8b64fd..b3b72927 100644 --- a/lib/Epub/Epub/converters/JpegToFramebufferConverter.cpp +++ b/lib/Epub/Epub/converters/JpegToFramebufferConverter.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -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(width); + const int heightInt = static_cast(height); + if (width > static_cast(std::numeric_limits::max()) || + height > static_cast(std::numeric_limits::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(width); out.height = static_cast(height); return true; diff --git a/lib/GfxRenderer/FontCacheManager.cpp b/lib/GfxRenderer/FontCacheManager.cpp index 01841ad6..b55fdf08 100644 --- a/lib/GfxRenderer/FontCacheManager.cpp +++ b/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() { diff --git a/src/FontInstaller.h b/src/FontInstaller.h index 0115d009..c331b5d4 100644 --- a/src/FontInstaller.h +++ b/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); diff --git a/src/RecentBooksStore.cpp b/src/RecentBooksStore.cpp index d53a281b..a4c069d5 100644 --- a/src/RecentBooksStore.cpp +++ b/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, - it->sdFontFamilyOverride, fontSizeOverride, it->bionicReadingOverride); + const std::string sdOverride = (fontFamilyOverride >= 0) ? std::string() : it->sdFontFamilyOverride; + 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, - it->sdFontFamilyOverride, fontSizeOverride, bionicReadingOverride); + const std::string sdOverride = (fontFamilyOverride >= 0) ? std::string() : it->sdFontFamilyOverride; + return setReaderOverrides(path, embeddedStyleOverride, imageRenderingOverride, fontFamilyOverride, sdOverride, + fontSizeOverride, bionicReadingOverride); } bool RecentBooksStore::setReaderOverrides(const std::string& path, const int8_t embeddedStyleOverride, diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 69aa3983..b4713a64 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/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 && - bookFontSizeOverride == fontSizeOverride && bookBionicReadingOverride == bionicReadingOverride) { + bookFontFamilyOverride == normalizedFontFamilyOverride && + bookSdFontFamilyOverride == normalizedSdFontFamilyOverride && bookFontSizeOverride == fontSizeOverride && + bookBionicReadingOverride == bionicReadingOverride) { return; } bookEmbeddedStyleOverride = embeddedStyleOverride; bookImageRenderingOverride = imageRenderingOverride; - bookFontFamilyOverride = fontFamilyOverride; - bookSdFontFamilyOverride = sdFontFamilyOverride; + bookFontFamilyOverride = normalizedFontFamilyOverride; + 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(); } diff --git a/src/activities/reader/EpubReaderMenuActivity.cpp b/src/activities/reader/EpubReaderMenuActivity.cpp index 6c4c35a1..beedd5ee 100644 --- a/src/activities/reader/EpubReaderMenuActivity.cpp +++ b/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. diff --git a/src/network/CrossPointWebServer.cpp b/src/network/CrossPointWebServer.cpp index f110cf2e..dcc169e1 100644 --- a/src/network/CrossPointWebServer.cpp +++ b/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 (memcmp(up.buf, "CPFONT\0\0", 8) != 0) { - LOG_ERR("WEB", "Invalid .cpfont magic bytes"); - fontUpload.valid = false; - break; + if (!fontUpload.magicChecked) { + size_t needed = 8 - fontUpload.headerBytesReceived; + size_t take = (up.currentSize < needed) ? up.currentSize : needed; + if (take > 0) { + 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); - fontUpload.bytesWritten += fontUpload.bufferPos; + const size_t expected = 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(written), + static_cast(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); - fontUpload.bytesWritten += fontUpload.bufferPos; + const size_t expected = 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(written), + static_cast(expected)); + fontUpload.valid = false; + } fontUpload.bufferPos = 0; } fontUpload.file.close(); diff --git a/src/network/CrossPointWebServer.h b/src/network/CrossPointWebServer.h index 1a32c1ba..78421280 100644 --- a/src/network/CrossPointWebServer.h +++ b/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 buffer; diff --git a/src/network/html/FontsPage.html b/src/network/html/FontsPage.html index 51eb5f50..a3023fcb 100644 --- a/src/network/html/FontsPage.html +++ b/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 + '"'; });