Improve decode error diagnostics

src/BlockDecoder.cpp

@@ -1,6 +1,5 @@
 #include <string>
-#include <iostream>
+#include <utility>
-#include <sstream>
 
 #include "BlockDecoder.h"
 #include "ZlibDecompressor.h"
@@ -46,13 +45,12 @@ void BlockDecoder::decode(const ustring &datastr)
 {
 	const unsigned char *data = datastr.c_str();
 	size_t length = datastr.length();
-	// TODO: bounds checks
+	// TODO: Add strict bounds checks everywhere
 
 	uint8_t version = data[0];
 	if (version < 22) {
-		std::ostringstream oss;
+		auto err = "Unsupported map version " + std::to_string(version);
-		oss << "Unsupported map version " << (int)version;
+		throw std::runtime_error(err);
-		throw std::runtime_error(oss.str());
 	}
 	m_version = version;
 
@@ -87,7 +85,7 @@ void BlockDecoder::decode(const ustring &datastr)
 			else if (name == "ignore")
 				m_blockIgnoreId = nodeId;
 			else
-				m_nameMap[nodeId] = name;
+				m_nameMap[nodeId] = std::move(name);
 			dataOffset += nameLen;
 		}
 	};
@@ -99,14 +97,20 @@ void BlockDecoder::decode(const ustring &datastr)
 	dataOffset++;
 	uint8_t paramsWidth = data[dataOffset];
 	dataOffset++;
-	if (contentWidth != 1 && contentWidth != 2)
+	if (contentWidth != 1 && contentWidth != 2) {
-		throw std::runtime_error("unsupported map version (contentWidth)");
+		auto err = "Unsupported map version contentWidth=" + std::to_string(contentWidth);
-	if (paramsWidth != 2)
+		throw std::runtime_error(err);
-		throw std::runtime_error("unsupported map version (paramsWidth)");
+	}
+	if (paramsWidth != 2) {
+		auto err = "Unsupported map version paramsWidth=" + std::to_string(paramsWidth);
+		throw std::runtime_error(err);
+	}
 	m_contentWidth = contentWidth;
+	const size_t mapDataSize = (contentWidth + paramsWidth) * 4096;
 
 	if (version >= 29) {
-		size_t mapDataSize = (contentWidth + paramsWidth) * 4096;
+		if (length < dataOffset + mapDataSize)
+			throw std::runtime_error("Map data buffer truncated");
 		m_mapData.assign(data + dataOffset, mapDataSize);
 		return; // we have read everything we need and can return early
 	}
@@ -118,6 +122,9 @@ void BlockDecoder::decode(const ustring &datastr)
 	decompressor.decompress(m_scratch); // unused metadata
 	dataOffset = decompressor.seekPos();
 
+	if (m_mapData.size() < mapDataSize)
+		throw std::runtime_error("Map data buffer truncated");
+
 	// Skip unused node timers
 	if (version == 23)
 		dataOffset += 1;
@@ -132,7 +139,7 @@ void BlockDecoder::decode(const ustring &datastr)
 
 	// Skip unused static objects
 	dataOffset++; // Skip static object version
-	int staticObjectCount = readU16(data + dataOffset);
+	uint16_t staticObjectCount = readU16(data + dataOffset);
 	dataOffset += 2;
 	for (int i = 0; i < staticObjectCount; ++i) {
 		dataOffset += 13;
@@ -161,7 +168,7 @@ const std::string &BlockDecoder::getNode(u8 x, u8 y, u8 z) const
 		return empty;
 	NameMap::const_iterator it = m_nameMap.find(content);
 	if (it == m_nameMap.end()) {
-		errorstream << "Skipping node with invalid ID." << std::endl;
+		errorstream << "Skipping node with invalid ID " << (int)content << std::endl;
 		return empty;
 	}
 	return it->second;

src/TileGenerator.cpp

@@ -546,6 +546,19 @@ void TileGenerator::renderMap()
 	const int16_t yMin = mod16(m_yMin);
 	size_t count = 0;
 
+	// returns true to skip
+	auto decode = [&] (BlockPos pos, const ustring &buf) -> bool {
+		blk.reset();
+		try {
+			blk.decode(buf);
+		} catch (std::exception &e) {
+			errorstream << "While decoding block " << pos.x << ',' << pos.y << ',' << pos.z
+				<< ':' << std::endl;
+			throw;
+		};
+		return blk.isEmpty();
+	};
+
 	auto renderSingle = [&] (int16_t xPos, int16_t zPos, BlockList &blockStack) {
 		m_readPixels.reset();
 		m_readInfo.reset();
@@ -562,10 +575,7 @@ void TileGenerator::renderMap()
 			assert(pos.x == xPos && pos.z == zPos);
 			assert(pos.y >= yMin && pos.y < yMax);
 
-			blk.reset();
+			decode(pos, it.second);
-			blk.decode(it.second);
-			if (blk.isEmpty())
-				continue;
 			renderMapBlock(blk, pos);
 
 			// Exit out if all pixels for this MapBlock are covered

src/mapper.cpp

@@ -274,12 +274,12 @@ int main(int argc, char *argv[])
 			return 0;
 		}
 
-		if(colors.empty())
+		if (colors.empty())
 			colors = search_colors(input);
 		generator.parseColorsFile(colors);
 		generator.generate(input, output);
 
-	} catch (const std::exception &e) {
+	} catch (std::exception &e) {
 		errorstream << "Exception: " << e.what() << std::endl;
 		return 1;
 	}