Re: Fix Type1 (PFA and PFB) font embedding (issue 313930043 by address@hidden)

[trueroad]

Reviewers: lemzwerg,

Message:
Thank you for your reviewing.

> https://codereview.appspot.com/313930043/diff/1/lily/pfb.cc
> File lily/pfb.cc (right):

> https://codereview.appspot.com/313930043/diff/1/lily/pfb.cc#newcode49
> lily/pfb.cc:49: seglen |= (static_cast<Byte>(*p++) << 24);
> I suggest to add a guard against invalid, possibly extremely large
`seglen'
> values...

> https://codereview.appspot.com/313930043/diff/1/lily/pfb.cc#newcode62
> lily/pfb.cc:62: static_cast<int>(static_cast<Byte>(*p++)));
> As above, we should probably add a guard against invalid data...

I'll consider the detection of out-of-range access.

> I also wonder whether `sprintf' is the most efficient tool for
converting two
> hex digits into a byte value.

How about stringstream?

std::stringstream ss;
ss << std::hex << std::setw (2) << std::setfill ('0')
   << static_cast<int>(static_cast<Byte>(*p++));
out.push_back (ss.str[0]);
out.push_back (ss.str[1]);



Description:
Fix Type1 (PFA and PFB) font embedding

Fix Type1 (PFB) font embedding

Type1 PFA data which is converted from PFB files can contain '\0'.

In order to avoid problems with '\0',
this commit makes to use the file length
instead of zero-terminated string.

Fix Type1 (PFA) font embedding

Type1 (PFA) fonts can contain '\0'.
e.g. URW++ June 2016 (Ghostscript 9.20)

In order to avoid problems with '\0',
this commit makes to use the file length
instead of zero-terminated string.

Please review this at https://codereview.appspot.com/313930043/

Affected files (+23, -36 lines):
  M lily/include/font-metric.hh
  M lily/pfb.cc
  M lily/pfb-scheme.cc


Index: lily/include/font-metric.hh
diff --git a/lily/include/font-metric.hh b/lily/include/font-metric.hh
index  
e385a1e7af7ad603a9bb043b0c7fa4dcb6b9a995..7a388f86f35c1031f59c023805802b5c973c9cfc  
100644
--- a/lily/include/font-metric.hh
+++ b/lily/include/font-metric.hh
@@ -72,6 +72,6 @@ protected:
 };


-char *pfb2pfa (Byte const *pfb, int length);
+vector<char> pfb2pfa (const vector<char> &pfb);

 #endif /* FONT_METRIC_HH */
Index: lily/pfb-scheme.cc
diff --git a/lily/pfb-scheme.cc b/lily/pfb-scheme.cc
index  
e718a13bf079f36c89708aebf65ddbcf1aa98611..37e44da8b05ed7fd1eff145b6c138b40c03c77ea  
100644
--- a/lily/pfb-scheme.cc
+++ b/lily/pfb-scheme.cc
@@ -25,16 +25,14 @@ LY_DEFINE (ly_type1_2_pfa, "ly:type1->pfa",
   if ((Byte) type1_string[0] == 0x80)
     {
       /* The file is in PFB format. Convert it to PFA format. */
-      char *pfa = pfb2pfa ((Byte *) &type1_string[0],
-                           (int) type1_string.size ());
-      pfa_scm = scm_from_latin1_string (pfa);
-      free (pfa);
+      vector<char> pfa = pfb2pfa (type1_string);
+      pfa_scm = scm_from_latin1_stringn (&pfa[0], pfa.size ());
     }
   else
     {
       /* The file is in PFA format. Pass it through. */
-      type1_string.push_back(0);
-      pfa_scm = scm_from_latin1_string (&type1_string[0]);
+      pfa_scm = scm_from_latin1_stringn (&type1_string[0],
+                                         type1_string.size ());
     }

   debug_output ("]", false);
Index: lily/pfb.cc
diff --git a/lily/pfb.cc b/lily/pfb.cc
index  
c971024548087c547c826da0d323dfa5a37c972c..4c385748a1b4b25a2bc26a1c2cb79289f1901cd6  
100644
--- a/lily/pfb.cc
+++ b/lily/pfb.cc
@@ -29,55 +29,44 @@ using namespace std;
 #include "main.hh"
 #include "warn.hh"

-char *
-pfb2pfa (Byte const *pfb, int length)
+vector<char> pfb2pfa (const vector<char> &pfb)
 {
-  char *out = (char *) malloc (sizeof (char));
-  long olen = 0;
+  vector<char> out;

-  Byte const *p = pfb;
-  while (p < pfb + length)
+  vector<char>::const_iterator p = pfb.begin ();
+  while (p < pfb.end ())
     {
-      if (*p++ != 128)
+      if (static_cast<Byte>(*p++) != 128)
         break;

-      Byte type = *p++;
+      Byte type = static_cast<Byte>(*p++);
       if (type == 3)
         break;

-      unsigned seglen
-        = p[0] | (p[1] << 8)
-          | (p[2] << 16) | (p[3] << 24);
+      size_t seglen = static_cast<Byte>(*p++);
+      seglen |= (static_cast<Byte>(*p++) << 8);
+      seglen |= (static_cast<Byte>(*p++) << 16);
+      seglen |= (static_cast<Byte>(*p++) << 24);

-      p += 4;
       if (type == 1)
         {
-          out = (char *)realloc (out, olen + seglen + 1);
-          char *outp = out + olen;
-          memcpy (outp, p, seglen);
-          olen += seglen;
+          copy (p, p + seglen, back_inserter (out));
           p += seglen;
         }
       else if (type == 2)
         {
-          unsigned outlength = (seglen * 2) + (seglen / 32) + 2;
-
-          out = (char *)realloc (out, olen + outlength + 1);
-
-          char *outp = out + olen;
-          for (int i = seglen; i--;)
+          for (size_t i = seglen; i > 0; --i)
             {
-              sprintf (outp, "%02x", *p++);
-              outp += 2;
+              char buff[3];
+              snprintf (buff, sizeof (buff), "%02x",
+                        static_cast<int>(static_cast<Byte>(*p++)));
+              out.push_back (buff[0]);
+              out.push_back (buff[1]);
               if (! (i % 32))
-                *outp++ = '\n';
+                out.push_back ('\n');
             }
-
-          olen = outp - out;
         }
     }
-  out[olen] = 0;

   return out;
 }
-