From 8cc7060ef1d8d3279600b82043ed41a670050fd7 Mon Sep 17 00:00:00 2001 From: artemp Date: Fri, 3 Feb 2017 17:20:25 +0100 Subject: [PATCH 1/3] remove bounding box validity check (ref #3611) --- include/mapnik/json/extract_bounding_box_grammar_impl.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/mapnik/json/extract_bounding_box_grammar_impl.hpp b/include/mapnik/json/extract_bounding_box_grammar_impl.hpp index bdc1b98b2..6be94a954 100644 --- a/include/mapnik/json/extract_bounding_box_grammar_impl.hpp +++ b/include/mapnik/json/extract_bounding_box_grammar_impl.hpp @@ -64,10 +64,10 @@ struct push_box_impl template void operator() (T0 & boxes, T1 const& begin, T2 const& box, T3 const& range) const { - if (box.valid()) boxes.emplace_back(box, - std::make_pair(std::distance(begin, - range.begin()), - std::distance(range.begin(), range.end()))); + boxes.emplace_back(box, + std::make_pair(std::distance(begin, + range.begin()), + std::distance(range.begin(), range.end()))); } }; From d2d62bc95cea28b9f43469f4bb29a3cc8b63c577 Mon Sep 17 00:00:00 2001 From: artemp Date: Fri, 3 Feb 2017 17:21:05 +0100 Subject: [PATCH 2/3] Return failure on invalid bounding box when `--validate-features` option is used. (ref #3611) --- utils/mapnik-index/process_geojson_file.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/utils/mapnik-index/process_geojson_file.cpp b/utils/mapnik-index/process_geojson_file.cpp index 44553185f..c00c55494 100644 --- a/utils/mapnik-index/process_geojson_file.cpp +++ b/utils/mapnik-index/process_geojson_file.cpp @@ -137,6 +137,11 @@ std::pair process_geojson_file(T & boxe } } } + else if (validate_features) + { + if (verbose) std::clog << "Invalid bbox encountered " << item.first << std::endl; + return std::make_pair(false, extent); + } } return std::make_pair(true, extent); } From 74e66bac585864e3a43a48023decfeb0b390bdcc Mon Sep 17 00:00:00 2001 From: artemp Date: Fri, 3 Feb 2017 17:36:28 +0100 Subject: [PATCH 3/3] test - update malformed featurecollection test (ref #3611) --- test/data | 2 +- test/unit/datasource/geojson.cpp | 68 +++++++++++++++++--------------- 2 files changed, 37 insertions(+), 33 deletions(-) diff --git a/test/data b/test/data index 5ad14e6bd..4035e3453 160000 --- a/test/data +++ b/test/data @@ -1 +1 @@ -Subproject commit 5ad14e6bdf2c5e6babd8ac04aa057ed6c67ac617 +Subproject commit 4035e34534d6f0ad7060fc4a2f5f6018c61d7a1b diff --git a/test/unit/datasource/geojson.cpp b/test/unit/datasource/geojson.cpp index b03668436..337f8098b 100644 --- a/test/unit/datasource/geojson.cpp +++ b/test/unit/datasource/geojson.cpp @@ -514,7 +514,8 @@ TEST_CASE("geojson") { for (auto const& c_str : {"./test/data/json/feature-malformed-1.geojson", "./test/data/json/feature-malformed-2.geojson", - "./test/data/json/feature-malformed-3.geojson"}) + "./test/data/json/feature-malformed-3.geojson", + "./test/data/json/feature-malformed-4.geojson"}) { std::string filename(c_str); params["file"] = filename; @@ -554,43 +555,46 @@ TEST_CASE("geojson") { SECTION("GeoJSON ensure mapnik::featureset::next() throws on malformed input") { - std::string filename{"./test/data/json/featurecollection-malformed.json"}; mapnik::parameters params; params["type"] = "geojson"; - params["file"] = filename; - - // cleanup in the case of a failed previous run - if (mapnik::util::exists(filename + ".index")) + for (auto const& c_str : {"./test/data/json/featurecollection-malformed.json", + "./test/data/json/featurecollection-malformed-2.json"}) { - mapnik::util::remove(filename + ".index"); - } + std::string filename(c_str); + params["file"] = filename; + // cleanup in the case of a failed previous run + if (mapnik::util::exists(filename + ".index")) + { + mapnik::util::remove(filename + ".index"); + } - CHECK(!mapnik::util::exists(filename + ".index")); - int ret = create_disk_index(filename); - int ret_posix = (ret >> 8) & 0x000000ff; - INFO(ret); - INFO(ret_posix); - CHECK(mapnik::util::exists(filename + ".index")); + CHECK(!mapnik::util::exists(filename + ".index")); + int ret = create_disk_index(filename); + int ret_posix = (ret >> 8) & 0x000000ff; + INFO(ret); + INFO(ret_posix); + CHECK(mapnik::util::exists(filename + ".index")); - for (auto cache_features : {true,false}) - { - params["cache_features"] = cache_features; - auto ds = mapnik::datasource_cache::instance().create(params); - auto fields = ds->get_descriptor().get_descriptors(); - mapnik::query query(ds->envelope()); - auto features = ds->features(query); - REQUIRE_THROWS( - auto feature = features->next(); - while (feature != nullptr) - { - feature = features->next(); - }); - } + for (auto cache_features : {true,false}) + { + params["cache_features"] = cache_features; + auto ds = mapnik::datasource_cache::instance().create(params); + auto fields = ds->get_descriptor().get_descriptors(); + mapnik::query query(ds->envelope()); + auto features = ds->features(query); + REQUIRE_THROWS( + auto feature = features->next(); + while (feature != nullptr) + { + feature = features->next(); + }); + } - // cleanup - if (mapnik::util::exists(filename + ".index")) - { - mapnik::util::remove(filename + ".index"); + // cleanup + if (mapnik::util::exists(filename + ".index")) + { + mapnik::util::remove(filename + ".index"); + } } }