Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
@SmallJoker
SmallJoker committed Mar 20, 2021
1 parent a8cc3bd commit 0571991
Show file tree
Hide file tree
Showing 7 changed files with 153 additions and 86 deletions.
129 changes: 76 additions & 53 deletions src/mapgen/mg_schematic.cpp
View file
Expand Up @@ -76,10 +76,6 @@ void SchematicManager::clear()
///////////////////////////////////////////////////////////////////////////////


Schematic::Schematic()
= default;


Schematic::~Schematic()
{
delete []schemdata;
Expand Down Expand Up @@ -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]);
}
}

Expand Down Expand Up @@ -279,8 +281,7 @@ void Schematic::placeOnMap(ServerMap *map, v3s16 p, u32 flags,
}


bool Schematic::deserializeFromMts(std::istream *is,
std::vector<std::string> *names)
bool Schematic::deserializeFromMts(std::istream *is)
{
std::istream &ss = *is;
content_t cignore = CONTENT_IGNORE;
Expand Down Expand Up @@ -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);
Expand All @@ -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;

Expand Down Expand Up @@ -358,9 +364,11 @@ bool Schematic::deserializeFromMts(std::istream *is,
}


bool Schematic::serializeToMts(std::ostream *os,
const std::vector<std::string> &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
Expand All @@ -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,
Expand All @@ -382,8 +391,7 @@ bool Schematic::serializeToMts(std::ostream *os,
}


bool Schematic::serializeToLua(std::ostream *os,
const std::vector<std::string> &names, bool use_comments,
bool Schematic::serializeToLua(std::ostream *os, bool use_comments,
u32 indent_spaces) const
{
std::ostream &ss = *os;
Expand All @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}
Expand All @@ -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<std::string> ndef_nodenames;
std::vector<std::string> *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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -584,26 +604,29 @@ void Schematic::applyProbabilities(v3s16 p0,
}


void generate_nodelist_and_update_ids(MapNode *nodes, size_t nodecount,
std::vector<std::string> *usednodes, const NodeDefManager *ndef)
void Schematic::condenseContentIds()
{
std::unordered_map<content_t, content_t> 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<content_t, content_t>::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);
}
}
16 changes: 8 additions & 8 deletions src/mapgen/mg_schematic.h
View file
Expand Up @@ -92,7 +92,7 @@ enum SchematicFormatType {

class Schematic : public ObjDef, public NodeResolver {
public:
Schematic();
Schematic() = default;
virtual ~Schematic();

ObjDef *clone() const;
Expand All @@ -105,11 +105,9 @@ class Schematic : public ObjDef, public NodeResolver {
const NodeDefManager *ndef);
bool getSchematicFromMap(Map *map, v3s16 p1, v3s16 p2);

bool deserializeFromMts(std::istream *is, std::vector<std::string> *names);
bool serializeToMts(std::ostream *os,
const std::vector<std::string> &names) const;
bool serializeToLua(std::ostream *os, const std::vector<std::string> &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);
Expand All @@ -124,6 +122,10 @@ class Schematic : public ObjDef, public NodeResolver {
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 {
Expand Down Expand Up @@ -151,5 +153,3 @@ class SchematicManager : public ObjDefManager {
Server *m_server;
};

void generate_nodelist_and_update_ids(MapNode *nodes, size_t nodecount,
std::vector<std::string> *usednodes, const NodeDefManager *ndef);
16 changes: 14 additions & 2 deletions src/nodedef.cpp
View file
Expand Up @@ -1675,8 +1675,7 @@ bool NodeDefManager::nodeboxConnects(MapNode from, MapNode to,

NodeResolver::NodeResolver()
{
m_nodenames.reserve(16);
m_nnlistsizes.reserve(4);
reset();
}


Expand Down Expand Up @@ -1779,3 +1778,16 @@ bool NodeResolver::getIdsFromNrBacklog(std::vector<content_t> *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);
}
31 changes: 28 additions & 3 deletions src/nodedef.h
View file
Expand Up @@ -44,6 +44,9 @@ class ITextureSource;
class IShaderSource;
class IGameDef;
class NodeResolver;
#if BUILD_UNITTESTS
class TestSchematic;
#endif

enum ContentParamType
{
Expand Down Expand Up @@ -789,10 +792,13 @@ class NodeDefManager {

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
Expand All @@ -804,12 +810,31 @@ class NodeResolver {
bool getIdsFromNrBacklog(std::vector<content_t> *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<std::string> 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<size_t> 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;
};

0 comments on commit 0571991

Please sign in to comment.