larrylynn-wf commented on a change in pull request #117:
URL: https://github.com/apache/pdfbox/pull/117#discussion_r631379275
##########
File path:
pdfbox/src/main/java/org/apache/pdfbox/pdmodel/font/TrueTypeEmbedder.java
##########
@@ -136,13 +136,20 @@ public final void buildFontFile2(InputStream ttfStream)
throws IOException
/**
* Returns true if the fsType in the OS/2 table permits embedding.
*/
- private boolean isEmbeddingPermitted(TrueTypeFont ttf) throws IOException
+ protected boolean isEmbeddingPermitted(TrueTypeFont ttf) throws IOException
{
if (ttf.getOS2Windows() != null)
{
int fsType = ttf.getOS2Windows().getFsType();
- if ((fsType & OS2WindowsMetricsTable.FSTYPE_RESTRICTED) ==
- OS2WindowsMetricsTable.FSTYPE_RESTRICTED)
+ int maskedFsType = fsType & 0x000F;
+ // use two's complement arithmetic to check if fsType is a power
of 2
+ // all legal fsType combinations for fonts having a version 3 OS2
table are powers of 2
+ // there are more legal combinations for versions 0-2, but the
most permissive pit takes precedence
+ boolean powerOfTwo = maskedFsType != 0 && (maskedFsType &
(maskedFsType - 1)) == 0;
Review comment:
I think that it's worth pointing out that this code will fail open on an
illegal configuration.
If a font with a version 3 OS2 table has multiple permission bits set, this
is an illegal configuration because version 3 explicitly specifies that
permission bits should be mutually exclusive. Suppose we are processing a font
with version 3 and permission set to 0110. This code would allow embedding of
the font with the illegal permission configuration. Essentially, it will fail
open.
I think this is OK because in this case, the creator of the font has not
made their wishes unambiguous and clear. What if the font creator wanted us to
respect the other permission bit and not FSTYPE_RESTRICTED. I think failing
open is OK, and it essentially puts the onus on font creators. If they want to
lock their fonts down and prevent embedding, it is up to them to provide a
legal fsType configuration that makes it clear that that is their intention.
I can see other points of view on this issue though, so I thought it was at
least worth pointing out for possible discussion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
