Unify error handling a bit.

src/cff/argstack.rs

@@ -1,5 +1,5 @@
 use crate::cff::number::Number;
-use crate::Error::MalformedFont;
+use crate::Error::CFFError;
 use crate::Result;
 
 /// The maximum number of operands allowed during parsing.
@@ -26,7 +26,7 @@ impl<'a> ArgumentsStack<'a> {
     #[inline]
     pub fn push(&mut self, n: Number<'a>) -> Result<()> {
         if self.len() == MAX_OPERANDS_LEN {
-            Err(MalformedFont)
+            Err(CFFError)
         } else {
             self.data.push(n);
             Ok(())

src/cff/charstring.rs

@@ -4,7 +4,7 @@ use crate::cff::operator::Operator;
 use crate::cff::subroutines::SubroutineHandler;
 use crate::read::Reader;
 use crate::write::Writer;
-use crate::Error::{MalformedFont, Unimplemented};
+use crate::Error::{CFFError, Unimplemented};
 use crate::{Error, Result};
 use operators::*;
 use std::fmt::{Debug, Formatter};
@@ -52,21 +52,20 @@ impl<'a> Decompiler<'a> {
         while !r.at_end() {
             // We need to peak instead of read because parsing a number requires
             // access to the whole buffer.
-            let op = r.peak::<u8>().ok_or(Error::MalformedFont)?;
+            let op = r.peak::<u8>().ok_or(Error::CFFError)?;
 
             // Numbers
             if matches!(op, 28 | 32..=255) {
-                let number =
-                    Number::parse_char_string_number(&mut r).ok_or(MalformedFont)?;
+                let number = Number::parse_char_string_number(&mut r).ok_or(CFFError)?;
                 self.stack.push(number.clone())?;
                 program.push(Instruction::Operand(number));
                 continue;
             }
 
             // No numbers can appear now, so now we can actually read it.
-            let op = r.read::<u8>().ok_or(Error::MalformedFont)?;
+            let op = r.read::<u8>().ok_or(CFFError)?;
             let operator = if op == 12 {
-                Operator::from_two_byte(r.read::<u8>().ok_or(Error::MalformedFont)?)
+                Operator::from_two_byte(r.read::<u8>().ok_or(CFFError)?)
             } else {
                 Operator::from_one_byte(op)
             };
@@ -98,23 +97,23 @@ impl<'a> Decompiler<'a> {
 
                     // TODO: Add depth limit / detect recursions
                     let biased_index =
-                        self.stack.pop().and_then(|n| n.as_i32()).ok_or(MalformedFont)?;
+                        self.stack.pop().and_then(|n| n.as_i32()).ok_or(CFFError)?;
                     let gsubr = self
                         .gsubr_handler
                         .get_with_biased(biased_index)
-                        .ok_or(MalformedFont)?;
+                        .ok_or(CFFError)?;
                     self.decompile_inner(gsubr, program)?;
                 }
                 CALL_LOCAL_SUBROUTINE => {
                     // Pop the subroutine index from the program.
                     program.0.pop();
                     // TODO: Add depth limit / detect recursions
                     let biased_index =
-                        self.stack.pop().and_then(|n| n.as_i32()).ok_or(MalformedFont)?;
+                        self.stack.pop().and_then(|n| n.as_i32()).ok_or(CFFError)?;
                     let lsubr = self
                         .lsubr_handler
                         .get_with_biased(biased_index)
-                        .ok_or(MalformedFont)?;
+                        .ok_or(CFFError)?;
                     self.decompile_inner(lsubr, program)?;
                 }
                 HINT_MASK | COUNTER_MASK => {
@@ -125,9 +124,8 @@ impl<'a> Decompiler<'a> {
                         self.hint_mask_bytes = (self.hint_count + 7) / 8;
                     }
 
-                    let hint_bytes = r
-                        .read_bytes(self.hint_mask_bytes as usize)
-                        .ok_or(MalformedFont)?;
+                    let hint_bytes =
+                        r.read_bytes(self.hint_mask_bytes as usize).ok_or(CFFError)?;
                     program.push(Instruction::HintMask(hint_bytes));
                 }
                 ENDCHAR => {
@@ -140,7 +138,7 @@ impl<'a> Decompiler<'a> {
                     program.push(Instruction::Operator(operator));
                 }
                 _ => {
-                    return Err(MalformedFont);
+                    return Err(CFFError);
                 }
             }
         }

src/cff/cid_font.rs

@@ -6,7 +6,7 @@ use crate::cff::index::{parse_index, Index};
 use crate::cff::remapper::FontDictRemapper;
 use crate::read::{LazyArray16, Reader};
 use crate::write::Writer;
-use crate::Error::SubsetError;
+use crate::Error::{MalformedFont, SubsetError};
 use crate::GlyphRemapper;
 use crate::Result;
 
@@ -125,7 +125,7 @@ pub fn rewrite_fd_index(
     w.write::<u8>(0);
 
     for gid in gid_remapper.remapped_gids() {
-        let old_fd = fd_select.font_dict_index(gid).ok_or(SubsetError)?;
+        let old_fd = fd_select.font_dict_index(gid).ok_or(MalformedFont)?;
         let new_fd = fd_remapper.get(old_fd).ok_or(SubsetError)?;
         w.write(new_fd);
     }

src/cff/dict/font_dict.rs

@@ -7,7 +7,7 @@ use crate::cff::remapper::{FontDictRemapper, SidRemapper};
 use crate::cff::{dict, Offsets};
 use crate::read::Reader;
 use crate::write::Writer;
-use crate::Error::{MalformedFont, SubsetError};
+use crate::Error::SubsetError;
 use crate::Result;
 use std::array;
 
@@ -71,7 +71,7 @@ pub fn rewrite_font_dict_index(
 
         // Write the font name, if applicable.
         if let Some(sid) = dict.font_name_sid {
-            let new_sid = sid_remapper.get(sid).ok_or(MalformedFont)?;
+            let new_sid = sid_remapper.get(sid).ok_or(SubsetError)?;
             w.write(Number::from_i32(new_sid.0 as i32));
             w.write(dict::operators::FONT_NAME);
         }

src/cff/index.rs

@@ -1,7 +1,7 @@
 use crate::cff::number::U24;
 use crate::read::{Readable, Reader};
 use crate::write::{Writeable, Writer};
-use crate::Error::MalformedFont;
+use crate::Error::OverflowError;
 use crate::Result;
 
 pub trait IndexSize: for<'a> Readable<'a> {
@@ -236,7 +236,7 @@ impl Default for OwnedIndex {
 
 /// Create an index from a vector of data.
 pub fn create_index(data: Vec<Vec<u8>>) -> Result<OwnedIndex> {
-    let count = u16::try_from(data.len()).map_err(|_| MalformedFont)?;
+    let count = u16::try_from(data.len()).map_err(|_| OverflowError)?;
     // + 1 Since we start counting from the preceding byte.
     let offsize = data.iter().map(|v| v.len() as u32).sum::<u32>() + 1;
 
@@ -266,11 +266,11 @@ pub fn create_index(data: Vec<Vec<u8>>) -> Result<OwnedIndex> {
 
         match offset_size {
             OffsetSize::Size1 => {
-                let num = u8::try_from(cur_offset).map_err(|_| MalformedFont)?;
+                let num = u8::try_from(cur_offset).map_err(|_| OverflowError)?;
                 w.write(num);
             }
             OffsetSize::Size2 => {
-                let num = u16::try_from(cur_offset).map_err(|_| MalformedFont)?;
+                let num = u16::try_from(cur_offset).map_err(|_| OverflowError)?;
                 w.write(num);
             }
             OffsetSize::Size3 => {

src/cff/mod.rs

@@ -25,7 +25,7 @@ use crate::cff::index::{create_index, parse_index, skip_index, Index, OwnedIndex
 use crate::cff::remapper::{FontDictRemapper, SidRemapper};
 use crate::cff::sid_font::SIDMetadata;
 use crate::cff::subroutines::{SubroutineCollection, SubroutineContainer};
-use crate::Error::SubsetError;
+use crate::Error::{OverflowError, SubsetError};
 use charset::charset_id;
 use number::{IntegerNumber, StringId};
 use std::cmp::PartialEq;
@@ -64,7 +64,7 @@ const DUMMY_OFFSET: DeferredOffset = DeferredOffset { location: 0, value: DUMMY_
 
 impl DeferredOffset {
     fn update_value(&mut self, value: usize) -> Result<()> {
-        self.value = IntegerNumber(i32::try_from(value).map_err(|_| SubsetError)?);
+        self.value = IntegerNumber(i32::try_from(value).map_err(|_| OverflowError)?);
         Ok(())
     }
 
@@ -365,7 +365,7 @@ impl<'a> Table<'a> {
         let major = r.read::<u8>().ok_or(MalformedFont)?;
 
         if major != 1 {
-            return Err(Error::Unimplemented);
+            return Err(Error::CFFError);
         }
 
         r.skip::<u8>(); // minor

src/glyf.rs

@@ -10,7 +10,6 @@
 //! 4. We need to calculate which format to use in the `loca` table.
 //! 5. We need to update the `loca` table itself with the new offsets.
 use super::*;
-use crate::Error::{MalformedFont, SubsetError};
 
 /// Form the glyph closure of all glyphs in `gid_set`.
 pub fn glyph_closure(face: &Face, glyph_remapper: &mut GlyphRemapper) -> Result<()> {
@@ -177,7 +176,7 @@ fn remap_component_glyph(mapper: &GlyphRemapper, data: &[u8]) -> Result<Vec<u8>>
         let flags = r.read::<u16>().ok_or(MalformedFont)?;
         w.write(flags);
         let old_component = r.read::<u16>().ok_or(MalformedFont)?;
-        let new_component = mapper.get(old_component).ok_or(SubsetError)?;
+        let new_component = mapper.get(old_component).ok_or(MalformedFont)?;
         w.write(new_component);
 
         if flags & ARG_1_AND_2_ARE_WORDS != 0 {

src/head.rs

@@ -3,7 +3,6 @@
 //! glyph data. The checksum will be recalculated in the very end.
 
 use super::*;
-use crate::Error::MalformedFont;
 
 pub fn subset(ctx: &mut Context) -> Result<()> {
     let mut head = ctx.expect_table(Tag::HEAD).ok_or(MalformedFont)?.to_vec();

src/hmtx.rs

@@ -9,7 +9,7 @@
 //! left side bearing metrics.
 
 use super::*;
-use crate::Error::{MalformedFont, SubsetError};
+use crate::Error::OverflowError;
 
 pub fn subset(ctx: &mut Context) -> Result<()> {
     let hmtx = ctx.expect_table(Tag::HMTX).ok_or(MalformedFont)?;
@@ -26,7 +26,7 @@ pub fn subset(ctx: &mut Context) -> Result<()> {
         };
 
         let last_advance_width = {
-            let index = 4 * num_h_metrics.checked_sub(1).ok_or(MalformedFont)? as usize;
+            let index = 4 * num_h_metrics.checked_sub(1).ok_or(OverflowError)? as usize;
             let mut r = Reader::new(hmtx.get(index..).ok_or(MalformedFont)?);
             r.read::<u16>().ok_or(MalformedFont)?
         };
@@ -58,7 +58,7 @@ pub fn subset(ctx: &mut Context) -> Result<()> {
 
     // Find out the last index we need to include the advance width for.
     let mut last_advance_width_index =
-        u16::try_from(new_metrics.len()).map_err(|_| SubsetError)? - 1;
+        u16::try_from(new_metrics.len()).map_err(|_| OverflowError)? - 1;
     let last_advance_width = new_metrics[last_advance_width_index as usize].0;
 
     for gid in new_metrics.iter().rev().skip(1) {
@@ -72,7 +72,7 @@ pub fn subset(ctx: &mut Context) -> Result<()> {
     let mut sub_hmtx = Writer::new();
 
     for (index, metric) in new_metrics.iter().enumerate() {
-        let index = u16::try_from(index).map_err(|_| SubsetError)?;
+        let index = u16::try_from(index).map_err(|_| OverflowError)?;
         if index <= last_advance_width_index {
             sub_hmtx.write::<u16>(metric.0);
         }

src/lib.rs

@@ -139,8 +139,10 @@ fn parse(data: &[u8], index: u32) -> Result<Face<'_>> {
         let subdata = data.get(offset as usize..).ok_or(MalformedFont)?;
         r = Reader::new(subdata);
         kind = r.read::<FontKind>().ok_or(MalformedFont)?;
+
+        // Cannot have nested collection
         if kind == FontKind::Collection {
-            return Err(UnknownKind);
+            return Err(MalformedFont);
         }
     }
 
@@ -461,15 +463,19 @@ type Result<T> = std::result::Result<T, Error>;
 pub enum Error {
     /// The file contains an unknown kind of font.
     UnknownKind,
-    /// The font seems to be malformed.
+    /// The font is malformed (or there is a bug in the font parsing logic).
     MalformedFont,
-    /// The font relies on some unimplemented feature, and thus we cannot guarantee
-    /// that the subsetted font would be correct.
+    /// The font relies on an unimplemented feature, and thus the subsetting
+    /// process couldn't be completed.
     Unimplemented,
-    /// An error occurred when subsetting the font.
+    /// An unexpected error occurred when subsetting the font. Indicates that there
+    /// is a logical bug in the subsetter.
     SubsetError,
-    /// Invald Mapper
-    InvalidGidMapper,
+    /// An overflow occurred during the computation. Could be either an issue
+    /// with the font itself, or a bug in the subsetter logic.
+    OverflowError,
+    /// An error occurred while processing the CFF table.
+    CFFError,
 }
 
 impl Display for Error {
@@ -479,7 +485,8 @@ impl Display for Error {
             Self::MalformedFont => f.write_str("malformed font"),
             Self::Unimplemented => f.write_str("unsupported feature in font"),
             Self::SubsetError => f.write_str("subsetting of font failed"),
-            Self::InvalidGidMapper => f.write_str("invalid gid mapper"),
+            Self::OverflowError => f.write_str("overflow occurred"),
+            Self::CFFError => f.write_str("processing CFF table failed"),
         }
     }
 }

src/maxp.rs

@@ -3,7 +3,6 @@
 //! can be copied from the old table.
 
 use super::*;
-use crate::Error::MalformedFont;
 
 pub fn subset(ctx: &mut Context) -> Result<()> {
     let maxp = ctx.expect_table(Tag::MAXP).ok_or(MalformedFont)?;

src/post.rs

@@ -6,7 +6,7 @@
 
 use super::*;
 use crate::read::LazyArray16;
-use crate::Error::{MalformedFont, SubsetError};
+use crate::Error::OverflowError;
 
 pub fn subset(ctx: &mut Context) -> Result<()> {
     let post = ctx.expect_table(Tag::POST).ok_or(MalformedFont)?;
@@ -39,8 +39,8 @@ pub fn subset(ctx: &mut Context) -> Result<()> {
             // Phetsarath-Regular.ttf from Google Fonts seems to have a wrong name table.
             // If name cannot be fetched, use empty name instead.
             let name = names.get(index as usize).copied().unwrap_or(&[][..]);
-            let name_len = u8::try_from(name.len()).map_err(|_| MalformedFont)?;
-            let index = u16::try_from(string_index + 258).map_err(|_| SubsetError)?;
+            let name_len = u8::try_from(name.len()).map_err(|_| OverflowError)?;
+            let index = u16::try_from(string_index + 258).map_err(|_| OverflowError)?;
             sub_post.write(index);
 
             string_storage.write(name_len);