search

foliojs / font-manager

A C++ module for Node.js providing access to the system font catalog.
MIT License
295 stars 100 forks source link

OS X null pointer crash on font missing family attribute #20

Closed lynaghk closed 4 years ago

lynaghk commented 7 years ago

Thanks for this and for fontkit, both are great libraries!

One of our designers ran into a crash on OS X that we tracked down to a font that was missing a "family" attribute. Specifically, this line, where CTFontDescriptorCopyAttribute returns null, which eventually blows up when the resulting FontDescriptor gets coerced to JavaScript world.

Options for a fix seem to be:

  1. Checking the string attributes for null and falling back to a default (e.g., "No Family Detected"), or

  2. Ignoring fonts that have missing attributes. The approach I'd take on this is to allow createFontDescriptor to return null if the font is busted, and explicitly handle this null in the places where it's called (getAvailableFonts, findFonts, and substituteFont).

The reasoning for the second option is that a font that's missing a family is probably broken in other ways. (Suitcase Fusion 6 describes the font that crashed font-manager as "corrupted".)

Happy to submit an OS X patch for whatever resolution you prefer. (Would also be happy if you want to just code it up yourself.)

Thanks again for the great libraries!

implausible commented 6 years ago

@lynaghk You wouldn't happen to have the font that caused your crashes hanging around, would you?

lynaghk commented 6 years ago

I do not, sorry. The font was from a client and I didn't save a copy after we resolved the issue on our own fork of this library. For what it's worth, the font was frutiger65_bold.ttf, though I have no idea where it was purchased/downloaded, etc.

fahrio commented 5 years ago

@implausible I just stumbled upon this issue when I installed a web font with a problematic 'name' table structure. I am attaching the font (it's a "trial" font) if you are still interested. 3825B1_0_0.ttf.zip

@lynaghk The font that is causing the crash is a web font and was not intended to be installed on the OS. It falls into your 2. category and can be ignored. I am building an app with this library, would you be able to share your patch or how to modify the library to ignore these "corrupt" fonts?

implausible commented 5 years ago

@fahrio Check out https://www.npmjs.com/package/font-scanner. It's a fork of this package with a few fixes we discovered when deploying this library in GitKraken.

devongovett commented 4 years ago

Should be fixed by #35.

@implausible if there are other fixes you guys made in your fork, we'd love for you to contribute them back!

devongovett commented 4 years ago

@implausible also, if you'd like to be a maintainer of font-manager itself, I'd add you to the repo.

implausible commented 4 years ago

@devongovett I'd definitely take you up on that. We've gotta fix the Electron issue https://github.com/foliojs/font-manager/issues/42