From 05719913aca97e53ff5b1dde49e1a033a327551f Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Sat, 20 Mar 2021 13:02:15 +0100 Subject: Schematic: Properly deal with before/after node resolving and document (#11011) This fixes an out-of-bounds index access when the node resolver was already applied to the schematic (i.e. biome decoration). Also improves the handling of the two cases: prior node resolving (m_nodenames), and after node resolving (manual lookup) --- src/mapgen/mg_schematic.cpp | 129 ++++++++++++++++++++++--------------- src/mapgen/mg_schematic.h | 16 ++--- src/nodedef.cpp | 16 ++++- src/nodedef.h | 31 ++++++++- src/script/lua_api/l_mapgen.cpp | 5 +- src/unittest/test_noderesolver.cpp | 2 + src/unittest/test_schematic.cpp | 40 +++++++----- 7 files changed, 153 insertions(+), 86 deletions(-) diff --git a/src/mapgen/mg_schematic.cpp b/src/mapgen/mg_schematic.cpp index e70e97e48..653bad4fe 100644 --- a/src/mapgen/mg_schematic.cpp +++ b/src/mapgen/mg_schematic.cpp @@ -76,10 +76,6 @@ void SchematicManager::clear() /////////////////////////////////////////////////////////////////////////////// -Schematic::Schematic() -= default; - - Schematic::~Schematic() { delete []schemdata; @@ -108,13 +104,19 @@ ObjDef *Schematic::clone() const void Schematic::resolveNodeNames() { + c_nodes.clear(); getIdsFromNrBacklog(&c_nodes, true, CONTENT_AIR); size_t bufsize = size.X * size.Y * size.Z; for (size_t i = 0; i != bufsize; i++) { content_t c_original = schemdata[i].getContent(); - content_t c_new = c_nodes[c_original]; - schemdata[i].setContent(c_new); + if (c_original >= c_nodes.size()) { + errorstream << "Corrupt schematic. name=\"" << name + << "\" at index " << i << std::endl; + c_original = 0; + } + // Unfold condensed ID layout to content_t + schemdata[i].setContent(c_nodes[c_original]); } } @@ -279,8 +281,7 @@ void Schematic::placeOnMap(ServerMap *map, v3s16 p, u32 flags, } -bool Schematic::deserializeFromMts(std::istream *is, - std::vector *names) +bool Schematic::deserializeFromMts(std::istream *is) { std::istream &ss = *is; content_t cignore = CONTENT_IGNORE; @@ -312,6 +313,8 @@ bool Schematic::deserializeFromMts(std::istream *is, slice_probs[y] = (version >= 3) ? readU8(ss) : MTSCHEM_PROB_ALWAYS_OLD; //// Read node names + NodeResolver::reset(); + u16 nidmapcount = readU16(ss); for (int i = 0; i != nidmapcount; i++) { std::string name = deSerializeString16(ss); @@ -324,9 +327,12 @@ bool Schematic::deserializeFromMts(std::istream *is, have_cignore = true; } - names->push_back(name); + m_nodenames.push_back(name); } + // Prepare for node resolver + m_nnlistsizes.push_back(m_nodenames.size()); + //// Read node data size_t nodecount = size.X * size.Y * size.Z; @@ -358,9 +364,11 @@ bool Schematic::deserializeFromMts(std::istream *is, } -bool Schematic::serializeToMts(std::ostream *os, - const std::vector &names) const +bool Schematic::serializeToMts(std::ostream *os) const { + // Nodes must not be resolved (-> condensed) + // checking here is not possible because "schemdata" might be temporary. + std::ostream &ss = *os; writeU32(ss, MTSCHEM_FILE_SIGNATURE); // signature @@ -370,9 +378,10 @@ bool Schematic::serializeToMts(std::ostream *os, for (int y = 0; y != size.Y; y++) // Y slice probabilities writeU8(ss, slice_probs[y]); - writeU16(ss, names.size()); // name count - for (size_t i = 0; i != names.size(); i++) - ss << serializeString16(names[i]); // node names + writeU16(ss, m_nodenames.size()); // name count + for (size_t i = 0; i != m_nodenames.size(); i++) { + ss << serializeString16(m_nodenames[i]); // node names + } // compressed bulk node data MapNode::serializeBulk(ss, SER_FMT_VER_HIGHEST_WRITE, @@ -382,8 +391,7 @@ bool Schematic::serializeToMts(std::ostream *os, } -bool Schematic::serializeToLua(std::ostream *os, - const std::vector &names, bool use_comments, +bool Schematic::serializeToLua(std::ostream *os, bool use_comments, u32 indent_spaces) const { std::ostream &ss = *os; @@ -392,6 +400,9 @@ bool Schematic::serializeToLua(std::ostream *os, if (indent_spaces > 0) indent.assign(indent_spaces, ' '); + bool resolve_done = isResolveDone(); + FATAL_ERROR_IF(resolve_done && !m_ndef, "serializeToLua: NodeDefManager is required"); + //// Write header { ss << "schematic = {" << std::endl; @@ -436,9 +447,22 @@ bool Schematic::serializeToLua(std::ostream *os, u8 probability = schemdata[i].param1 & MTSCHEM_PROB_MASK; bool force_place = schemdata[i].param1 & MTSCHEM_FORCE_PLACE; - ss << indent << indent << "{" - << "name=\"" << names[schemdata[i].getContent()] - << "\", prob=" << (u16)probability * 2 + // After node resolving: real content_t, lookup using NodeDefManager + // Prior node resolving: condensed ID, lookup using m_nodenames + content_t c = schemdata[i].getContent(); + + ss << indent << indent << "{" << "name=\""; + + if (!resolve_done) { + // Prior node resolving (eg. direct schematic load) + FATAL_ERROR_IF(c >= m_nodenames.size(), "Invalid node list"); + ss << m_nodenames[c]; + } else { + // After node resolving (eg. biome decoration) + ss << m_ndef->get(c).name; + } + + ss << "\", prob=" << (u16)probability * 2 << ", param2=" << (u16)schemdata[i].param2; if (force_place) @@ -467,25 +491,24 @@ bool Schematic::loadSchematicFromFile(const std::string &filename, return false; } - size_t origsize = m_nodenames.size(); - if (!deserializeFromMts(&is, &m_nodenames)) - return false; + if (!m_ndef) + m_ndef = ndef; - m_nnlistsizes.push_back(m_nodenames.size() - origsize); + if (!deserializeFromMts(&is)) + return false; name = filename; if (replace_names) { - for (size_t i = origsize; i < m_nodenames.size(); i++) { - std::string &node_name = m_nodenames[i]; + for (std::string &node_name : m_nodenames) { StringMap::iterator it = replace_names->find(node_name); if (it != replace_names->end()) node_name = it->second; } } - if (ndef) - ndef->pendNodeResolve(this); + if (m_ndef) + m_ndef->pendNodeResolve(this); return true; } @@ -494,33 +517,26 @@ bool Schematic::loadSchematicFromFile(const std::string &filename, bool Schematic::saveSchematicToFile(const std::string &filename, const NodeDefManager *ndef) { - MapNode *orig_schemdata = schemdata; - std::vector ndef_nodenames; - std::vector *names; + Schematic *schem = this; - if (m_resolve_done && ndef == NULL) - ndef = m_ndef; + bool needs_condense = isResolveDone(); - if (ndef) { - names = &ndef_nodenames; + if (!m_ndef) + m_ndef = ndef; - u32 volume = size.X * size.Y * size.Z; - schemdata = new MapNode[volume]; - for (u32 i = 0; i != volume; i++) - schemdata[i] = orig_schemdata[i]; + if (needs_condense) { + if (!m_ndef) + return false; - generate_nodelist_and_update_ids(schemdata, volume, names, ndef); - } else { // otherwise, use the names we have on hand in the list - names = &m_nodenames; + schem = (Schematic *)this->clone(); + schem->condenseContentIds(); } std::ostringstream os(std::ios_base::binary); - bool status = serializeToMts(&os, *names); + bool status = schem->serializeToMts(&os); - if (ndef) { - delete []schemdata; - schemdata = orig_schemdata; - } + if (needs_condense) + delete schem; if (!status) return false; @@ -556,6 +572,10 @@ bool Schematic::getSchematicFromMap(Map *map, v3s16 p1, v3s16 p2) } delete vm; + + // Reset and mark as complete + NodeResolver::reset(true); + return true; } @@ -584,26 +604,29 @@ void Schematic::applyProbabilities(v3s16 p0, } -void generate_nodelist_and_update_ids(MapNode *nodes, size_t nodecount, - std::vector *usednodes, const NodeDefManager *ndef) +void Schematic::condenseContentIds() { std::unordered_map nodeidmap; content_t numids = 0; + // Reset node resolve fields + NodeResolver::reset(); + + size_t nodecount = size.X * size.Y * size.Z; for (size_t i = 0; i != nodecount; i++) { content_t id; - content_t c = nodes[i].getContent(); + content_t c = schemdata[i].getContent(); - std::unordered_map::const_iterator it = nodeidmap.find(c); + auto it = nodeidmap.find(c); if (it == nodeidmap.end()) { id = numids; numids++; - usednodes->push_back(ndef->get(c).name); - nodeidmap.insert(std::make_pair(c, id)); + m_nodenames.push_back(m_ndef->get(c).name); + nodeidmap.emplace(std::make_pair(c, id)); } else { id = it->second; } - nodes[i].setContent(id); + schemdata[i].setContent(id); } } diff --git a/src/mapgen/mg_schematic.h b/src/mapgen/mg_schematic.h index 6b31251b6..5f64ea280 100644 --- a/src/mapgen/mg_schematic.h +++ b/src/mapgen/mg_schematic.h @@ -92,7 +92,7 @@ enum SchematicFormatType { class Schematic : public ObjDef, public NodeResolver { public: - Schematic(); + Schematic() = default; virtual ~Schematic(); ObjDef *clone() const; @@ -105,11 +105,9 @@ public: const NodeDefManager *ndef); bool getSchematicFromMap(Map *map, v3s16 p1, v3s16 p2); - bool deserializeFromMts(std::istream *is, std::vector *names); - bool serializeToMts(std::ostream *os, - const std::vector &names) const; - bool serializeToLua(std::ostream *os, const std::vector &names, - bool use_comments, u32 indent_spaces) const; + bool deserializeFromMts(std::istream *is); + bool serializeToMts(std::ostream *os) const; + bool serializeToLua(std::ostream *os, bool use_comments, u32 indent_spaces) const; void blitToVManip(MMVManip *vm, v3s16 p, Rotation rot, bool force_place); bool placeOnVManip(MMVManip *vm, v3s16 p, u32 flags, Rotation rot, bool force_place); @@ -124,6 +122,10 @@ public: v3s16 size; MapNode *schemdata = nullptr; u8 *slice_probs = nullptr; + +private: + // Counterpart to the node resolver: Condense content_t to a sequential "m_nodenames" list + void condenseContentIds(); }; class SchematicManager : public ObjDefManager { @@ -151,5 +153,3 @@ private: Server *m_server; }; -void generate_nodelist_and_update_ids(MapNode *nodes, size_t nodecount, - std::vector *usednodes, const NodeDefManager *ndef); diff --git a/src/nodedef.cpp b/src/nodedef.cpp index 57d4c008f..8a1f6203b 100644 --- a/src/nodedef.cpp +++ b/src/nodedef.cpp @@ -1675,8 +1675,7 @@ bool NodeDefManager::nodeboxConnects(MapNode from, MapNode to, NodeResolver::NodeResolver() { - m_nodenames.reserve(16); - m_nnlistsizes.reserve(4); + reset(); } @@ -1779,3 +1778,16 @@ bool NodeResolver::getIdsFromNrBacklog(std::vector *result_out, return success; } + +void NodeResolver::reset(bool resolve_done) +{ + m_nodenames.clear(); + m_nodenames_idx = 0; + m_nnlistsizes.clear(); + m_nnlistsizes_idx = 0; + + m_resolve_done = resolve_done; + + m_nodenames.reserve(16); + m_nnlistsizes.reserve(4); +} diff --git a/src/nodedef.h b/src/nodedef.h index 6fc20518d..3e77624eb 100644 --- a/src/nodedef.h +++ b/src/nodedef.h @@ -44,6 +44,9 @@ class ITextureSource; class IShaderSource; class IGameDef; class NodeResolver; +#if BUILD_UNITTESTS +class TestSchematic; +#endif enum ContentParamType { @@ -789,10 +792,13 @@ private: NodeDefManager *createNodeDefManager(); +// NodeResolver: Queue for node names which are then translated +// to content_t after the NodeDefManager was initialized class NodeResolver { public: NodeResolver(); virtual ~NodeResolver(); + // Callback which is run as soon NodeDefManager is ready virtual void resolveNodeNames() = 0; // required because this class is used as mixin for ObjDef @@ -804,12 +810,31 @@ public: bool getIdsFromNrBacklog(std::vector *result_out, bool all_required = false, content_t c_fallback = CONTENT_IGNORE); - void nodeResolveInternal(); + inline bool isResolveDone() const { return m_resolve_done; } + void reset(bool resolve_done = false); - u32 m_nodenames_idx = 0; - u32 m_nnlistsizes_idx = 0; + // Vector containing all node names in the resolve "queue" std::vector m_nodenames; + // Specifies the "set size" of node names which are to be processed + // this is used for getIdsFromNrBacklog + // TODO: replace or remove std::vector m_nnlistsizes; + +protected: + friend class NodeDefManager; // m_ndef + const NodeDefManager *m_ndef = nullptr; + // Index of the next "m_nodenames" entry to resolve + u32 m_nodenames_idx = 0; + +private: +#if BUILD_UNITTESTS + // Unittest requires access to m_resolve_done + friend class TestSchematic; +#endif + void nodeResolveInternal(); + + // Index of the next "m_nnlistsizes" entry to process + u32 m_nnlistsizes_idx = 0; bool m_resolve_done = false; }; diff --git a/src/script/lua_api/l_mapgen.cpp b/src/script/lua_api/l_mapgen.cpp index 12a497b1e..cc93bdbd0 100644 --- a/src/script/lua_api/l_mapgen.cpp +++ b/src/script/lua_api/l_mapgen.cpp @@ -1734,11 +1734,10 @@ int ModApiMapgen::l_serialize_schematic(lua_State *L) std::ostringstream os(std::ios_base::binary); switch (schem_format) { case SCHEM_FMT_MTS: - schem->serializeToMts(&os, schem->m_nodenames); + schem->serializeToMts(&os); break; case SCHEM_FMT_LUA: - schem->serializeToLua(&os, schem->m_nodenames, - use_comments, indent_spaces); + schem->serializeToLua(&os, use_comments, indent_spaces); break; default: return 0; diff --git a/src/unittest/test_noderesolver.cpp b/src/unittest/test_noderesolver.cpp index 28da43620..ed66093a9 100644 --- a/src/unittest/test_noderesolver.cpp +++ b/src/unittest/test_noderesolver.cpp @@ -56,6 +56,8 @@ void TestNodeResolver::runTests(IGameDef *gamedef) class Foobar : public NodeResolver { public: + friend class TestNodeResolver; // m_ndef + void resolveNodeNames(); content_t test_nr_node1; diff --git a/src/unittest/test_schematic.cpp b/src/unittest/test_schematic.cpp index da4ce50d2..d2f027eb4 100644 --- a/src/unittest/test_schematic.cpp +++ b/src/unittest/test_schematic.cpp @@ -66,13 +66,14 @@ void TestSchematic::testMtsSerializeDeserialize(const NodeDefManager *ndef) std::stringstream ss(std::ios_base::binary | std::ios_base::in | std::ios_base::out); - std::vector names; - names.emplace_back("foo"); - names.emplace_back("bar"); - names.emplace_back("baz"); - names.emplace_back("qux"); - - Schematic schem, schem2; + Schematic schem; + { + std::vector &names = schem.m_nodenames; + names.emplace_back("foo"); + names.emplace_back("bar"); + names.emplace_back("baz"); + names.emplace_back("qux"); + } schem.flags = 0; schem.size = size; @@ -83,18 +84,21 @@ void TestSchematic::testMtsSerializeDeserialize(const NodeDefManager *ndef) for (s16 y = 0; y != size.Y; y++) schem.slice_probs[y] = MTSCHEM_PROB_ALWAYS; - UASSERT(schem.serializeToMts(&ss, names)); + UASSERT(schem.serializeToMts(&ss)); ss.seekg(0); - names.clear(); - UASSERT(schem2.deserializeFromMts(&ss, &names)); + Schematic schem2; + UASSERT(schem2.deserializeFromMts(&ss)); - UASSERTEQ(size_t, names.size(), 4); - UASSERTEQ(std::string, names[0], "foo"); - UASSERTEQ(std::string, names[1], "bar"); - UASSERTEQ(std::string, names[2], "baz"); - UASSERTEQ(std::string, names[3], "qux"); + { + std::vector &names = schem2.m_nodenames; + UASSERTEQ(size_t, names.size(), 4); + UASSERTEQ(std::string, names[0], "foo"); + UASSERTEQ(std::string, names[1], "bar"); + UASSERTEQ(std::string, names[2], "baz"); + UASSERTEQ(std::string, names[3], "qux"); + } UASSERT(schem2.size == size); for (size_t i = 0; i != volume; i++) @@ -120,14 +124,14 @@ void TestSchematic::testLuaTableSerialize(const NodeDefManager *ndef) for (s16 y = 0; y != size.Y; y++) schem.slice_probs[y] = MTSCHEM_PROB_ALWAYS; - std::vector names; + std::vector &names = schem.m_nodenames; names.emplace_back("air"); names.emplace_back("default:lava_source"); names.emplace_back("default:glass"); std::ostringstream ss(std::ios_base::binary); - UASSERT(schem.serializeToLua(&ss, names, false, 0)); + UASSERT(schem.serializeToLua(&ss, false, 0)); UASSERTEQ(std::string, ss.str(), expected_lua_output); } @@ -159,6 +163,8 @@ void TestSchematic::testFileSerializeDeserialize(const NodeDefManager *ndef) schem1.slice_probs[0] = 80; schem1.slice_probs[1] = 160; schem1.slice_probs[2] = 240; + // Node resolving happened manually. + schem1.m_resolve_done = true; for (size_t i = 0; i != volume; i++) { content_t c = content_map[test_schem2_data[i]]; -- cgit v1.2.3