Return error values in ReadableFontData::Read*().

Change Read*Byte methods in ReadableFontData to return sentinel values
on error. Check the return values in other methods, and repeat the
process if they are also Read() methods.
Fixes https://crbug.com/641460

2 files changed, 77 insertions, 36 deletions

diff --git a/cpp/src/sfntly/data/readable_font_data.cc b/cpp/src/sfntly/data/readable_font_data.cc
index 06d783f..0f93fdb 100644
--- a/cpp/src/sfntly/data/readable_font_data.cc
+++ b/cpp/src/sfntly/data/readable_font_data.cc
@@ -59,23 +59,25 @@ void ReadableFontData::SetCheckSumRanges(const IntegerList& ranges) {
 
 int32_t ReadableFontData::ReadUByte(int32_t index) {
   int32_t b = array_->Get(BoundOffset(index));
-#if !defined (SFNTLY_NO_EXCEPTION)
   if (b < 0) {
+#if !defined (SFNTLY_NO_EXCEPTION)
     throw IndexOutOfBoundException(
         "Index attempted to be read from is out of bounds", index);
-  }
 #endif
+    return -1;
+  }
   return b;
 }
 
 int32_t ReadableFontData::ReadByte(int32_t index) {
   int32_t b = array_->Get(BoundOffset(index));
-#if !defined (SFNTLY_NO_EXCEPTION)
   if (b < 0) {
+#if !defined (SFNTLY_NO_EXCEPTION)
     throw IndexOutOfBoundException(
         "Index attempted to be read from is out of bounds", index);
-  }
 #endif
+    return kInvalidByte;
+  }
   return (b << 24) >> 24;
 }
 
