Skip to content

Commit

Permalink
Merge pull request #879 from mapnik/fix-svg-parsing-strictness-master
Browse files Browse the repository at this point in the history
[master] Fix behavior of SVG parsing
  • Loading branch information
springmeyer committed Jun 14, 2018
2 parents 39aea2d + 498d55a commit 66d2c11
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 11 deletions.
29 changes: 20 additions & 9 deletions src/mapnik_image.cpp
Expand Up @@ -2532,6 +2532,9 @@ void Image::EIO_AfterOpen(uv_work_t* req)
* @static
* @param {string} path - path to SVG image
* @param {Object} [options]
* @param {number} [options.strict] - Throw on unhandled elements and invalid values in SVG. The default is `false`, which means that
* unhandled elements will be ignored and invalid values will be set to defaults (and may not render correctly). If `true` then all occurances
* of invalid value or unhandled elements will be collected and an error will be thrown reporting them all.
* @param {number} [options.scale] - scale the image. For example passing `0.5` as scale would render
* your SVG at 50% the original size.
* @param {number} [options.max_size] - the maximum allowed size of the svg dimensions * scale. The default is 2048.
Expand All @@ -2553,6 +2556,9 @@ NAN_METHOD(Image::fromSVGBytesSync)
* @name fromSVGSync
* @param {string} filename
* @param {Object} [options]
* @param {number} [options.strict] - Throw on unhandled elements and invalid values in SVG. The default is `false`, which means that
* unhandled elements will be ignored and invalid values will be set to defaults (and may not render correctly). If `true` then all occurances
* of invalid value or unhandled elements will be collected and an error will be thrown reporting them all.
* @param {number} [options.scale] - scale the image. For example passing `0.5` as scale would render
* your SVG at 50% the original size.
* @param {number} [options.max_size] - the maximum allowed size of the svg dimensions * scale. The default is 2048.
Expand Down Expand Up @@ -2646,11 +2652,11 @@ v8::Local<v8::Value> Image::_fromSVGSync(bool fromFile, Nan::NAN_METHOD_ARGS_TYP
vertex_stl_adapter<svg_path_storage> stl_storage(marker_path->source());
svg_path_adapter svg_path(stl_storage);
svg_converter_type svg(svg_path, marker_path->attributes());
svg_parser p(svg, strict);
svg_parser p(svg);
if (fromFile)
{
p.parse(TOSTR(info[0]));
if (!p.err_handler().error_messages().empty())
if (strict && !p.err_handler().error_messages().empty())
{
std::ostringstream errorMessage;
errorMessage << "SVG parse error:" << std::endl;
Expand All @@ -2671,7 +2677,7 @@ v8::Local<v8::Value> Image::_fromSVGSync(bool fromFile, Nan::NAN_METHOD_ARGS_TYP
}
std::string svg_buffer(node::Buffer::Data(obj),node::Buffer::Length(obj));
p.parse_from_string(svg_buffer);
if (!p.err_handler().error_messages().empty())
if (strict && !p.err_handler().error_messages().empty())
{
std::ostringstream errorMessage;
errorMessage << "SVG parse error:" << std::endl;
Expand Down Expand Up @@ -2786,6 +2792,9 @@ typedef struct {
* @name fromSVG
* @param {string} filename
* @param {Object} [options]
* @param {number} [options.strict] - Throw on unhandled elements and invalid values in SVG. The default is `false`, which means that
* unhandled elements will be ignored and invalid values will be set to defaults (and may not render correctly). If `true` then all occurances
* of invalid value or unhandled elements will be collected and an error will be thrown reporting them all.
* @param {number} [options.scale] - scale the image. For example passing `0.5` as scale would render
* your SVG at 50% the original size.
* @param {number} [options.max_size] - the maximum allowed size of the svg dimensions * scale. The default is 2048.
Expand Down Expand Up @@ -2896,9 +2905,9 @@ void Image::EIO_FromSVG(uv_work_t* req)
vertex_stl_adapter<svg_path_storage> stl_storage(marker_path->source());
svg_path_adapter svg_path(stl_storage);
svg_converter_type svg(svg_path, marker_path->attributes());
svg_parser p(svg, closure->strict);
svg_parser p(svg);
p.parse(closure->filename);
if (!p.err_handler().error_messages().empty())
if (closure->strict && !p.err_handler().error_messages().empty())
{
std::ostringstream errorMessage;
errorMessage << "SVG parse error:" << std::endl;
Expand Down Expand Up @@ -3007,10 +3016,12 @@ void Image::EIO_AfterFromSVG(uv_work_t* req)
* @static
* @param {string} path - path to SVG image
* @param {Object} [options]
* @param {number} [options.strict] - Throw on unhandled elements and invalid values in SVG. The default is `false`, which means that
* unhandled elements will be ignored and invalid values will be set to defaults (and may not render correctly). If `true` then all occurances
* of invalid value or unhandled elements will be collected and an error will be thrown reporting them all.
* @param {number} [options.scale] - scale the image. For example passing `0.5` as scale would render
* your SVG at 50% the original size.
* @param {number} [options.max_size] - the maximum allowed size of the svg dimensions * scale. The default is 2048.
* @param {boolean} [options.strict] - enable `strict` parsing mode e.g throw on unsupported element/attribute. The default is `false`.
* This option can be passed a smaller or larger size in order to control the final size of the image allocated for
* rasterizing the SVG.
* @param {Function} callback = `function(err, img)`
Expand Down Expand Up @@ -3048,7 +3059,7 @@ NAN_METHOD(Image::fromSVGBytes)

double scale = 1.0;
std::uint32_t max_size = 2048;
bool strict = true;
bool strict = false;
if (info.Length() >= 3)
{
if (!info[1]->IsObject())
Expand Down Expand Up @@ -3124,11 +3135,11 @@ void Image::EIO_FromSVGBytes(uv_work_t* req)
vertex_stl_adapter<svg_path_storage> stl_storage(marker_path->source());
svg_path_adapter svg_path(stl_storage);
svg_converter_type svg(svg_path, marker_path->attributes());
svg_parser p(svg, closure->strict);
svg_parser p(svg);

std::string svg_buffer(closure->data,closure->dataLength);
p.parse_from_string(svg_buffer);
if (!p.err_handler().error_messages().empty())
if (closure->strict && !p.err_handler().error_messages().empty())
{
std::ostringstream errorMessage;
errorMessage << "SVG parse error:" << std::endl;
Expand Down
46 changes: 44 additions & 2 deletions test/image.svg.test.js
Expand Up @@ -179,15 +179,57 @@ describe('mapnik.Image SVG', function() {
});
});

it('should error with async file full of errors', function(done) {
mapnik.Image.fromSVG('./test/data/vector_tile/errors.svg', function(err, svg) {
it('should not error with async in non-strict mode on svg with validation and parse errors', function(done) {
mapnik.Image.fromSVG('./test/data/vector_tile/errors.svg', {strict:false}, function(err, svg) {
assert.ok(!err);
assert.ok(svg);
done();
});
});

it('should error with async in strict mode on svg with validation and parse errors', function(done) {
mapnik.Image.fromSVG('./test/data/vector_tile/errors.svg', {strict:true}, function(err, svg) {
assert.ok(err);
assert.ok(err.message.match(/SVG parse error:/));
console.log(err.message)
assert.equal(svg, undefined);
done();
});
});

it('should not error on svg with unhandled elements in non-strict mode', function() {
var svgdata = "<svg width='100' height='100'><g id='a'><text></text><ellipse fill='#FFFFFF' stroke='#000000' stroke-width='4' cx='50' cy='50' rx='25' ry='25'/></g></svg>";
var buffer = new Buffer(svgdata);
var img = mapnik.Image.fromSVGBytesSync(buffer,{strict:false});
assert.ok(img);
assert.ok(img instanceof mapnik.Image);
assert.equal(img.width(), 100);
assert.equal(img.height(), 100);
assert.equal(img.encodeSync("png").length, 1270);
});


// test the mapnik core known unsupported elements: https://github.com/mapnik/mapnik/blob/634928fcbe780e8a5a355ddb3cd075ce2450adb4/src/svg/svg_parser.cpp#L106-L114
it('should error on svg with unhandled elements in strict mode', function() {
var svgdata = "<svg width='100' height='100'><g id='a'><unsupported></unsupported><text></text><symbol></symbol><image></image><marker></marker><view></view><switch></switch><a></a><ellipse fill='#FFFFFF' stroke='#000000' stroke-width='4' cx='50' cy='50' rx='25' ry='25'/></g></svg>";
var buffer = new Buffer(svgdata);
var error = false;
try {
var img = mapnik.Image.fromSVGBytesSync(buffer,{strict:true});
assert.ok(!img);
} catch (err) {
error = err;
}
assert.ok(error);
assert.ok(error.message.match(/<text> element is not supported/));
assert.ok(error.message.match(/<switch> element is not supported/));
assert.ok(error.message.match(/<symbol> element is not supported/));
assert.ok(error.message.match(/<marker> element is not supported/));
assert.ok(error.message.match(/<view> element is not supported/));
assert.ok(error.message.match(/<a> element is not supported/));
assert.ok(error.message.match(/<image> element is not supported/));
});

it('#fromSVGSync load from SVG file', function() {
var img = mapnik.Image.fromSVGSync('./test/data/vector_tile/tile0.expected-svg.svg');
assert.ok(img);
Expand Down

0 comments on commit 66d2c11

Please sign in to comment.