Merge pull request #1632 from mapbox/1632-handle_currupt_data
Corrupt data not handled properly
This commit is contained in:
@@ -287,26 +287,25 @@ TileData::State Source::addTile(MapData& data,
|
||||
new_tile.data = cache.get(normalized_id.to_uint64());
|
||||
}
|
||||
|
||||
auto successCallback = std::bind(&Source::emitTileLoaded, this, true);
|
||||
auto failureCallback = std::bind(&Source::emitTileLoadingFailed, this, std::placeholders::_1);
|
||||
|
||||
if (!new_tile.data) {
|
||||
auto callback = std::bind(&Source::tileLoadingCompleteCallback, this, normalized_id);
|
||||
|
||||
// If we don't find working tile data, we're just going to load it.
|
||||
if (info.type == SourceType::Vector) {
|
||||
new_tile.data =
|
||||
std::make_shared<VectorTileData>(normalized_id, data.transform.getMaxZoom(), style, glyphAtlas,
|
||||
glyphStore, spriteAtlas, sprite, info);
|
||||
new_tile.data->request(
|
||||
style.workers, transformState.getPixelRatio(), successCallback, failureCallback);
|
||||
style.workers, transformState.getPixelRatio(), callback);
|
||||
} else if (info.type == SourceType::Raster) {
|
||||
new_tile.data = std::make_shared<RasterTileData>(normalized_id, texturePool, info);
|
||||
new_tile.data->request(
|
||||
style.workers, transformState.getPixelRatio(), successCallback, failureCallback);
|
||||
style.workers, transformState.getPixelRatio(), callback);
|
||||
} else if (info.type == SourceType::Annotations) {
|
||||
new_tile.data = std::make_shared<LiveTileData>(normalized_id, data.annotationManager,
|
||||
data.transform.getMaxZoom(), style, glyphAtlas,
|
||||
glyphStore, spriteAtlas, sprite, info);
|
||||
new_tile.data->reparse(style.workers, successCallback);
|
||||
new_tile.data->reparse(style.workers, callback);
|
||||
} else {
|
||||
throw std::runtime_error("source type not implemented");
|
||||
}
|
||||
@@ -539,6 +538,25 @@ void Source::setObserver(Observer* observer) {
|
||||
observer_ = observer;
|
||||
}
|
||||
|
||||
void Source::tileLoadingCompleteCallback(const TileID& normalized_id) {
|
||||
auto it = tile_data.find(normalized_id);
|
||||
if (it == tile_data.end()) {
|
||||
return;
|
||||
}
|
||||
|
||||
util::ptr<TileData> data = it->second.lock();
|
||||
if (!data) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (data->getState() == TileData::State::obsolete && !data->getError().empty()) {
|
||||
emitTileLoadingFailed(data->getError());
|
||||
return;
|
||||
}
|
||||
|
||||
emitTileLoaded(true);
|
||||
}
|
||||
|
||||
void Source::emitSourceLoaded() {
|
||||
if (observer_) {
|
||||
observer_->onSourceLoaded();
|
||||
|
||||
@@ -104,6 +104,8 @@ public:
|
||||
bool enabled;
|
||||
|
||||
private:
|
||||
void tileLoadingCompleteCallback(const TileID& normalized_id);
|
||||
|
||||
void emitSourceLoaded();
|
||||
void emitSourceLoadingFailed(const std::string& message);
|
||||
void emitTileLoaded(bool isNewTile);
|
||||
|
||||
@@ -45,14 +45,13 @@ Sprite::Sprite(const std::string& baseUrl, float pixelRatio_)
|
||||
jsonRequest = nullptr;
|
||||
if (res.status == Response::Successful) {
|
||||
body = res.data;
|
||||
parseJSON();
|
||||
parseJSON(jsonURL);
|
||||
} else {
|
||||
std::stringstream message;
|
||||
message << "Failed to load [" << jsonURL << "]: " << res.message;
|
||||
emitSpriteLoadingFailed(message.str());
|
||||
return;
|
||||
}
|
||||
loadedJSON = true;
|
||||
emitSpriteLoadedIfComplete();
|
||||
});
|
||||
|
||||
@@ -60,14 +59,13 @@ Sprite::Sprite(const std::string& baseUrl, float pixelRatio_)
|
||||
spriteRequest = nullptr;
|
||||
if (res.status == Response::Successful) {
|
||||
image = res.data;
|
||||
parseImage();
|
||||
parseImage(spriteURL);
|
||||
} else {
|
||||
std::stringstream message;
|
||||
message << "Failed to load [" << spriteURL << "]: " << res.message;
|
||||
emitSpriteLoadingFailed(message.str());
|
||||
return;
|
||||
}
|
||||
loadedImage = true;
|
||||
emitSpriteLoadedIfComplete();
|
||||
});
|
||||
}
|
||||
@@ -105,21 +103,29 @@ bool Sprite::hasPixelRatio(float ratio) const {
|
||||
return pixelRatio == (ratio > 1 ? 2 : 1);
|
||||
}
|
||||
|
||||
void Sprite::parseImage() {
|
||||
void Sprite::parseImage(const std::string& spriteURL) {
|
||||
raster = std::make_unique<util::Image>(image);
|
||||
if (!*raster) {
|
||||
raster.reset();
|
||||
std::stringstream message;
|
||||
message << "Failed to parse [" << spriteURL << "]";
|
||||
emitSpriteLoadingFailed(message.str());
|
||||
return;
|
||||
}
|
||||
|
||||
image.clear();
|
||||
loadedImage = true;
|
||||
}
|
||||
|
||||
void Sprite::parseJSON() {
|
||||
void Sprite::parseJSON(const std::string& jsonURL) {
|
||||
rapidjson::Document d;
|
||||
d.Parse<0>(body.c_str());
|
||||
body.clear();
|
||||
|
||||
if (d.HasParseError()) {
|
||||
Log::Warning(Event::Sprite, "sprite JSON is invalid");
|
||||
std::stringstream message;
|
||||
message << "Failed to parse [" << jsonURL << "]: " << d.GetErrorOffset() << " - " << d.GetParseError();
|
||||
emitSpriteLoadingFailed(message.str());
|
||||
} else if (d.IsObject()) {
|
||||
for (rapidjson::Value::ConstMemberIterator itr = d.MemberBegin(); itr != d.MemberEnd(); ++itr) {
|
||||
const std::string& name = itr->name.GetString();
|
||||
@@ -142,8 +148,11 @@ void Sprite::parseJSON() {
|
||||
pos.emplace(name, SpritePosition { x, y, width, height, spritePixelRatio, sdf });
|
||||
}
|
||||
}
|
||||
loadedJSON = true;
|
||||
} else {
|
||||
Log::Warning(Event::Sprite, "sprite JSON root is not an object");
|
||||
std::stringstream message;
|
||||
message << "Failed to parse [" << jsonURL << "]: Root is not an object";
|
||||
emitSpriteLoadingFailed(message.str());
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -59,8 +59,8 @@ private:
|
||||
void emitSpriteLoadedIfComplete();
|
||||
void emitSpriteLoadingFailed(const std::string& message);
|
||||
|
||||
void parseJSON();
|
||||
void parseImage();
|
||||
void parseJSON(const std::string& jsonURL);
|
||||
void parseImage(const std::string& spriteURL);
|
||||
|
||||
std::string body;
|
||||
std::string image;
|
||||
|
||||
@@ -38,18 +38,18 @@ void TileData::setState(const State& state_) {
|
||||
|
||||
void TileData::request(Worker& worker,
|
||||
float pixelRatio,
|
||||
const std::function<void()>& successCallback,
|
||||
const std::function<void(const std::string& message)>& failureCallback) {
|
||||
const std::function<void()>& callback) {
|
||||
std::string url = source.tileURL(id, pixelRatio);
|
||||
state = State::loading;
|
||||
|
||||
req = env.request({ Resource::Kind::Tile, url }, [url, successCallback, failureCallback, &worker, this](const Response &res) {
|
||||
req = env.request({ Resource::Kind::Tile, url }, [url, callback, &worker, this](const Response &res) {
|
||||
req = nullptr;
|
||||
|
||||
if (res.status != Response::Successful) {
|
||||
std::stringstream message;
|
||||
message << "Failed to load [" << url << "]: " << res.message;
|
||||
failureCallback(message.str());
|
||||
setError(message.str());
|
||||
callback();
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -57,7 +57,7 @@ void TileData::request(Worker& worker,
|
||||
data = res.data;
|
||||
|
||||
// Schedule tile parsing in another thread
|
||||
reparse(worker, successCallback);
|
||||
reparse(worker, callback);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -88,3 +88,8 @@ bool TileData::reparse(Worker& worker, std::function<void()> callback) {
|
||||
workRequest = worker.send([this] { parse(); endParsing(); }, callback);
|
||||
return true;
|
||||
}
|
||||
|
||||
void TileData::setError(const std::string& message) {
|
||||
error = message;
|
||||
setState(State::obsolete);
|
||||
}
|
||||
|
||||
@@ -44,10 +44,7 @@ public:
|
||||
TileData(const TileID&, const SourceInfo&);
|
||||
~TileData();
|
||||
|
||||
void request(Worker&,
|
||||
float pixelRatio,
|
||||
const std::function<void()>& successCallback,
|
||||
const std::function<void(const std::string& message)>& failureCallback);
|
||||
void request(Worker&, float pixelRatio, const std::function<void()>& callback);
|
||||
|
||||
// Schedule a tile reparse on a worker thread and call the callback on
|
||||
// completion. It will return true if the work was schedule or false it was
|
||||
@@ -71,13 +68,20 @@ public:
|
||||
// We let subclasses override setState() so they
|
||||
// can intercept the state change and react accordingly.
|
||||
virtual void setState(const State& state);
|
||||
|
||||
inline State getState() const {
|
||||
return state;
|
||||
}
|
||||
|
||||
void endParsing();
|
||||
|
||||
// Error message to be set in case of request
|
||||
// and parsing errors.
|
||||
void setError(const std::string& message);
|
||||
|
||||
std::string getError() const {
|
||||
return error;
|
||||
}
|
||||
|
||||
// Override this in the child class.
|
||||
virtual void parse() = 0;
|
||||
virtual Bucket* getBucket(StyleLayer const &layer_desc) = 0;
|
||||
@@ -106,6 +110,8 @@ protected:
|
||||
private:
|
||||
std::atomic<State> state;
|
||||
|
||||
std::string error;
|
||||
|
||||
protected:
|
||||
// Contains the tile ID string for painting debug information.
|
||||
DebugFontBuffer debugFontBuffer;
|
||||
|
||||
@@ -59,9 +59,9 @@ void VectorTileData::parse() {
|
||||
setState(State::parsed);
|
||||
}
|
||||
} catch (const std::exception& ex) {
|
||||
Log::Error(Event::ParseTile, "Parsing [%d/%d/%d] failed: %s", id.z, id.x, id.y, ex.what());
|
||||
setState(State::obsolete);
|
||||
return;
|
||||
std::stringstream message;
|
||||
message << "Failed to parse [" << int(id.z) << "/" << id.x << "/" << id.y << "]: " << ex.what();
|
||||
setError(message.str());
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -23,14 +23,14 @@ GlyphPBF::GlyphPBF(const std::string& glyphURL,
|
||||
const GlyphLoadingFailedCallback& failureCallback)
|
||||
: parsed(false), env(env_) {
|
||||
// Load the glyph set URL
|
||||
std::string url = util::replaceTokens(glyphURL, [&](const std::string &name) -> std::string {
|
||||
url = util::replaceTokens(glyphURL, [&](const std::string &name) -> std::string {
|
||||
if (name == "fontstack") return util::percentEncode(fontStack);
|
||||
if (name == "range") return util::toString(glyphRange.first) + "-" + util::toString(glyphRange.second);
|
||||
return "";
|
||||
});
|
||||
|
||||
// The prepare call jumps back to the main thread.
|
||||
req = env.request({ Resource::Kind::Glyphs, url }, [&, url, successCallback, failureCallback](const Response &res) {
|
||||
req = env.request({ Resource::Kind::Glyphs, url }, [&, successCallback, failureCallback](const Response &res) {
|
||||
req = nullptr;
|
||||
|
||||
if (res.status != Response::Successful) {
|
||||
|
||||
@@ -29,6 +29,10 @@ public:
|
||||
void parse(FontStack &stack);
|
||||
bool isParsed() const;
|
||||
|
||||
std::string getURL() const {
|
||||
return url;
|
||||
}
|
||||
|
||||
private:
|
||||
GlyphPBF(const GlyphPBF &) = delete;
|
||||
GlyphPBF(GlyphPBF &&) = delete;
|
||||
@@ -36,6 +40,7 @@ private:
|
||||
GlyphPBF &operator=(GlyphPBF &&) = delete;
|
||||
|
||||
std::string data;
|
||||
std::string url;
|
||||
std::atomic<bool> parsed;
|
||||
|
||||
Environment& env;
|
||||
|
||||
@@ -34,8 +34,14 @@ bool GlyphStore::requestGlyphRangesIfNeeded(const std::string& fontStackName,
|
||||
|
||||
auto successCallback = [this, fontStackName](GlyphPBF* glyph) {
|
||||
auto fontStack = createFontStack(fontStackName);
|
||||
glyph->parse(**fontStack);
|
||||
asyncEmitGlyphRangeLoaded->send();
|
||||
try {
|
||||
glyph->parse(**fontStack);
|
||||
asyncEmitGlyphRangeLoaded->send();
|
||||
} catch (const std::exception&) {
|
||||
std::lock_guard<std::mutex> lock(errorMessageMutex);
|
||||
errorMessage = "Failed to parse [" + glyph->getURL() + "]";
|
||||
asyncEmitGlyphRangeLoadedingFailed->send();
|
||||
}
|
||||
};
|
||||
|
||||
auto failureCallback = [this](const std::string& message) {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
#include "../fixtures/util.hpp"
|
||||
#include "mock_file_source.hpp"
|
||||
|
||||
#include <mbgl/storage/request.hpp>
|
||||
@@ -8,30 +9,64 @@ namespace mbgl {
|
||||
|
||||
class MockFileSource::Impl {
|
||||
public:
|
||||
Impl(uv_loop_t*, const std::string& matchFail) : matchFail_(matchFail) {}
|
||||
Impl(uv_loop_t*, Type type, const std::string& match) : type_(type), match_(match) {}
|
||||
|
||||
void handleRequest(Request* req) const;
|
||||
|
||||
private:
|
||||
std::string matchFail_;
|
||||
void replyWithFailure(Response* res) const;
|
||||
void replyWithCorruptedData(Response* res, const std::string& url) const;
|
||||
void replyWithSuccess(Response* res, const std::string& url) const;
|
||||
|
||||
Type type_;
|
||||
std::string match_;
|
||||
};
|
||||
|
||||
void MockFileSource::Impl::handleRequest(Request* req) const {
|
||||
const Resource& resource = req->resource;
|
||||
void MockFileSource::Impl::replyWithFailure(Response* res) const {
|
||||
res->status = Response::Status::Error;
|
||||
res->message = "Failed by the test case";
|
||||
}
|
||||
|
||||
void MockFileSource::Impl::replyWithCorruptedData(Response* res, const std::string& url) const {
|
||||
res->status = Response::Status::Successful;
|
||||
res->data = util::read_file(url);
|
||||
res->data.insert(0, "CORRUPTED");
|
||||
}
|
||||
|
||||
void MockFileSource::Impl::replyWithSuccess(Response* res, const std::string& url) const {
|
||||
res->status = Response::Status::Successful;
|
||||
res->data = util::read_file(url);
|
||||
}
|
||||
|
||||
void MockFileSource::Impl::handleRequest(Request* req) const {
|
||||
const std::string& url = req->resource.url;
|
||||
std::shared_ptr<Response> response = std::make_shared<Response>();
|
||||
if (matchFail_.empty() || resource.url.find(matchFail_) == std::string::npos) {
|
||||
response->status = Response::Status::Successful;
|
||||
response->data = util::read_file(resource.url.c_str());
|
||||
} else {
|
||||
response->message = "Failed by the test case";
|
||||
|
||||
if (url.find(match_) == std::string::npos) {
|
||||
replyWithSuccess(response.get(), url);
|
||||
req->notify(response);
|
||||
return;
|
||||
}
|
||||
|
||||
switch (type_) {
|
||||
case Type::Success:
|
||||
replyWithSuccess(response.get(), url);
|
||||
break;
|
||||
case Type::RequestFail:
|
||||
replyWithFailure(response.get());
|
||||
break;
|
||||
case Type::RequestWithCorruptedData:
|
||||
replyWithCorruptedData(response.get(), url);
|
||||
break;
|
||||
default:
|
||||
EXPECT_TRUE(false) << "Should never be reached.";
|
||||
}
|
||||
|
||||
req->notify(response);
|
||||
}
|
||||
|
||||
MockFileSource::MockFileSource(const std::string& matchFail)
|
||||
: thread_(std::make_unique<util::Thread<Impl>>("FileSource", util::ThreadPriority::Low, matchFail)) {
|
||||
MockFileSource::MockFileSource(Type type, const std::string& match)
|
||||
: thread_(std::make_unique<util::Thread<Impl>>("FileSource", util::ThreadPriority::Low, type, match)) {
|
||||
}
|
||||
|
||||
Request* MockFileSource::request(const Resource& resource, uv_loop_t* loop, Callback callback) {
|
||||
|
||||
@@ -13,12 +13,18 @@ template <typename T> class Thread;
|
||||
}
|
||||
|
||||
// This mock FileSource will read data from the disk and will fail
|
||||
// the request if the URL matches the 'matchFail' string.
|
||||
// the request if the URL matches a string.
|
||||
class MockFileSource : public FileSource {
|
||||
public:
|
||||
enum Type {
|
||||
Success,
|
||||
RequestFail,
|
||||
RequestWithCorruptedData
|
||||
};
|
||||
|
||||
class Impl;
|
||||
|
||||
MockFileSource(const std::string& matchFail);
|
||||
MockFileSource(Type type, const std::string& match);
|
||||
~MockFileSource() override = default;
|
||||
|
||||
// FileSource implementation.
|
||||
|
||||
@@ -107,18 +107,13 @@ private:
|
||||
std::function<void(std::exception_ptr error)> callback_;
|
||||
};
|
||||
|
||||
}
|
||||
|
||||
class ResourceLoaderTest : public ::testing::TestWithParam<std::string> {
|
||||
};
|
||||
|
||||
TEST_P(ResourceLoaderTest, RequestFailure) {
|
||||
void runTestCase(MockFileSource::Type type,
|
||||
const std::string& param,
|
||||
const std::string& message) {
|
||||
util::RunLoop loop(uv_default_loop());
|
||||
|
||||
const std::string param(GetParam());
|
||||
|
||||
MockView view;
|
||||
MockFileSource fileSource(param);
|
||||
MockFileSource fileSource(type, param);
|
||||
|
||||
FixtureLogObserver* log = new FixtureLogObserver();
|
||||
Log::setObserver(std::unique_ptr<Log::Observer>(log));
|
||||
@@ -127,9 +122,6 @@ TEST_P(ResourceLoaderTest, RequestFailure) {
|
||||
try {
|
||||
if (error) {
|
||||
std::rethrow_exception(error);
|
||||
} else {
|
||||
// Control test, should succeed.
|
||||
EXPECT_TRUE(param.empty());
|
||||
}
|
||||
} catch (const util::GlyphRangeLoadingException&) {
|
||||
EXPECT_EQ(param, "glyphs.pbf");
|
||||
@@ -156,24 +148,57 @@ TEST_P(ResourceLoaderTest, RequestFailure) {
|
||||
// join and cease logging after this point.
|
||||
context.reset();
|
||||
|
||||
std::stringstream message;
|
||||
message << "Failed to load [test/fixtures/resources/" << param << "]: Failed by the test case";
|
||||
|
||||
const FixtureLogObserver::LogMessage logMessage {
|
||||
EventSeverity::Error,
|
||||
Event::ResourceLoader,
|
||||
int64_t(-1),
|
||||
message.str(),
|
||||
message,
|
||||
};
|
||||
|
||||
if (param.empty()) {
|
||||
if (type == MockFileSource::Success) {
|
||||
EXPECT_EQ(log->count(logMessage), 0u);
|
||||
} else {
|
||||
EXPECT_GT(log->count(logMessage), 0u);
|
||||
}
|
||||
|
||||
EXPECT_EQ(log->unchecked().size(), 0u);
|
||||
// Clear the remaining error messages
|
||||
log->unchecked().size();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
class ResourceLoaderTest : public ::testing::TestWithParam<std::string> {
|
||||
};
|
||||
|
||||
TEST_P(ResourceLoaderTest, Success) {
|
||||
runTestCase(MockFileSource::Success, GetParam(), std::string());
|
||||
}
|
||||
|
||||
TEST_P(ResourceLoaderTest, RequestFail) {
|
||||
std::stringstream message;
|
||||
message << "Failed to load [test/fixtures/resources/" << GetParam() << "]: Failed by the test case";
|
||||
|
||||
runTestCase(MockFileSource::RequestFail, GetParam(), message.str());
|
||||
}
|
||||
|
||||
TEST_P(ResourceLoaderTest, RequestWithCorruptedData) {
|
||||
const std::string param(GetParam());
|
||||
|
||||
std::stringstream message;
|
||||
message << "Failed to parse ";
|
||||
|
||||
if (param == "vector.pbf") {
|
||||
message << "[15/16384/16384]: pbf unknown field type exception";
|
||||
} else {
|
||||
message << "[test/fixtures/resources/" << param << "]";
|
||||
}
|
||||
|
||||
if (param.find("json") != std::string::npos) {
|
||||
message << ": 0 - Expect either an object or array at root";
|
||||
}
|
||||
|
||||
runTestCase(MockFileSource::RequestWithCorruptedData, GetParam(), message.str());
|
||||
}
|
||||
|
||||
INSTANTIATE_TEST_CASE_P(ResourceLoader, ResourceLoaderTest,
|
||||
::testing::Values("", "source.json", "sprite.json", "sprite.png", "vector.pbf", "glyphs.pbf"));
|
||||
::testing::Values("source.json", "sprite.json", "sprite.png", "vector.pbf", "glyphs.pbf"));
|
||||
|
||||
Reference in New Issue
Block a user