@@ -91,24 +93,52 @@ int32_t ReadableFontData::ReadChar(int32_t index) {
 }
 
 int32_t ReadableFontData::ReadUShort(int32_t index) {
-  return 0xffff & (ReadUByte(index) << 8 | ReadUByte(index + 1));
+  int32_t b1 = ReadUByte(index);
+  if (b1 < 0)
+    return -1;
+  int32_t b2 = ReadUByte(index + 1);
+  if (b2 < 0)
+    return -1;
+  return 0xffff & (b1 << 8 | b2);
 }
 
 int32_t ReadableFontData::ReadShort(int32_t index) {
-  return ((ReadByte(index) << 8 | ReadUByte(index + 1)) << 16) >> 16;
+  int32_t b1 = ReadByte(index);
+  if (b1 == kInvalidByte)
+    return kInvalidShort;
+  int32_t b2 = ReadUByte(index + 1);
+  if (b2 < 0)
+    return kInvalidShort;
+  return ((b1 << 8 | b2) << 16) >> 16;
 }
 
 int32_t ReadableFontData::ReadUInt24(int32_t index) {
-  return 0xffffff & (ReadUByte(index) << 16 |
-                     ReadUByte(index + 1) << 8 |
-                     ReadUByte(index + 2));
+  int32_t b1 = ReadUByte(index);
+  if (b1 < 0)
+    return -1;
+  int32_t b2 = ReadUByte(index + 1);
+  if (b2 < 0)
+    return -1;
+  int32_t b3 = ReadUByte(index + 2);
+  if (b3 < 0)
+    return -1;
+  return 0xffffff & (b1 << 16 | b2 << 8 | b3);
 }
 
 int64_t ReadableFontData::ReadULong(int32_t index) {
-  return 0xffffffffL & (ReadUByte(index) << 24 |
-                        ReadUByte(index + 1) << 16 |
-                        ReadUByte(index + 2) << 8 |
-                        ReadUByte(index + 3));
+  int32_t b1 = ReadUByte(index);
+  if (b1 < 0)
+    return -1;
+  int32_t b2 = ReadUByte(index + 1);
+  if (b2 < 0)
+    return -1;
+  int32_t b3 = ReadUByte(index + 2);
+  if (b3 < 0)
+    return -1;
+  int32_t b4 = ReadUByte(index + 3);
+  if (b4 < 0)
+    return -1;
+  return 0xffffffffL & (b1 << 24 | b2 << 16 | b3 << 8 | b4);
 }
 
 int32_t ReadableFontData::ReadULongAsInt(int32_t index) {
@@ -122,10 +152,19 @@ int32_t ReadableFontData::ReadULongAsInt(int32_t index) {
 }
 
 int64_t ReadableFontData::ReadULongLE(int32_t index) {
-  return 0xffffffffL & (ReadUByte(index) |
-                        ReadUByte(index + 1) << 8 |
-                        ReadUByte(index + 2) << 16 |
-                        ReadUByte(index + 3) << 24);
+  int32_t b1 = ReadUByte(index);
+  if (b1 < 0)
+    return -1;
+  int32_t b2 = ReadUByte(index + 1);
+  if (b2 < 0)
+    return -1;
+  int32_t b3 = ReadUByte(index + 2);
+  if (b3 < 0)
+    return -1;
+  int32_t b4 = ReadUByte(index + 3);
+  if (b4 < 0)
+    return -1;
+  return 0xffffffffL & (b1 | b2 << 8 | b3 << 16 | b4 << 24);
 }
 
 int32_t ReadableFontData::ReadLong(int32_t index) {
@@ -187,12 +226,11 @@ int32_t ReadableFontData::SearchUShort(int32_t start_index,
 #if defined (SFNTLY_DEBUG_FONTDATA)
       fprintf(stderr, "**start: %d; end: %d\n", location_start, location_end);
 #endif
-      if (key <= location_end) {
+      if (key <= location_end)
         return location;
-      } else {
-        // location is above the current location
-        bottom = location + 1;
-      }
+
+      // location is above the current location
+      bottom = location + 1;
     }
   }
   return -1;
@@ -208,14 +246,15 @@ int32_t ReadableFontData::SearchUShort(int32_t start_index,
   while (top != bottom) {
     location = (top + bottom) / 2;
     int32_t location_start = ReadUShort(start_index + location * start_offset);
+    if (key == location_start)
+      return location;
+
     if (key < location_start) {
       // location is below current location
       top = location;
-    } else if (key > location_start) {
+    } else {
       // location is above current location
       bottom = location + 1;
-    } else {
-      return location;
     }
   }
   return -1;
@@ -243,12 +282,11 @@ int32_t ReadableFontData::SearchULong(int32_t start_index,
 #if defined (SFNTLY_DEBUG_FONTDATA)
       fprintf(stderr, "**start: %d; end: %d\n", location_start, location_end);
 #endif
-      if (key <= location_end) {
+      if (key <= location_end)
         return location;
-      } else {
-        // location is above the current location
-        bottom = location + 1;
-      }
+
+      // location is above the current location
+      bottom = location + 1;
     }
   }
   return -1;
diff --git a/cpp/src/sfntly/data/readable_font_data.h b/cpp/src/sfntly/data/readable_font_data.h
index b43c626..a4a3e27 100644
--- a/cpp/src/sfntly/data/readable_font_data.h
+++ b/cpp/src/sfntly/data/readable_font_data.h
@@ -22,8 +22,8 @@
 
 namespace sfntly {
 
-class WritableFontData;
 class OutputStream;
+class WritableFontData;
 
 // Writable font data wrapper. Supports reading of data primitives in the
 // TrueType / OpenType spec.
@@ -51,6 +51,9 @@ class ReadableFontData : public FontData,
   explicit ReadableFontData(ByteArray* array);
   virtual ~ReadableFontData();
 
+  static const int32_t kInvalidByte = 128;
+  static const int32_t kInvalidShort = 32768;
+
   static CALLER_ATTACH ReadableFontData* CreateReadableFontData(ByteVector* b);
 
   // Gets a computed checksum for the data. This checksum uses the OpenType spec
@@ -76,7 +79,7 @@ class ReadableFontData : public FontData,
 
   // Read the BYTE at the given index.
   // @param index index into the font data
-  // @return the BYTE
+  // @return the BYTE; |kInvalidByte| if outside the bounds of the font data
   // @throws IndexOutOfBoundsException if index is outside the FontData's range
   virtual int32_t ReadByte(int32_t index);
 
@@ -94,25 +97,25 @@ class ReadableFontData : public FontData,
 
   // Read the CHAR at the given index.
   // @param index index into the font data
-  // @return the CHAR
+  // @return the CHAR; -1 if outside the bounds of the font data
   // @throws IndexOutOfBoundsException if index is outside the FontData's range
   virtual int32_t ReadChar(int32_t index);
 
   // Read the USHORT at the given index.
   // @param index index into the font data
-  // @return the USHORT
+  // @return the USHORT; -1 if outside the bounds of the font data
   // @throws IndexOutOfBoundsException if index is outside the FontData's range
   virtual int32_t ReadUShort(int32_t index);
 
   // Read the SHORT at the given index.
   // @param index index into the font data
-  // @return the SHORT
+  // @return the SHORT; |kInvalidShort| if outside the bounds of the font data
   // @throws IndexOutOfBoundsException if index is outside the FontData's range
   virtual int32_t ReadShort(int32_t index);
 
   // Read the UINT24 at the given index.
   // @param index index into the font data
-  // @return the UINT24
+  // @return the UINT24; -1 if outside the bounds of the font data
   // @throws IndexOutOfBoundsException if index is outside the FontData's range
   virtual int32_t ReadUInt24(int32_t index);