ticket:28 fixed byte array sourced font bounds checking

java/src/com/google/typography/font/sfntly/data/ByteArray.java

@@ -98,7 +98,7 @@ public int get(int index, byte[] b) {
    */
   public int get(int index, byte[] b, int offset, int length) {
     if (index < 0 || index >= this.filledLength) {
-      return 0;
+      return -1;
     }
     int actualLength = Math.min(length, this.filledLength - index);
     return this.internalGet(index, b, offset, actualLength);

java/src/com/google/typography/font/sfntly/data/FontData.java

@@ -198,4 +198,14 @@ protected final int boundOffset(int offset) {
   protected final int boundLength(int offset, int length) {
     return Math.min(length, this.boundLength - offset);
   }
+
+  protected final boolean boundsCheck(int offset, int length) {
+    if (offset < 0 || offset >= this.boundLength) {
+      return false;
+    }
+    if (length < 0 || length + offset > this.boundLength) {
+      return false;
+    }
+    return true;
+  }
 }

java/src/com/google/typography/font/sfntly/data/ReadableFontData.java

@@ -311,6 +311,10 @@ public int[] checkSumRange() {
    * @throws IndexOutOfBoundsException if index is outside the FontData's range
    */
   public int readUByte(int index) {
+    if (!this.boundsCheck(index, 1)) {
+      throw new IndexOutOfBoundsException(
+          "Index attempted to be read from is out of bounds: " + Integer.toHexString(index));
+    }
     int b = this.array.get(this.boundOffset(index));
     if (b < 0) {
       throw new IndexOutOfBoundsException(
@@ -328,6 +332,10 @@ public int readUByte(int index) {
    * @throws IndexOutOfBoundsException if index is outside the FontData's range
    */
   public int readByte(int index) {
+    if (!this.boundsCheck(index, 1)) {
+      throw new IndexOutOfBoundsException(
+          "Index attempted to be read from is out of bounds: " + Integer.toHexString(index));
+    }
     int b = this.array.get(this.boundOffset(index));
     if (b < 0) {
       throw new IndexOutOfBoundsException(
@@ -348,7 +356,13 @@ public int readByte(int index) {
    *         bounds of the font data
    */
   public int readBytes(int index, byte[] b, int offset, int length) {
-    return this.array.get(this.boundOffset(index), b, offset, this.boundLength(index, length));
+    int bytesRead = 
+        this.array.get(this.boundOffset(index), b, offset, this.boundLength(index, length));
+    if (bytesRead < 0) {
+      throw new IndexOutOfBoundsException(
+          "Index attempted to be read from is out of bounds: " + Integer.toHexString(index));      
+    }
+    return bytesRead;
   }
 
   /**

java/src/com/google/typography/font/sfntly/table/truetype/Glyph.java

@@ -45,6 +45,9 @@ protected Glyph(ReadableFontData data, int offset, int length, Glyph.GlyphType g
   }
 
   private static Glyph.GlyphType glyphType(ReadableFontData data, int offset, int length) {
+    if (offset > data.length()) {
+      throw new IndexOutOfBoundsException();
+    }
     if (length == 0) {
       return GlyphType.Simple;
     }
@@ -87,6 +90,15 @@ public Glyph.GlyphType glyphType() {
     return this.glyphType;
   }
 
+  /**
+   * Gets the number of contours in the glyph. If this returns a number greater
+   * than or equal to zero it is the actual number of contours and this is a
+   * simple glyph. If there are zero contours in the glyph then none of the
+   * other data operations will return usable values. If it -1 then the glyph is
+   * a composite glyph.
+   * 
+   * @return number of contours
+   */
   public int numberOfContours() {
     return this.numberOfContours;
   }

java/test/com/google/typography/font/sfntly/issue_tests/Issue28Tests.java

@@ -0,0 +1,91 @@
+package com.google.typography.font.sfntly.issue_tests;
+
+import com.google.typography.font.sfntly.Font;
+import com.google.typography.font.sfntly.FontFactory;
+import com.google.typography.font.sfntly.Tag;
+import com.google.typography.font.sfntly.table.truetype.Glyph;
+import com.google.typography.font.sfntly.table.truetype.GlyphTable;
+import com.google.typography.font.sfntly.table.truetype.LocaTable;
+import com.google.typography.font.sfntly.testutils.TestFont;
+
+import junit.framework.TestCase;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+/**
+ * Test class showing inconsistent behaviour between loading from a byte array
+ * vs loading from a file input stream.
+ */
+public class Issue28Tests extends TestCase {
+
+  private static byte[] readToByteArray(File file) throws IOException {
+    byte[] data = new byte[(int) file.length()];
+    FileInputStream fis = new FileInputStream(file);
+
+    try {
+      fis.read(data);
+    } finally {
+      fis.close();
+    }
+
+    return data;
+  }
+
+  private static Glyph getLastGlyph(Font font) {
+    LocaTable locaTable = font.getTable(Tag.loca);
+    int glyphId = locaTable.numGlyphs() - 1;
+    GlyphTable glyfTable = font.getTable(Tag.glyf);
+    int offset = locaTable.glyphOffset(glyphId);
+    int length = locaTable.glyphLength(glyphId);
+    return glyfTable.glyph(offset, length);
+  }
+
+  /**
+   * Ensure that the stream and byte array sourced fonts both throw an exception when you
+   * read off the end of a sliced ReadableFontData
+   */
+  public void testStreamVsBytes() throws Exception {
+    FontFactory factory = FontFactory.getInstance();
+
+    byte[] data = readToByteArray(TestFont.TestFontNames.ROBOTO.getFile());
+    Font byteFont = factory.loadFonts(data)[0];
+
+    InputStream is = new FileInputStream(TestFont.TestFontNames.ROBOTO.getFile());
+    Font streamFont;
+    try {
+      streamFont = factory.loadFonts(is)[0];
+    } finally {
+      is.close();
+    }
+
+
+    // first test for byte array sourced font
+    {
+      boolean thrown = false;
+      Glyph byteGlyph = getLastGlyph(byteFont);
+      try {
+        int byteXMin = byteGlyph.xMin();
+      } catch (IndexOutOfBoundsException e) {
+        // expected exception
+        thrown = true;
+      }
+      assertTrue("IndexOutOfBoundsException was expected but was not thrown.", thrown);
+    }
+
+    // next test for stream sourced font
+    {
+      boolean thrown = false;
+      Glyph streamGlyph = getLastGlyph(streamFont);
+      try {
+        int streamXMin = streamGlyph.xMin();
+      } catch (IndexOutOfBoundsException e) {
+        // expected exception
+        thrown = true;
+      }
+      assertTrue("IndexOutOfBoundsException was expected but was not thrown.", thrown);
+    }
+  }
+}

java/test/com/google/typography/font/sfntly/testutils/TestFont.java

@@ -33,7 +33,8 @@ public class TestFont {
 
   public enum TestFontNames {
     DROIDSANS("DroidSans-Regular.ttf"),
-    OPENSANS("OpenSans-Regular.ttf");
+    OPENSANS("OpenSans-Regular.ttf"),
+    ROBOTO("Roboto-Regular.ttf");
 
     private String path;