FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

ft2font: Make glyph loading failures more robust by QuLogic · Pull Request #32226 · matplotlib/matplotlib · GitHub

/ matplotlib Public

ft2font: Make glyph loading failures more robust - #32226

Open
QuLogic wants to merge 1 commit into
matplotlib:mainfrom
QuLogic:ft2font-load-failure
ConversationCommits (1)ChecksFiles changed
Open

ft2font: Make glyph loading failures more robust#32226
QuLogic wants to merge 1 commit into
matplotlib:mainfrom
QuLogic:ft2font-load-failure

Conversation

QuLogic commented Aug 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

PR summary

The main fix is to FT2Font::load_char, which didn't correctly fail if no fallback was defined to catch missing glyphs.

Also, while PyGlyph_from_FT2Font is internal and should never be called before loading a glyph, add a check in there anyway, just to be safe.

Fixes #32224

AI Disclosure

None

PR quality check

  • Use an expressive title, e.g. "Fix title font property precedence"
  • New and changed code is tested
  • [n/a] Plotting related features are demonstrated in an example
  • [n/a] New features and API changes have release notes
  • [n/a] Documentation complies with general and docstring guidelines

QuLogic added this to the v3.11.2 milestone Aug 18, 2026
iccir reviewed Aug 21, 2026
View reviewed changes
Comment thread src/ft2font_wrapper.cpp
bool fallback = true;
FT2Font *ft_object = nullptr;

self->load_char(charcode, static_cast<FT_Int32>(flags), ft_object, fallback);

iccir Aug 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm worried about a future change to load_char() failing to fill ft_object. A subsequent call to PyGlyph_from_FT2Font() will crash due to the nullptr dereference.

From a C/Obj-C background, I'd like to see an explicit check against nullptr after this call and an exception thrown if nullptr. However, I'm not sure if that matches our C++.

The PR addresses the original bug; however, and I'm fine with approving it as-is.

QuLogic Aug 21, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

That's also a pretty reasonable thing to check.

ft2font: Make glyph loading failures more robust
6adedac 
The main fix is to `FT2Font::load_char`, which didn't correctly fail if
no fallback was defined to catch missing glyphs.

Also, while `PyGlyph_from_FT2Font` is internal and should never be
called before loading a glyph, add a check in there anyway, just to be
safe.
QuLogic force-pushed the ft2font-load-failure branch from 291e0ec to 6adedac Compare August 21, 2026 03:43
iccir approved these changes Aug 21, 2026
View reviewed changes

iccir left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good now!

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

iccir iccir approved these changes

Assignees

No one assigned

Labels

topic: text/fonts

Projects

None yet

Milestone

v3.11.2

Development

Successfully merging this pull request may close these issues.

[Bug]: FT2Font.load_char() segfaults (null-pointer dereference)

2 participants

Back | FazBrowse Home | New Git URL