[core] harden Glyph PBF parsing

4 files changed, 188 insertions, 0 deletions

diff --git a/test/fixtures/resources/fake_glyphs-0-255.pbf b/test/fixtures/resources/fake_glyphs-0-255.pbf
new file mode 100644
index 0000000000..8d7445d5c2
--- /dev/null
+++ b/test/fixtures/resources/fake_glyphs-0-255.pbf
diff --git a/test/fixtures/resources/fake_glyphs-0-255.pbf.txt b/test/fixtures/resources/fake_glyphs-0-255.pbf.txt
new file mode 100644
index 0000000000..6433af5d3d
--- /dev/null
+++ b/test/fixtures/resources/fake_glyphs-0-255.pbf.txt
@@ -0,0 +1,86 @@
+# Make changes below, then run this command to update the encoded PBF file:
+# protoc --encode glyphs test/fixtures/resources/fake_glyphs.proto < test/fixtures/resources/fake_glyphs-0-255.pbf.txt > test/fixtures/resources/fake_glyphs-0-255.pbf
+
+stacks {
+  # testing invalid metrics and bitmap too short
+  glyphs {
+    id: 65
+    bitmap: "fake"
+    width: -1
+    height: 65536
+    left: -10000000
+    top: 10000000
+    advance: -42
+  }
+
+  # testing missing bitmap => malformed
+  glyphs {
+    id: 66
+    width: 12
+    height: 17
+    left: 2
+    top: -11
+    advance: 15
+  }
+
+  # testing bitmap too long => malformed
+  glyphs {
+    id: 67
+    bitmap: "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+    width: 1
+    height: 1
+  }
+
+  # testing missing ID => malformed
+  glyphs {
+    width: 12
+    height: 17
+    left: 2
+    top: -11
+    advance: 15
+  }
+
+  # invalid metrics + bitmap => malformed
+  glyphs {
+    id: 65
+    bitmap: "fake"
+    width: -1
+    height: 65536
+    left: -10000000
+    top: 10000000
+    advance: -42
+  }
+
+  # invalid metrics => malformed
+  glyphs {
+    id: 68
+    bitmap: "x"
+    width: 1
+    height: 1
+    left: -10000000
+    top: 10000000
+    advance: -42
+  }
+
+  # a valid glyph
+  glyphs {
+    id: 69
+    bitmap: "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+    width: 1
+    height: 1
+    left: 20
+    top: 2
+    advance: 8
+  }
+
+  # a valid glyph that has an ID outside the range
+  glyphs {
+    id: 300
+    bitmap: ""
+    width: 0
+    height: 0
+    left: 0
+    top: 0
+    advance: 0
+  }
+}
diff --git a/test/fixtures/resources/fake_glyphs.proto b/test/fixtures/resources/fake_glyphs.proto
new file mode 100644
index 0000000000..d074541d69
--- /dev/null
+++ b/test/fixtures/resources/fake_glyphs.proto
@@ -0,0 +1,31 @@
+syntax = "proto2";
+
+option optimize_for = LITE_RUNTIME;
+
+// Stores a glyph with metrics and optional SDF bitmap information.
+message glyph {
+    optional int64 id = 1;
+
+    // A signed distance field of the glyph with a border of 3 pixels.
+    optional bytes bitmap = 2;
+
+    // Glyph metrics.
+    optional int64 width = 3;
+    optional int64 height = 4;
+    optional sint64 left = 5;
+    optional sint64 top = 6;
+    optional int64 advance = 7;
+}
+
+// Stores fontstack information and a list of faces.
+message fontstack {
+    optional string name = 1;
+    optional string range = 2;
+    repeated glyph glyphs = 3;
+}
+
+message glyphs {
+    repeated fontstack stacks = 1;
+
+    extensions 16 to 8191;
+}
diff --git a/test/text/glyph_pbf.test.cpp b/test/text/glyph_pbf.test.cpp
new file mode 100644
index 0000000000..1e28dfbc31
--- /dev/null
+++ b/test/text/glyph_pbf.test.cpp
@@ -0,0 +1,71 @@
+#include <mbgl/test/util.hpp>
+
+#include <mbgl/text/glyph_set.hpp>
+#include <mbgl/text/glyph_atlas_observer.hpp>
+#include <mbgl/text/glyph_atlas.hpp>
+#include <mbgl/text/glyph_pbf.hpp>
+#include <mbgl/storage/default_file_source.hpp>
+#include <mbgl/util/run_loop.hpp>
+#include <mbgl/util/string.hpp>
+
+#include <future>
+
+using namespace mbgl;
+
+using namespace std::string_literals;
+
+class MockGlyphAtlasObserver : public GlyphAtlasObserver {
+public:
+    std::function<void(const FontStack&, const GlyphRange&)> glyphsLoaded;
+    std::function<void(const FontStack&, const GlyphRange&, std::exception_ptr)> glyphsError;
+
+    void onGlyphsLoaded(const FontStack& fontStack, const GlyphRange& glyphRange) override {
+        if (glyphsLoaded) {
+            glyphsLoaded(fontStack, glyphRange);
+        }
+    }
+    void onGlyphsError(const FontStack& fontStack, const GlyphRange& glyphRange, std::exception_ptr error) override {
+        if (glyphsError) {
+            glyphsError(fontStack, glyphRange, error);
+        }
+    }
+};
+
+TEST(GlyphPBF, Parsing) {
+    util::RunLoop loop;
+    DefaultFileSource fileSource{ ":memory:", "." };
+    GlyphAtlas glyphAtlas{ { 1024, 1024 }, fileSource };
+    FontStack fontStack{ "fake_glyphs" };
+    GlyphRange glyphRange{ 0, 255 };
+
+    glyphAtlas.setURL("asset://test/fixtures/resources/{fontstack}-{range}.pbf");
+
+    MockGlyphAtlasObserver glyphAtlasObserver;
+    glyphAtlasObserver.glyphsLoaded = [&](const FontStack&, const GlyphRange&) {
+        loop.stop();
+
+        auto sdfs = glyphAtlas.getGlyphSet(fontStack)->getSDFs();
+
+        // The fake glyphs don't contain a glyph that has the ID 0; it only contains glyphs with
+        // undefined IDs, but the parser should remove them.
+
+        EXPECT_TRUE(sdfs.size() == 1);
+        EXPECT_TRUE(sdfs.find(69) != sdfs.end());
+        auto& sdf = sdfs[69];
+        EXPECT_EQ("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"s, sdf.bitmap);
+        EXPECT_EQ(1u, sdf.metrics.width);
+        EXPECT_EQ(1u, sdf.metrics.height);
+        EXPECT_EQ(20, sdf.metrics.left);
+        EXPECT_EQ(2, sdf.metrics.top);
+        EXPECT_EQ(8u, sdf.metrics.advance);
+    };
+
+    glyphAtlasObserver.glyphsError = [&](const FontStack&, const GlyphRange&, std::exception_ptr error) {
+        loop.stop();
+        FAIL() << util::toString(error);
+    };
+
+    GlyphPBF pbf(&glyphAtlas, fontStack, glyphRange, &glyphAtlasObserver, fileSource);
+
+    loop.run();
+}