fix: reject fonts with reserved bit 7 in glyf simple glyph flags#13144
fix: reject fonts with reserved bit 7 in glyf simple glyph flags#13144FurryR wants to merge 14 commits into
Conversation
DroidSansFallback.ttf (and Full variant) has reserved bit 7 set on 19959/19961 simple glyph coordinate flags. While ttf_parser tolerates this, skrifa/swash enforces the spec and rejects all outlines, causing CJK glyphs to render as .notdef squares. Reuse fontdb's mmap'd data to check for this corruption when loading file-based fonts. Corrupted fonts are removed from the database immediately so they cannot be matched by future font fallback queries. Non-corrupted fonts incur zero extra I/O. Signed-off-by: FurryR <awathefox@gmail.com>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds a font-load-time check that removes file-backed fonts whose simple glyf flags use reserved bit 7 so CJK fallback can skip corrupted Linux fonts.
Concerns
- The corruption check only inspects the first face returned by fontdb, so font collections can still load a corrupted requested face when it is not first.
- The detector samples a small fixed set of glyphs and checks only the first flag byte in each sampled glyph, so it can miss fonts that still contain invalid simple-glyph flags.
- This is a user-visible rendering fix, but the PR description has no screenshot or recording demonstrating CJK rendering before/after as required for user-visible changes.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
…etection for corrupted glyf tables - Load-time check now iterates all fontdb_ids (not just first), correctly handling font collections (.ttc) where non-first faces may be corrupted. - Add lazy runtime detection in swash rasterizer: track consecutive SwashCache::get_image_uncached failures per font; after 3 failures the font is removed from fontdb so future font fallback queries skip it. - Add corrupted_fonts set and raster_failures counter to TextLayoutSystem. - Short-circuit rasterization for confirmed-corrupted fonts. Signed-off-by: FurryR <awathefox@gmail.com>
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds Linux font filtering for corrupted glyf simple glyph flags and a runtime quarantine path for fonts that repeatedly fail swash rasterization, aiming to restore CJK fallback rendering when a bad DroidSansFallback face is installed.
Concerns
- The load-time corruption detector is incomplete: it samples only a few glyph IDs and only inspects the first flag byte for each sampled glyph, so fonts with reserved bit 7 set elsewhere can still be accepted.
- The runtime quarantine treats generic swash raster misses as evidence that an entire font is corrupted, which can remove valid fonts from fontdb and make future fallback skip them.
- This is a user-facing rendering fix, but the PR description does not include screenshots or a screen recording demonstrating the before/after CJK rendering behavior end to end.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Fully parse all simple glyph flags (respecting repeat flag) at load time for file-based fonts. Corrupted faces are removed from fontdb immediately so they cannot be matched by future fallback queries. Closes warpdotdev#9372
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR attempts to reject or quarantine fonts whose simple glyph flags set reserved bit 7, so corrupted Linux fallback fonts do not cause CJK glyphs to render as missing-glyph squares.
Concerns
- The swash rasterizer now calls corruption-tracking methods on
TextLayoutSystem, but the PR does not add those methods or any backing state, so the changed code will not compile. - This is a user-visible font rendering fix, but the PR description does not include screenshots or a screen recording demonstrating the before/after behavior end to end. Repo guidance requires visual evidence for user-visible behavior changes.
Verdict
Found: 1 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds font-load-time detection for file-based fonts whose glyf simple glyph flags set reserved bit 7, then removes corrupted faces from the font database so fallback can select another font. No approved spec context was provided, and the security pass did not find a concrete issue in the changed diff.
Concerns
- This is a user-visible text-rendering fix, but the PR does not include screenshots or a short recording demonstrating the before/after CJK rendering behavior. The repository review policy requires visual evidence for user-facing changes before approval.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
/oz-review Already attached a screenshot. Please reconsider. |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR filters file-backed fonts during WarpUI font loading by scanning simple glyph flags for reserved bit 7 and removing corrupted faces so fallback can avoid fonts that swash/skrifa reject. No approved spec context was provided, and I did not find security-specific issues in the changed diff.
Concerns
- This is a user-visible text-rendering change, but the PR description has no screenshot or screen recording attached and the screenshots/videos checkbox is left unchecked. For this user-facing change, please include screenshots or a short screen recording demonstrating the affected CJK fallback rendering working end to end.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
/oz-review |
|
This is your last |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: Comment Powered by Oz |
Oz no longer requests changes for this pull request after the latest automated review.
- Cap count_font_faces TTC numFonts to available offset table entries to prevent denial-of-service from malformed font files with exaggerated face counts. - Treat cached false values in BIT7_CHECKED_FONTS as corrupted instead of skipping them, preventing the background scanner from racing ahead and blocking removal of just-loaded corrupted faces. Signed-off-by: FurryR <awathefox@gmail.com>
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds detection for malformed TrueType simple glyph flags and removes corrupted font faces so font fallback can avoid fonts that cause CJK glyphs to render as .notdef squares.
Concerns
- Removing corrupted faces does not update
self.families, so later selection can still fall back to a removedFontIdand panic when the ID is missing fromfont_id_map.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
When drain_corrupted_fonts removes a face from fontdb, also remove its FontId from every family that references it, then drop families that become empty. This prevents select_font from falling back to a stale FontId after the face has been removed from fontdb, which would cause a panic in build_attrs_list when looking up the ID in font_id_map. Wrap families field in RwLock to support mutable access from the &self drain_corrupted_fonts method. Signed-off-by: FurryR <awathefox@gmail.com>
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR filters corrupted glyf simple glyph fonts from the winit text layout font database so Linux fallback rendering can skip fonts that set reserved bit 7.
Concerns
- The new
familiesread lock can be held while fallback loading re-enters corrupted-font cleanup, which may require afamilieswrite lock and deadlock font selection when pending corrupted fonts are drained.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
select_font held the families read lock across load_fallback_fonts, which can re-enter insert_font/drain_corrupted_fonts and deadlock when drain_corrupted_fonts tries to acquire families.write(). Copy the family name and first font ID from the read guard, then drop it before querying fontdb or loading fallbacks. Signed-off-by: FurryR <awathefox@gmail.com>
Previously drain_corrupted_fonts removed the font_id_map entry before cleaning the families map, creating a race window where select_font could read a stale FontId from families that was no longer in font_id_map, causing a panic in build_attrs_list. Now: read font_id from font_id_map, clean families first, then remove from font_id_map, so select_font can never return a FontId that is absent from font_id_map. Signed-off-by: FurryR <awathefox@gmail.com>
When drain_corrupted_fonts removes all faces for a family, that family is deleted from the families map. If select_font is called for the same family_id concurrently, it must not panic on the missing family. Instead, fall back to any valid font ID already loaded. Also clear font_selections for families that become empty, so stale cached FontIds from deleted families are not reused. Signed-off-by: FurryR <awathefox@gmail.com>
Keep the original panic behavior when a family is missing in select_font. Remove the any_valid_font_id fallback that was over-engineering for a race that cannot happen with single-threaded access. Signed-off-by: FurryR <awathefox@gmail.com>
|
/oz-review |
|
This is your last |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: Comment Powered by Oz |
Oz no longer requests changes for this pull request after the latest automated review.
There was a problem hiding this comment.
Overview
This PR adds a scanner for reserved bit 7 in simple glyf glyph flags and skips/removes corrupted file-backed fonts so Linux fallback can choose a working CJK font instead of rendering .notdef squares.
Concerns
- The synchronous check now walks glyph flags for uncached file-backed faces on the font-load path; consider narrowing the blocking work if startup or fallback latency regresses on systems with large font collections.
Verdict
Found: 0 critical, 0 important, 1 suggestions
Approve with nits
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
it sounds like i ask because if it would reject at load time, that might be a better way to fix this, but if not, then this seems like the approach we need to take. |
The latter one. The PR has already added multi-threaded (partially) non-blocking scans against corrupted fonts so I guess current workaround would be fine, but eventually we may need to report this to upstream dependency. |
|
Thanks for triggering CI. Working on formatting fixes. |
Signed-off-by: FurryR <awathefox@gmail.com>




Description
Reject fonts with reserved bit 7 set on simple glyph coordinate flags in the
glyftable. Some Linux fonts (notably DroidSansFallback.ttf fromttf-droid) have this corruption —skrifa/swashcorrectly enforces the spec and rejects all outlines, causing CJK glyphs to render as.notdefsquares.Detection runs at font-load time using
fontdb::with_face_data(reuses fontdb's existing mmap; zero extra I/O for healthy fonts). Corrupted faces are removed from the database so they can never be matched by future fallback queries.Linked Issue
ready-to-specorready-to-implement.Closes #9372.
Testing
./script/runManual verification:
has_reserved_bit7_in_glyph_flagsreturnstrueforDroidSansFallback.ttfandDroidSansFallbackFull.ttf,falseforDroidSansFallbackLegacy.ttf.DroidSansFallback.ttfrender as empty.Comparison:
We can actually use DroidSansFallback.ttf but it requires some hacky patching. If that is allowed then I will refactor this fix.
Agent Mode
Changelog Entries for Stable
CHANGELOG-BUG-FIX: Reject fonts with corrupted glyf table (reserved bit 7 on simple glyph flags) that caused CJK glyphs to render as .notdef squares on Linux