summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorKonstantin Käfer <mail@kkaefer.com>2017-01-13 12:02:04 +0100
committerKonstantin Käfer <mail@kkaefer.com>2017-01-17 19:23:20 +0100
commitb6c11191723d6eb884de5ee17d658298f5dd4127 (patch)
tree2de2bcc074b47563a8dcbe2039e3384faf32ddf9 /src
parent4439aecbcc21aae8b71c9b9a0bd65d9d128d6c83 (diff)
downloadqtlocation-mapboxgl-b6c11191723d6eb884de5ee17d658298f5dd4127.tar.gz
[core] harden Glyph PBF parsing
Diffstat (limited to 'src')
-rw-r--r--src/mbgl/text/glyph.hpp6
-rw-r--r--src/mbgl/text/glyph_atlas.cpp7
-rw-r--r--src/mbgl/text/glyph_pbf.cpp40
3 files changed, 41 insertions, 12 deletions
diff --git a/src/mbgl/text/glyph.hpp b/src/mbgl/text/glyph.hpp
index d07fbdff21..2bf1448492 100644
--- a/src/mbgl/text/glyph.hpp
+++ b/src/mbgl/text/glyph.hpp
@@ -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
diff --git a/src/mbgl/text/glyph_atlas.cpp b/src/mbgl/text/glyph_atlas.cpp
index 5d30dacdce..17b3e7e482 100644
--- a/src/mbgl/text/glyph_atlas.cpp
+++ b/src/mbgl/text/glyph_atlas.cpp
@@ -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;
diff --git a/src/mbgl/text/glyph_pbf.cpp b/src/mbgl/text/glyph_pbf.cpp
index 21edfe436c..cdeac57984 100644
--- a/src/mbgl/text/glyph_pbf.cpp
+++ b/src/mbgl/text/glyph_pbf.cpp
@@ -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;