fix: Skip large CSS files to prevent crashes by sam-lord · Pull Request #952 · crosspoint-reader/crosspoint-reader

sam-lord · 2026-02-17T15:45:30Z

Summary

What is the goal of this PR? (e.g., Implements the new feature for file uploading.)

Fixes: Crash when epub contains large CSS file #947

What changes are included?

Check to see if there's free heap memory before processing CSS (should we be doing this type of check or is it better to just crash if we exhaust the memory?)
Skip CSS files larger than 128kb

Additional Context

I found that a copy of Release it contained a 250kb+ CSS file, from the homepage of the publisher. It has nothing to do with the epub, so we should just skip it
Major question: Are there better ways to detect CSS that doesn't belong in a book, or is this size-based approach valid?
Another question: Are there any epubs we know of that legitimately include >128kb CSS files?

Code changes themselves created with an agent, all investigation and write-up done by human. If you (the maintainers) would prefer a different fix for this issue, let me know.

AI Usage

While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it
helps set the right context for reviewers.

Did you use AI tools to help write this code? < YES >

coderabbitai · 2026-02-17T15:45:55Z

📝 Walkthrough

Walkthrough

Introduces pre-parsing safeguards in parseCssFiles: checks available heap via ESP.getFreeHeap() and skips parsing if below 64 KB; checks CSS file size via getItemSize() and skips files larger than 128 KB. Errors are logged when parsing is skipped. No public API changes.

Changes

Cohort / File(s)	Summary
CSS Parsing Safeguards `lib/Epub/Epub.cpp`	Adds `MAX_CSS_FILE_SIZE` (128 KB) and `MIN_HEAP_FOR_CSS_PARSING` (64 KB). Before parsing each CSS file, checks free heap and file size with `ESP.getFreeHeap()` and `getItemSize()` respectively; skips parsing and logs errors when thresholds are not met.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Crash when epub contains large CSS file #947: Implements skipping of large CSS files and heap checks to prevent crashes when EPUB contains very large CSS assets.
Fix crash when EPUB CSS cache missing + avoid repeated cover thumb generation #761: Addresses the same EPUB CSS parsing path by adding heap and file-size guards to skip parsing under constrained conditions.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding safeguards to skip large CSS files that cause memory exhaustion and crashes.
Description check	✅ Passed	The description clearly relates to the changeset, explaining the rationale for adding CSS file size and heap memory checks to prevent crashes from large CSS files.
Linked Issues check	✅ Passed	The code changes directly address issue `#947` by implementing the requested fixes: skipping CSS files larger than 128 KB and checking available heap memory before processing CSS.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to preventing CSS parsing crashes; no unrelated modifications or refactoring introduced outside the issue requirements.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 97c3314 and 4bd156d.

📒 Files selected for processing (1)

lib/Epub/Epub.cpp

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: build

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/Epub/Epub.cpp`:
- Around line 285-293: The code currently continues when getItemSize(cssPath,
&cssFileSize) fails, which leaves cssFileSize unknown; change the logic in the
block around getItemSize so that a false return is treated as an error: log an
error via LOG_ERR (include cssPath) and continue (skip the file) instead of
proceeding; ensure the check still enforces cssFileSize > MAX_CSS_FILE_SIZE when
getItemSize returns true and only parses when getItemSize succeeded and size is
within MAX_CSS_FILE_SIZE.

coderabbitai · 2026-02-17T15:48:31Z

lib/Epub/Epub.cpp

+      // Check CSS file size before decompressing - skip files that are too large
+      size_t cssFileSize = 0;
+      if (getItemSize(cssPath, &cssFileSize)) {
+        if (cssFileSize > MAX_CSS_FILE_SIZE) {
+          LOG_ERR("EBP", "CSS file too large (%zu bytes > %zu max), skipping: %s",
+                  cssFileSize, MAX_CSS_FILE_SIZE, cssPath.c_str());
+          continue;
+        }
+      }


⚠️ Potential issue | 🟠 Major

Fail-closed when CSS size cannot be determined.
If getItemSize fails, the code still proceeds to parse the CSS, which can reintroduce the crash risk this change is meant to avoid. Treat an unknown size as unsafe and skip the file.

🔧 Proposed fix

- size_t cssFileSize = 0; - if (getItemSize(cssPath, &cssFileSize)) { - if (cssFileSize > MAX_CSS_FILE_SIZE) { - LOG_ERR("EBP", "CSS file too large (%zu bytes > %zu max), skipping: %s", - cssFileSize, MAX_CSS_FILE_SIZE, cssPath.c_str()); - continue; - } - } + size_t cssFileSize = 0; + if (!getItemSize(cssPath, &cssFileSize)) { + LOG_ERR("EBP", "Could not determine CSS file size, skipping: %s", cssPath.c_str()); + continue; + } + if (cssFileSize > MAX_CSS_FILE_SIZE) { + LOG_ERR("EBP", "CSS file too large (%zu bytes > %zu max), skipping: %s", + cssFileSize, MAX_CSS_FILE_SIZE, cssPath.c_str()); + continue; + }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

// Check CSS file size before decompressing - skip files that are too large

size_t cssFileSize = 0;

if (getItemSize(cssPath, &cssFileSize)) {

if (cssFileSize > MAX_CSS_FILE_SIZE) {

LOG_ERR("EBP", "CSS file too large (%zu bytes > %zu max), skipping: %s",

cssFileSize, MAX_CSS_FILE_SIZE, cssPath.c_str());

continue;

}

}

// Check CSS file size before decompressing - skip files that are too large

size_t cssFileSize = 0;

if (!getItemSize(cssPath, &cssFileSize)) {

LOG_ERR("EBP", "Could not determine CSS file size, skipping: %s", cssPath.c_str());

continue;

}

if (cssFileSize > MAX_CSS_FILE_SIZE) {

LOG_ERR("EBP", "CSS file too large (%zu bytes > %zu max), skipping: %s",

cssFileSize, MAX_CSS_FILE_SIZE, cssPath.c_str());

continue;

}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@lib/Epub/Epub.cpp` around lines 285 - 293, The code currently continues when getItemSize(cssPath, &cssFileSize) fails, which leaves cssFileSize unknown; change the logic in the block around getItemSize so that a false return is treated as an error: log an error via LOG_ERR (include cssPath) and continue (skip the file) instead of proceeding; ensure the check still enforces cssFileSize > MAX_CSS_FILE_SIZE when getItemSize returns true and only parses when getItemSize succeeded and size is within MAX_CSS_FILE_SIZE.

