[core] harden Glyph PBF parsing

This commit is contained in:
Konstantin Käfer
2017-01-13 12:02:04 +01:00
parent 4439aecbcc
commit b6c1119172
8 changed files with 230 additions and 12 deletions

View File

@@ -88,6 +88,7 @@ set(MBGL_TEST_FILES
# text
test/text/glyph_atlas.test.cpp
test/text/glyph_pbf.test.cpp
test/text/quads.test.cpp
# tile

View File

@@ -77,9 +77,13 @@ class Shaping {
class SDFGlyph {
public:
// We're using this value throughout the Mapbox GL ecosystem. If this is different, the glyphs
// also need to be reencoded.
static constexpr const uint8_t borderSize = 3;
uint32_t id = 0;
// A signed distance field of the glyph with a border of 3 pixels.
// A signed distance field of the glyph with a border (see above).
std::string bitmap;
// Glyph metrics

View File

@@ -107,9 +107,6 @@ Rect<uint16_t> GlyphAtlas::addGlyph(uintptr_t tileUID,
const FontStack& fontStack,
const SDFGlyph& glyph)
{
// Use constant value for now.
const uint8_t buffer = 3;
std::map<uint32_t, GlyphValue>& face = index[fontStack];
auto it = face.find(glyph.id);
@@ -125,8 +122,8 @@ Rect<uint16_t> GlyphAtlas::addGlyph(uintptr_t tileUID,
return Rect<uint16_t>{ 0, 0, 0, 0 };
}
uint16_t buffered_width = glyph.metrics.width + buffer * 2;
uint16_t buffered_height = glyph.metrics.height + buffer * 2;
uint16_t buffered_width = glyph.metrics.width + SDFGlyph::borderSize * 2;
uint16_t buffered_height = glyph.metrics.height + SDFGlyph::borderSize * 2;
// Add a 1px border around every image.
const uint16_t padding = 1;

View File

@@ -12,9 +12,11 @@
#include <protozero/pbf_reader.hpp>
namespace mbgl {
namespace {
void parseGlyphPBF(mbgl::GlyphSet& glyphSet, const std::string& data) {
void parseGlyphPBF(GlyphSet& glyphSet, const GlyphRange& glyphRange, const std::string& data) {
protozero::pbf_reader glyphs_pbf(data);
while (glyphs_pbf.next(1)) {
@@ -22,30 +24,39 @@ void parseGlyphPBF(mbgl::GlyphSet& glyphSet, const std::string& data) {
while (fontstack_pbf.next(3)) {
auto glyph_pbf = fontstack_pbf.get_message();
mbgl::SDFGlyph glyph;
SDFGlyph glyph;
bool hasID = false, hasWidth = false, hasHeight = false, hasLeft = false,
hasTop = false, hasAdvance = false;
while (glyph_pbf.next()) {
switch (glyph_pbf.tag()) {
case 1: // id
glyph.id = glyph_pbf.get_uint32();
hasID = true;
break;
case 2: // bitmap
glyph.bitmap = glyph_pbf.get_string();
break;
case 3: // width
glyph.metrics.width = glyph_pbf.get_uint32();
hasWidth = true;
break;
case 4: // height
glyph.metrics.height = glyph_pbf.get_uint32();
hasHeight = true;
break;
case 5: // left
glyph.metrics.left = glyph_pbf.get_sint32();
hasLeft = true;
break;
case 6: // top
glyph.metrics.top = glyph_pbf.get_sint32();
hasTop = true;
break;
case 7: // advance
glyph.metrics.advance = glyph_pbf.get_uint32();
hasAdvance = true;
break;
default:
glyph_pbf.skip();
@@ -53,15 +64,32 @@ void parseGlyphPBF(mbgl::GlyphSet& glyphSet, const std::string& data) {
}
}
glyphSet.insert(glyph.id, std::move(glyph));
// If the area of width/height is non-zero, we need to adjust the expected size
// with the implicit border size, otherwise we expect there to be no bitmap at all.
const uint32_t expectedBitmapSize =
glyph.metrics.width && glyph.metrics.height
? (glyph.metrics.width + 2 * SDFGlyph::borderSize) *
(glyph.metrics.height + 2 * SDFGlyph::borderSize)
: 0;
// Only treat this glyph as a correct glyph if it has all required fields, and if
// the bitmap has the correct length. It also needs to satisfy a few metrics conditions
// that ensure that the glyph isn't bogus. All other glyphs are malformed.
// We're also discarding all glyphs that are outside the expected glyph range.
if (hasID && hasWidth && hasHeight && hasLeft && hasTop && hasAdvance &&
glyph.metrics.width < 256 && glyph.metrics.height < 256 &&
glyph.metrics.left >= -128 && glyph.metrics.left < 128 &&
glyph.metrics.top >= -128 && glyph.metrics.top < 128 &&
glyph.metrics.advance < 256 && glyph.bitmap.size() == expectedBitmapSize &&
glyph.id >= glyphRange.first && glyph.id <= glyphRange.second) {
glyphSet.insert(glyph.id, std::move(glyph));
}
}
}
}
} // namespace
namespace mbgl {
GlyphPBF::GlyphPBF(GlyphAtlas* atlas,
const FontStack& fontStack,
const GlyphRange& glyphRange,
@@ -79,7 +107,7 @@ GlyphPBF::GlyphPBF(GlyphAtlas* atlas,
observer->onGlyphsLoaded(fontStack, glyphRange);
} else {
try {
parseGlyphPBF(**atlas->getGlyphSet(fontStack), *res.data);
parseGlyphPBF(**atlas->getGlyphSet(fontStack), glyphRange, *res.data);
} catch (...) {
observer->onGlyphsError(fontStack, glyphRange, std::current_exception());
return;

Binary file not shown.

View File

@@ -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
}
}

View File

@@ -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;
}

View File

@@ -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();
}