Merge pull request #267 from jfkthame/maxp-version

[maxp] Attempt to improve fixup of bad 'maxp' version numbers.
khaledhosny · Nov 12, 2023 · be94458 · be94458
2 parents 328c479 + 83ff34d
commit be94458
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 33 deletions.
diff --git a/src/maxp.cc b/src/maxp.cc
@@ -29,39 +29,49 @@ bool OpenTypeMAXP::Parse(const uint8_t *data, size_t length) {
     return Error("numGlyphs is 0");
   }
 
-  if (version >> 16 == 1) {
-    this->version_1 = true;
-    if (!table.ReadU16(&this->max_points) ||
-        !table.ReadU16(&this->max_contours) ||
-        !table.ReadU16(&this->max_c_points) ||
-        !table.ReadU16(&this->max_c_contours) ||
-        !table.ReadU16(&this->max_zones) ||
-        !table.ReadU16(&this->max_t_points) ||
-        !table.ReadU16(&this->max_storage) ||
-        !table.ReadU16(&this->max_fdefs) ||
-        !table.ReadU16(&this->max_idefs) ||
-        !table.ReadU16(&this->max_stack) ||
-        !table.ReadU16(&this->max_size_glyf_instructions) ||
-        !table.ReadU16(&this->max_c_components) ||
-        !table.ReadU16(&this->max_c_depth)) {
-      return Error("Failed to read version 1 table data");
-    }
-
-    if (this->max_zones == 0) {
-      // workaround for ipa*.ttf Japanese fonts.
-      Warning("Bad maxZones: %u", this->max_zones);
-      this->max_zones = 1;
-    } else if (this->max_zones == 3) {
-      // workaround for Ecolier-*.ttf fonts.
-      Warning("Bad maxZones: %u", this->max_zones);
-      this->max_zones = 2;
-    }
-
-    if ((this->max_zones != 1) && (this->max_zones != 2)) {
-      return Error("Bad maxZones: %u", this->max_zones);
-    }
-  } else {
-    this->version_1 = false;
+  this->version_1 = false;
+
+  // Per https://learn.microsoft.com/en-gb/typography/opentype/spec/maxp,
+  // the only two 'maxp' version numbers are 0.5 (for CFF/CFF2) and 1.0
+  // (for TrueType).
+  // If it's version 0.5, there is nothing more to read.
+  if (version == 0x00005000) {
+    return true;
+  }
+
+  if (version != 0x00010000) {
+    Warning("Unexpected version 0x%08x; attempting to read as version 1.0",
+            version);
+  }
+
+  // Otherwise, try to read the version 1.0 fields:
+  if (!table.ReadU16(&this->max_points) ||
+      !table.ReadU16(&this->max_contours) ||
+      !table.ReadU16(&this->max_c_points) ||
+      !table.ReadU16(&this->max_c_contours) ||
+      !table.ReadU16(&this->max_zones) ||
+      !table.ReadU16(&this->max_t_points) ||
+      !table.ReadU16(&this->max_storage) ||
+      !table.ReadU16(&this->max_fdefs) ||
+      !table.ReadU16(&this->max_idefs) ||
+      !table.ReadU16(&this->max_stack) ||
+      !table.ReadU16(&this->max_size_glyf_instructions) ||
+      !table.ReadU16(&this->max_c_components) ||
+      !table.ReadU16(&this->max_c_depth)) {
+    Warning("Failed to read version 1.0 fields, downgrading to version 0.5");
+    return true;
+  }
+
+  this->version_1 = true;
+
+  if (this->max_zones < 1) {
+    // workaround for ipa*.ttf Japanese fonts.
+    Warning("Bad maxZones: %u", this->max_zones);
+    this->max_zones = 1;
+  } else if (this->max_zones > 2) {
+    // workaround for Ecolier-*.ttf fonts and bad fonts in some PDFs
+    Warning("Bad maxZones: %u", this->max_zones);
+    this->max_zones = 2;
   }
 
   return true;

diff --git a/src/ots.cc b/src/ots.cc
@@ -762,12 +762,18 @@ bool ProcessGeneric(ots::FontFile *header,
   ots::Table *loca = font->GetTable(OTS_TAG_LOCA);
   ots::Table *cff  = font->GetTable(OTS_TAG_CFF);
   ots::Table *cff2 = font->GetTable(OTS_TAG_CFF2);
+  ots::OpenTypeMAXP *maxp = static_cast<ots::OpenTypeMAXP*>(
+    font->GetTypedTable(OTS_TAG_MAXP));
 
   if (glyf && loca) {
     if (font->version != 0x000010000) {
       OTS_WARNING_MSG_HDR("wrong sfntVersion for glyph data");
       font->version = 0x000010000;
     }
+    if (!maxp->version_1) {
+      return OTS_FAILURE_MSG_TAG("wrong maxp version for glyph data",
+                                 OTS_TAG_MAXP);
+    }
     if (cff)
        cff->Drop("font contains both CFF and glyf/loca tables");
     if (cff2)
@@ -781,6 +787,10 @@ bool ProcessGeneric(ots::FontFile *header,
        glyf->Drop("font contains both CFF and glyf tables");
     if (loca)
        loca->Drop("font contains both CFF and loca tables");
+    if (maxp->version_1) {
+      OTS_WARNING_MSG_HDR("fixing incorrect maxp version for CFF font");
+      maxp->version_1 = false;
+    }
   } else if (font->GetTable(OTS_TAG('C','B','D','T')) &&
              font->GetTable(OTS_TAG('C','B','L','C'))) {
       // We don't sanitize bitmap tables, but don’t reject bitmap-only fonts if