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

Allow add fonts as files instead of input streams #613

Merged
merged 2 commits into from
Nov 30, 2020

Conversation

danfickle
Copy link
Owner

#612 #608 #405

This is to workaround a createFont JDK bug. This bug has been recently fixed but many users will not be using the latest JDK.

The problem is that Font.createFont with an input stream creates a temporary file which is not deleted promptly after use. This PR means that users can now supply fonts to SVG and MathML implementations as files to workaround this bug.

This PR is an alternative to #612. That PR worked around the issue by trying to convert the input stream into a temp file and managing it ourselves rather than the JDK. I was a little scared of that solution given security issues with the temp directory (see this bug for example) and some possibility of files not being deleted correctly.

Recommendations (after 1.0.5 is released, very soon):

  • If using Java2D, SVG or MathML, don't use @font-face rules as these use an input stream behind the scenes.
  • Use the builder.useFont methods to add fonts.
  • For PDF only, the input stream methods are fine as Font.createFont is not used.
  • For Java2D, SVG and MathML use the File variants of builder.useFont.
  • Use the latest JDK if possible.

@syjer
Copy link
Contributor

syjer commented Nov 30, 2020

If using Java2D, SVG or MathML, don't use @font-face rules as these use an input stream behind the scenes.

I wonder if we could at least add for the SVGDrawer, as sometimes you can't really avoid the @font-face in css files, a configuration option to avoid loading InputStream based font.

@danfickle
Copy link
Owner Author

There are a couple of options:

@font-face {
   src: url(...);
   font-family: 'my-font';
   -fs-font-for: document;
}

ie. A comma separated list of uses for the font. It would probably have to default to all for backwards compatibility but that would allow avoiding loading @font-face fonts for SVG while using them for PDFs. After all most SVGs don't use text.

Another option would be to implement something like the local font tag to take a path instead of a uri. That would avoid input streams.

However, I promised a release for yesterday and I'm nearly out of today, so that will have to wait for the next release.

@syjer
Copy link
Contributor

syjer commented Nov 30, 2020

nice ideas, I'll have a look what it would mean in term of implementations :)

@syjer
Copy link
Contributor

syjer commented Nov 30, 2020

As another possible idea, if Font is thread safe (?), we could keep a ConcurrentMap<Hash, Font> and share it across the jvm process.

edit: actually, we could handle it with a lock on the createFont call and synchronize on the font object itself when calling deriveFont, ugly and quite unfortunate, but possible.

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.

None yet

2 participants