osteotek · 2026-02-17T15:49:52Z

CSS file bigger than 128kb sounds like malformed epub. We can probably just skip them without trying to parse. What do you think @jdk2pq ? I seem to remember you had a file limit in your recent PR but can't find it now

coderabbitai

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4bd156d and 4cd00b1.

📒 Files selected for processing (1)

lib/Epub/Epub.cpp

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: build
GitHub Check: cppcheck

🔇 Additional comments (2)

lib/Epub/Epub.cpp (2)

261-265: LGTM!

The threshold constants are well-chosen for ESP32 memory constraints and appropriately documented with inline comments explaining their purpose.

277-283: LGTM!

The heap check is correctly placed before each iteration to detect memory pressure from prior parsing. The logging provides useful diagnostics.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@lib/Epub/Epub.cpp`:
- Around line 285-293: The current logic in Epub::parse (around
getItemSize/cssFileSize/MAX_CSS_FILE_SIZE/cssPath) treats a false return from
getItemSize as if size is safe; change this to fail-closed by skipping the CSS
when getItemSize returns false: when getItemSize(cssPath, &cssFileSize) returns
false, log an error via LOG_ERR (include cssPath) and continue, rather than
falling through to parsing; ensure cssFileSize is only used when getItemSize
succeeded and that the MAX_CSS_FILE_SIZE check remains unchanged for the
successful case.

jdk2pq · 2026-02-18T02:18:42Z

Thank you for the fix @sam-lord!!

CSS file bigger than 128kb sounds like malformed epub. We can probably just skip them without trying to parse. What do you think @jdk2pq ? I seem to remember you had a file limit in your recent PR but can't find it now

Not necessarily malformed, but definitely poorly put together by the publisher (especially in @sam-lord's case where the publisher appears to be including their website's CSS inside of the epub 🙃). This fix makes sense to me!

As for the file limit that was in my PR previously, I removed that since the thought was we were already buffering the CSS file, and we'd hit the MAX_RULES limit anyway (and it looks like we do indeed 😄).

I tested the existing code out by making an EPUB with ~200KB of Tailwind's CSS, and I hit the same error. After the fix, it's working great. I'll add that test EPUB to my repo here.

Longer-term, we should think about:

only saving rules in rulesBySelector_ if they contain supported CSS properties. This should be a relatively simple change that could be follow-on after this PR.
dropping complex selectors and pseudo-selectors like :hover, :focus, etc. really aren't needed
using LibCSS for a dedicated parsing library that should have better memory usage

## Summary **What is the goal of this PR?** (e.g., Implements the new feature for file uploading.) * Fixes: crosspoint-reader#947 **What changes are included?** * Check to see if there's free heap memory before processing CSS (should we be doing this type of check or is it better to just crash if we exhaust the memory?) * Skip CSS files larger than 128kb ## Additional Context * I found that a copy of `Release it` contained a 250kb+ CSS file, from the homepage of the publisher. It has nothing to do with the epub, so we should just skip it * Major question: Are there better ways to detect CSS that doesn't belong in a book, or is this size-based approach valid? * Another question: Are there any epubs we know of that legitimately include >128kb CSS files? Code changes themselves created with an agent, all investigation and write-up done by human. If you (the maintainers) would prefer a different fix for this issue, let me know. --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**< YES >**_

coderabbitai bot reviewed Feb 17, 2026

View reviewed changes

Skip large CSS files to prevent crashes

4cd00b1

sam-lord force-pushed the fix/large-css-crash branch from 4bd156d to 4cd00b1 Compare February 17, 2026 15:58

coderabbitai bot reviewed Feb 17, 2026

View reviewed changes

jdk2pq approved these changes Feb 18, 2026

View reviewed changes

jpirnay mentioned this pull request Feb 18, 2026

feat: Avoid css crash, css rule selection logic #967

Closed

daveallie approved these changes Feb 19, 2026

View reviewed changes

daveallie merged commit e1074a8 into crosspoint-reader:master Feb 19, 2026
6 checks passed

daveallie mentioned this pull request Feb 19, 2026

chore(release): 1.1.0 Release Candidate #992

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Skip large CSS files to prevent crashes#952

fix: Skip large CSS files to prevent crashes#952
daveallie merged 1 commit intocrosspoint-reader:masterfrom
sam-lord:fix/large-css-crash

sam-lord commented Feb 17, 2026

Uh oh!

coderabbitai bot commented Feb 17, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Feb 17, 2026

Uh oh!

osteotek commented Feb 17, 2026 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

jdk2pq commented Feb 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

sam-lord commented Feb 17, 2026

Summary

Additional Context

AI Usage

Uh oh!

coderabbitai bot commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

osteotek commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

jdk2pq commented Feb 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coderabbitai bot commented Feb 17, 2026 •

edited

Loading

osteotek commented Feb 17, 2026 •

edited

Loading