add warning for integer overflow in the SymbolInstancesArray (#2966)

* add warning for integer overflow in the SymbolInstancesArray add warning for integer overflow in the SymbolInstancesArray * add MAX_QUADS constant * add integer overflow test * change MAX_QUADS from a prototype property to a constant property, use sinon.spy for testing the warning * use sinon.stub to supress warning output to console * reference SymbolBucket.MAX_QUADS instead of this.MAX_QUADS
mapbox · Aug 16, 2016 · 8a8460c · 8a8460c
1 parent 527213c
commit 8a8460c
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 35 deletions.
diff --git a/js/data/bucket/symbol_bucket.js b/js/data/bucket/symbol_bucket.js
@@ -39,6 +39,11 @@ function SymbolBucket(options) {
     this.fontstack = options.fontstack;
 }
 
+// this constant is based on the size of the glyphQuadEndIndex and iconQuadEndIndex
+// in the symbol_instances StructArrayType
+// eg the max valid UInt16 is 65,535
+SymbolBucket.MAX_QUADS = 65535;
+
 SymbolBucket.prototype = util.inherit(Bucket, {});
 
 SymbolBucket.prototype.serialize = function() {
@@ -594,6 +599,8 @@ SymbolBucket.prototype.addSymbolInstance = function(anchor, line, shapedText, sh
 
     var iconBoxStartIndex = iconCollisionFeature ? iconCollisionFeature.boxStartIndex : this.collisionBoxArray.length;
     var iconBoxEndIndex = iconCollisionFeature ? iconCollisionFeature.boxEndIndex : this.collisionBoxArray.length;
+    if (iconQuadEndIndex > SymbolBucket.MAX_QUADS) util.warnOnce("Too many symbols being rendered in a tile. See https://github.com/mapbox/mapbox-gl-js/issues/2907");
+    if (glyphQuadEndIndex > SymbolBucket.MAX_QUADS) util.warnOnce("Too many glyphs being rendered in a tile. See https://github.com/mapbox/mapbox-gl-js/issues/2907");
 
     return this.symbolInstancesArray.emplaceBack(
         textBoxStartIndex,

diff --git a/test/js/data/symbol_bucket.test.js b/test/js/data/symbol_bucket.test.js
@@ -1,6 +1,7 @@
 'use strict';
 
 var test = require('tap').test;
+var sinon = require('sinon');
 var fs = require('fs');
 var path = require('path');
 var Protobuf = require('pbf');
@@ -12,51 +13,53 @@ var SymbolInstancesArray = require('../../../js/symbol/symbol_instances');
 var SymbolQuadsArray = require('../../../js/symbol/symbol_quads');
 var GlyphAtlas = require('../../../js/symbol/glyph_atlas');
 var StyleLayer = require('../../../js/style/style_layer');
+var util = require('../../../js/util/util');
 
 // Load a point feature from fixture tile.
 var vt = new VectorTile(new Protobuf(new Uint8Array(fs.readFileSync(path.join(__dirname, '/../../fixtures/mbsv5-6-18-23.vector.pbf')))));
 var feature = vt.layers.place_label.feature(10);
 var glyphs = JSON.parse(fs.readFileSync(path.join(__dirname, '/../../fixtures/fontstack-glyphs.json')));
 
-test('SymbolBucket', function(t) {
-    /*eslint new-cap: 0*/
-    var buffers = {};
-    var collisionBoxArray = new CollisionBoxArray();
-    var symbolQuadsArray = new SymbolQuadsArray();
-    var symbolInstancesArray = new SymbolInstancesArray();
-    var collision = new Collision(0, 0, collisionBoxArray);
-    var atlas = new GlyphAtlas();
-    for (var id in glyphs) {
-        glyphs[id].bitmap = true;
-        glyphs[id].rect = atlas.addGlyph(id, 'Test', glyphs[id], 3);
-    }
+/*eslint new-cap: 0*/
+var buffers = {};
+var collisionBoxArray = new CollisionBoxArray();
+var symbolQuadsArray = new SymbolQuadsArray();
+var symbolInstancesArray = new SymbolInstancesArray();
+var collision = new Collision(0, 0, collisionBoxArray);
+var atlas = new GlyphAtlas();
+for (var id in glyphs) {
+    glyphs[id].bitmap = true;
+    glyphs[id].rect = atlas.addGlyph(id, 'Test', glyphs[id], 3);
+}
+
+var stacks = { 'Test': glyphs };
 
-    var stacks = { 'Test': glyphs };
+function bucketSetup() {
+    var layer = new StyleLayer({
+        id: 'test',
+        type: 'symbol',
+        layout: { 'text-font': ['Test'] }
+    });
 
-    function bucketSetup() {
-        var layer = new StyleLayer({
-            id: 'test',
-            type: 'symbol',
-            layout: { 'text-font': ['Test'] }
-        });
+    var bucket = new SymbolBucket({
+        buffers: buffers,
+        overscaling: 1,
+        zoom: 0,
+        collisionBoxArray: collisionBoxArray,
+        symbolInstancesArray: symbolInstancesArray,
+        symbolQuadsArray: symbolQuadsArray,
+        layer: layer,
+        childLayers: [layer],
+        tileExtent: 4096
+    });
+    bucket.createArrays();
+    bucket.textFeatures = ['abcde'];
+    bucket.features = [feature];
+    return bucket;
+}
 
-        var bucket = new SymbolBucket({
-            buffers: buffers,
-            overscaling: 1,
-            zoom: 0,
-            collisionBoxArray: collisionBoxArray,
-            symbolInstancesArray: symbolInstancesArray,
-            symbolQuadsArray: symbolQuadsArray,
-            layer: layer,
-            childLayers: [layer],
-            tileExtent: 4096
-        });
-        bucket.createArrays();
-        bucket.textFeatures = ['abcde'];
-        bucket.features = [feature];
-        return bucket;
-    }
 
+test('SymbolBucket', function(t) {
     var bucketA = bucketSetup();
     var bucketB = bucketSetup();
 
@@ -71,6 +74,26 @@ test('SymbolBucket', function(t) {
     t.equal(bucketB.populateArrays(collision, stacks), undefined);
     var b2 = collision.grid.keys.length;
     t.equal(a2, b2, 'detects collision and does not place feature');
+    t.end();
+});
 
+
+test('SymbolBucket integer overflow', function(t) {
+    var bucket = bucketSetup();
+    var numWarnings = 0;
+    sinon.stub(util, 'warnOnce', function(warning) {
+        if (warning.includes('Too many symbols being rendered in a tile.') || warning.includes('Too many glyphs being rendered in a tile.')) numWarnings++;
+    });
+    // save correct value of MAX_QUADS
+    var maxquads = SymbolBucket.MAX_QUADS;
+
+    // reduce MAX_QUADS to test warning
+    SymbolBucket.MAX_QUADS = 5;
+    bucket.populateArrays(collision, stacks);
+    t.equal(numWarnings, 2);
+
+    // reset MAX_QUADS to its original value
+    SymbolBucket.MAX_QUADS = maxquads;
     t.end();
 });
+