Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tx] fix hang in buildGIDNames #955

Merged
merged 1 commit into from
Sep 5, 2019
Merged

Conversation

cjchapman
Copy link
Contributor

(added upper bounds check on numGlyphs)

@cjchapman cjchapman changed the title fix hang in buildGIDNames [tx] fix hang in buildGIDNames Sep 4, 2019
Copy link
Contributor

@blueshade7 blueshade7 left a comment

Choose a reason for hiding this comment

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

I wonder how numGlyphs may become over 0xFFFF, CFF2 maybe?
There is nothing wrong about this fix, but there may be other places that may break with >64K glyphs.
Better to test with an actual font first.

blueshade7
blueshade7 previously approved these changes Sep 4, 2019
Copy link
Contributor

@blueshade7 blueshade7 left a comment

Choose a reason for hiding this comment

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

ok

added check for CFF2 glyph count exceeding limits imposed by other OpenType tables
@cjchapman
Copy link
Contributor Author

After consultation with @blueshade7 and @miguelsousa, I've added a check earlier in the code, and also added a test case.

Copy link
Contributor

@blueshade7 blueshade7 left a comment

Choose a reason for hiding this comment

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

looks good

@miguelsousa miguelsousa merged commit e4c023d into develop Sep 5, 2019
@miguelsousa miguelsousa deleted the cjc-fix-hang-in-buildGIDNames branch September 5, 2019 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants