Wrap NBTTagCompound#setTag in static helper with safeguards against circular references, fixes #299

This commit is contained in:
Electroblob77
2020-01-27 13:45:47 +00:00
parent 6cd6411c25
commit f5da195605
31 changed files with 96 additions and 48 deletions
@@ -12,9 +12,11 @@ import electroblob.wizardry.misc.Forfeit;
import electroblob.wizardry.packet.WizardryPacketHandler;
import electroblob.wizardry.registry.*;
import electroblob.wizardry.spell.Spell;
import electroblob.wizardry.util.NBTExtras;
import electroblob.wizardry.util.SpellProperties;
import electroblob.wizardry.worldgen.*;
import net.minecraft.item.Item;
import net.minecraft.nbt.NBTTagCompound;
import net.minecraft.util.SoundCategory;
import net.minecraftforge.common.MinecraftForge;
import net.minecraftforge.event.RegistryEvent;
@@ -31,7 +31,6 @@ import net.minecraft.inventory.Slot;
import net.minecraft.item.Item;
import net.minecraft.item.ItemStack;
import net.minecraft.util.ResourceLocation;
import net.minecraft.util.math.MathHelper;
import net.minecraft.util.text.TextFormatting;
import net.minecraftforge.client.event.TextureStitchEvent;
import net.minecraftforge.fml.common.Mod;
@@ -4,6 +4,7 @@ import electroblob.wizardry.Wizardry;
import electroblob.wizardry.data.SpellEmitterData;
import electroblob.wizardry.event.SpellCastEvent;
import electroblob.wizardry.spell.Spell;
import electroblob.wizardry.util.NBTExtras;
import electroblob.wizardry.util.SpellModifiers;
import io.netty.buffer.ByteBuf;
import net.minecraft.nbt.NBTTagCompound;
@@ -129,7 +130,7 @@ public class SpellEmitter implements ITickable {
nbt.setDouble("z", z);
nbt.setInteger("direction", direction.getIndex());
nbt.setInteger("duration", duration);
nbt.setTag("modifiers", modifiers.toNBT());
NBTExtras.storeTagSafely(nbt, "modifiers", modifiers.toNBT());
nbt.setInteger("castingTick", castingTick);
return nbt;
@@ -7,6 +7,7 @@ import electroblob.wizardry.packet.WizardryPacketHandler;
import electroblob.wizardry.registry.Spells;
import electroblob.wizardry.spell.None;
import electroblob.wizardry.spell.Spell;
import electroblob.wizardry.util.NBTExtras;
import electroblob.wizardry.util.SpellModifiers;
import net.minecraft.nbt.NBTTagCompound;
import net.minecraft.tileentity.TileEntity;
@@ -141,7 +142,7 @@ public abstract class BlockCastingData<T extends TileEntity> implements INBTSeri
nbt.setInteger("spell", spell.metadata());
nbt.setInteger("castingTick", castingTick);
nbt.setTag("modifiers", modifiers.toNBT());
NBTExtras.storeTagSafely(nbt, "modifiers", modifiers.toNBT());
return nbt;
}
@@ -1,5 +1,6 @@
package electroblob.wizardry.data;
import electroblob.wizardry.util.NBTExtras;
import io.netty.buffer.ByteBuf;
import net.minecraft.entity.player.EntityPlayer;
import net.minecraft.item.ItemStack;
@@ -89,7 +90,7 @@ public interface IStoredVariable<T> extends IVariable<T> {
@Override
public void write(NBTTagCompound nbt, T value){
if(value != null) nbt.setTag(key, serialiser.apply(value));
if(value != null) NBTExtras.storeTagSafely(nbt, key, serialiser.apply(value));
}
@Override
@@ -91,7 +91,7 @@ public class SpellEmitterData extends WorldSavedData {
@Override
public NBTTagCompound writeToNBT(NBTTagCompound compound){
compound.setTag("emitters", NBTExtras.listToNBT(emitters, SpellEmitter::toNBT));
NBTExtras.storeTagSafely(compound, "emitters", NBTExtras.listToNBT(emitters, SpellEmitter::toNBT));
return compound;
}
@@ -4,6 +4,7 @@ import electroblob.wizardry.Wizardry;
import electroblob.wizardry.packet.PacketGlyphData;
import electroblob.wizardry.packet.WizardryPacketHandler;
import electroblob.wizardry.spell.Spell;
import electroblob.wizardry.util.NBTExtras;
import net.minecraft.entity.player.EntityPlayerMP;
import net.minecraft.nbt.NBTTagCompound;
import net.minecraft.nbt.NBTTagList;
@@ -171,7 +172,7 @@ public class SpellGlyphData extends WorldSavedData {
tagList.appendTag(tag);
}
nbt.setTag("spellGlyphData", tagList);
NBTExtras.storeTagSafely(nbt, "spellGlyphData", tagList);
return nbt;
}
@@ -515,12 +515,12 @@ public class WizardData implements INBTSerializable<NBTTagCompound> {
NBTTagCompound properties = new NBTTagCompound();
properties.setTag("imbuements", NBTExtras.mapToNBT(this.imbuementDurations,
NBTExtras.storeTagSafely(properties, "imbuements", NBTExtras.mapToNBT(this.imbuementDurations,
imbuement -> new NBTTagInt(Enchantment.getEnchantmentID((Enchantment)imbuement)), NBTTagInt::new));
// Mmmmmm Java 8....
properties.setTag("allies", NBTExtras.listToNBT(this.allies, NBTUtil::createUUIDTag));
properties.setTag("allyNames", NBTExtras.listToNBT(this.allyNames, NBTTagString::new));
NBTExtras.storeTagSafely(properties, "allies", NBTExtras.listToNBT(this.allies, NBTUtil::createUUIDTag));
NBTExtras.storeTagSafely(properties, "allyNames", NBTExtras.listToNBT(this.allyNames, NBTTagString::new));
// Might be worth converting this over to WizardryUtilities.listToNBT.
int[] spells = new int[this.spellsDiscovered.size()];
@@ -531,7 +531,7 @@ public class WizardData implements INBTSerializable<NBTTagCompound> {
}
properties.setIntArray("discoveredSpells", spells);
properties.setTag("recentSpells", NBTExtras.listToNBT(recentSpells, s -> new NBTTagInt(s.metadata())));
NBTExtras.storeTagSafely(properties, "recentSpells", NBTExtras.listToNBT(recentSpells, s -> new NBTTagInt(s.metadata())));
storedVariables.forEach(k -> k.write(properties, this.spellData.get(k)));
@@ -5,6 +5,7 @@ import electroblob.wizardry.registry.Spells;
import electroblob.wizardry.spell.Spell;
import electroblob.wizardry.util.AllyDesignationSystem;
import electroblob.wizardry.util.MagicDamage;
import electroblob.wizardry.util.NBTExtras;
import electroblob.wizardry.util.WizardryUtilities;
import io.netty.buffer.ByteBuf;
import net.minecraft.block.Block;
@@ -183,7 +184,7 @@ public class EntityLevitatingBlock extends EntityFallingBlock implements IEntity
NBTBase nbtbase = this.tileEntityData.getTag(s);
if(!"x".equals(s) && !"y".equals(s) && !"z".equals(s)){
nbttagcompound.setTag(s, nbtbase.copy());
NBTExtras.storeTagSafely(nbttagcompound, s, nbtbase.copy());
}
}
@@ -1,6 +1,5 @@
package electroblob.wizardry.entity.construct;
import electroblob.wizardry.Wizardry;
import electroblob.wizardry.registry.Spells;
import electroblob.wizardry.spell.Spell;
import electroblob.wizardry.util.WizardryUtilities;
@@ -269,9 +269,9 @@ public class EntityEvilWizard extends EntityMob implements ISpellCaster, IEntity
super.writeEntityToNBT(nbt);
nbt.setInteger("element", this.getElement().ordinal());
nbt.setInteger("skin", this.textureIndex);
nbt.setTag("spells", NBTExtras.listToNBT(spells, spell -> new NBTTagInt(spell.metadata())));
NBTExtras.storeTagSafely(nbt, "spells", NBTExtras.listToNBT(spells, spell -> new NBTTagInt(spell.metadata())));
nbt.setBoolean("hasStructure", this.hasStructure);
nbt.setTag("groupUUIDs", NBTExtras.listToNBT(groupUUIDs, NBTUtil::createUUIDTag));
NBTExtras.storeTagSafely(nbt, "groupUUIDs", NBTExtras.listToNBT(groupUUIDs, NBTUtil::createUUIDTag));
}
@Override
@@ -19,9 +19,6 @@ import net.minecraftforge.fml.common.eventhandler.Event;
import net.minecraftforge.fml.common.eventhandler.SubscribeEvent;
import org.apache.commons.lang3.ArrayUtils;
import java.util.Arrays;
import java.util.function.IntPredicate;
@Mod.EventBusSubscriber
public class EntityLightningWraith extends EntityBlazeMinion {
@@ -389,15 +389,15 @@ public class EntityWizard extends EntityCreature implements INpc, IMerchant, ISp
super.writeEntityToNBT(nbt);
if(this.trades != null){
nbt.setTag("trades", this.trades.getRecipiesAsTags());
NBTExtras.storeTagSafely(nbt, "trades", this.trades.getRecipiesAsTags());
}
nbt.setInteger("element", this.getElement().ordinal());
nbt.setInteger("skin", this.textureIndex);
nbt.setTag("spells", NBTExtras.listToNBT(spells, spell -> new NBTTagInt(spell.metadata())));
NBTExtras.storeTagSafely(nbt, "spells", NBTExtras.listToNBT(spells, spell -> new NBTTagInt(spell.metadata())));
if(this.towerBlocks != null && this.towerBlocks.size() > 0){
nbt.setTag("towerBlocks", NBTExtras.listToNBT(this.towerBlocks, NBTUtil::createPosTag));
NBTExtras.storeTagSafely(nbt, "towerBlocks", NBTExtras.listToNBT(this.towerBlocks, NBTUtil::createPosTag));
}
}
@@ -9,7 +9,6 @@ import electroblob.wizardry.util.ParticleBuilder.Type;
import electroblob.wizardry.util.WizardryUtilities;
import net.minecraft.entity.EntityLiving;
import net.minecraft.entity.EntityLivingBase;
import net.minecraft.entity.player.EntityPlayer;
import net.minecraft.init.MobEffects;
import net.minecraft.potion.PotionEffect;
import net.minecraft.util.EnumParticleTypes;
@@ -1,7 +1,6 @@
package electroblob.wizardry.item;
import electroblob.wizardry.Wizardry;
import electroblob.wizardry.data.SpellGlyphData;
import electroblob.wizardry.event.SpellCastEvent;
import electroblob.wizardry.event.SpellCastEvent.Source;
import electroblob.wizardry.packet.PacketCastSpell;
@@ -20,7 +19,6 @@ import net.minecraftforge.common.MinecraftForge;
import net.minecraftforge.fml.common.network.simpleimpl.IMessage;
import net.minecraftforge.fml.relauncher.Side;
import net.minecraftforge.fml.relauncher.SideOnly;
import net.minecraftforge.oredict.OreDictionary;
import java.util.List;
@@ -27,7 +27,10 @@ import net.minecraft.inventory.Slot;
import net.minecraft.item.EnumAction;
import net.minecraft.item.Item;
import net.minecraft.item.ItemStack;
import net.minecraft.util.*;
import net.minecraft.util.ActionResult;
import net.minecraft.util.EnumActionResult;
import net.minecraft.util.EnumHand;
import net.minecraft.util.SoundCategory;
import net.minecraft.util.math.BlockPos;
import net.minecraft.util.math.RayTraceResult;
import net.minecraft.util.math.Vec3d;
@@ -2,6 +2,7 @@ package electroblob.wizardry.potion;
import electroblob.wizardry.Wizardry;
import electroblob.wizardry.registry.WizardryPotions;
import electroblob.wizardry.util.NBTExtras;
import electroblob.wizardry.util.WizardryUtilities;
import net.minecraft.entity.EntityLivingBase;
import net.minecraft.entity.player.EntityPlayerMP;
@@ -40,7 +41,7 @@ public class PotionContainment extends PotionMagicEffect {
// Initialise the containment position to the entity's position if it wasn't set already
if(!target.getEntityData().hasKey(ENTITY_TAG)){
target.getEntityData().setTag(ENTITY_TAG, NBTUtil.createPosTag(new BlockPos(target.getPositionVector().subtract(0.5, 0.5, 0.5))));
NBTExtras.storeTagSafely(target.getEntityData(), ENTITY_TAG, NBTUtil.createPosTag(new BlockPos(target.getPositionVector().subtract(0.5, 0.5, 0.5))));
}
Vec3d origin = WizardryUtilities.getCentre(NBTUtil.getPosFromTag(target.getEntityData().getCompoundTag(ENTITY_TAG)));
@@ -5,7 +5,6 @@ import electroblob.wizardry.constants.Tier;
import electroblob.wizardry.loot.RandomSpell;
import electroblob.wizardry.loot.WizardSpell;
import electroblob.wizardry.spell.Spell;
import net.minecraft.entity.EntityList;
import net.minecraft.entity.EnumCreatureType;
import net.minecraft.util.ResourceLocation;
import net.minecraft.world.storage.loot.*;
@@ -3,6 +3,7 @@ package electroblob.wizardry.spell;
import com.google.common.collect.ImmutableMap;
import electroblob.wizardry.item.IConjuredItem;
import electroblob.wizardry.registry.WizardryItems;
import electroblob.wizardry.util.NBTExtras;
import electroblob.wizardry.util.SpellModifiers;
import electroblob.wizardry.util.WizardryUtilities;
import net.minecraft.entity.player.EntityPlayer;
@@ -40,7 +41,7 @@ public class ConjureArmour extends SpellConjuration {
armour = new ItemStack(SPECTRAL_ARMOUR_MAP.get(slot));
IConjuredItem.setDurationMultiplier(armour, modifiers.get(WizardryItems.duration_upgrade));
// Sets a blank "ench" tag to trick the renderer into showing the enchantment effect on the armour model
armour.getTagCompound().setTag("ench", new NBTTagList());
NBTExtras.storeTagSafely(armour.getTagCompound(), "ench", new NBTTagList());
caster.setItemStackToSlot(slot, armour);
flag = true;
}
@@ -9,7 +9,6 @@ import net.minecraft.entity.EntityLivingBase;
import net.minecraft.entity.player.EntityPlayer;
import net.minecraft.item.EnumAction;
import net.minecraft.util.EnumHand;
import net.minecraft.util.SoundEvent;
import net.minecraft.world.World;
public class Glide extends Spell {
@@ -16,6 +16,7 @@ import electroblob.wizardry.packet.PacketPossession;
import electroblob.wizardry.packet.WizardryPacketHandler;
import electroblob.wizardry.registry.Spells;
import electroblob.wizardry.registry.WizardryItems;
import electroblob.wizardry.util.NBTExtras;
import electroblob.wizardry.util.ParticleBuilder;
import electroblob.wizardry.util.ParticleBuilder.Type;
import electroblob.wizardry.util.SpellModifiers;
@@ -267,7 +268,7 @@ public class Possession extends SpellRay {
// Inventory and items
if(possessor.getEntityData() != null){
possessor.getEntityData().setTag(INVENTORY_NBT_KEY, possessor.inventory.writeToNBT(new NBTTagList()));
NBTExtras.storeTagSafely(possessor.getEntityData(), INVENTORY_NBT_KEY, possessor.inventory.writeToNBT(new NBTTagList()));
}
possessor.inventory.clear();
@@ -1,6 +1,7 @@
package electroblob.wizardry.spell;
import electroblob.wizardry.registry.WizardryItems;
import electroblob.wizardry.util.NBTExtras;
import electroblob.wizardry.util.SpellModifiers;
import electroblob.wizardry.util.WizardryUtilities;
import net.minecraft.entity.Entity;
@@ -89,7 +90,7 @@ public class ShulkerBullet extends Spell {
targetTag.setInteger("X", pos.getX());
targetTag.setInteger("Y", pos.getY());
targetTag.setInteger("Z", pos.getZ());
nbt.setTag("Target", targetTag);
NBTExtras.storeTagSafely(nbt, "Target", targetTag);
bullet.readFromNBT(nbt); // LOL I just modified private fields without reflection
world.spawnEntity(bullet);
@@ -10,11 +10,7 @@ import net.minecraft.entity.player.EntityPlayer;
import net.minecraft.item.EnumAction;
import net.minecraft.util.EnumHand;
import net.minecraft.util.math.BlockPos;
import net.minecraft.util.text.ITextComponent;
import net.minecraft.util.text.TextComponentTranslation;
import net.minecraft.world.World;
import net.minecraftforge.fml.relauncher.Side;
import net.minecraftforge.fml.relauncher.SideOnly;
public class SummonSnowGolem extends Spell {
@@ -6,9 +6,6 @@ import net.minecraft.entity.EntityLivingBase;
import net.minecraft.item.EnumAction;
import net.minecraft.util.EnumFacing;
import net.minecraft.util.math.Vec3d;
import net.minecraft.world.World;
import javax.annotation.Nullable;
public class Tornado extends SpellConstruct<EntityTornado> {
@@ -7,6 +7,7 @@ import electroblob.wizardry.item.ItemCrystal;
import electroblob.wizardry.item.ItemSpellBook;
import electroblob.wizardry.registry.WizardryBlocks;
import electroblob.wizardry.registry.WizardryItems;
import electroblob.wizardry.util.NBTExtras;
import electroblob.wizardry.util.WandHelper;
import net.minecraft.block.Block;
import net.minecraft.entity.player.EntityPlayer;
@@ -205,7 +206,7 @@ public class TileEntityArcaneWorkbench extends TileEntity implements IInventory,
}
}
tagCompound.setTag("Inventory", itemList);
NBTExtras.storeTagSafely(tagCompound, "Inventory", itemList);
return tagCompound;
}
@@ -184,7 +184,7 @@ public class TileEntityShrineCore extends TileEntity implements ITickable {
for(EntityLivingBase entity : entities){
entity.addPotionEffect(new PotionEffect(WizardryPotions.containment, 219));
entity.getEntityData().setTag(PotionContainment.ENTITY_TAG, NBTUtil.createPosTag(this.pos));
NBTExtras.storeTagSafely(entity.getEntityData(), PotionContainment.ENTITY_TAG, NBTUtil.createPosTag(this.pos));
}
}
@@ -192,13 +192,13 @@ public class TileEntityShrineCore extends TileEntity implements ITickable {
public NBTTagCompound writeToNBT(NBTTagCompound compound){
compound.setBoolean("activated", this.activated);
if(linkedContainer != null) compound.setTag("linkedContainerPos", NBTUtil.createPosTag(linkedContainer.getPos()));
if(linkedContainer != null) NBTExtras.storeTagSafely(compound, "linkedContainerPos", NBTUtil.createPosTag(linkedContainer.getPos()));
NBTTagList tagList = new NBTTagList();
for(UUID uuid : linkedWizards){
if(uuid != null) tagList.appendTag(NBTUtil.createUUIDTag(uuid));
}
compound.setTag("wizards", tagList);
NBTExtras.storeTagSafely(compound, "wizards", tagList);
return super.writeToNBT(compound);
}
@@ -1,5 +1,6 @@
package electroblob.wizardry.tileentity;
import electroblob.wizardry.util.NBTExtras;
import electroblob.wizardry.util.WizardryUtilities;
import net.minecraft.block.Block;
import net.minecraft.entity.EntityList;
@@ -152,7 +153,7 @@ public class TileEntityStatue extends TileEntity implements ITickable {
tagCompound.setFloat("entityYawHead", creature.rotationYawHead);
tagCompound.setFloat("entityYawOffset", creature.renderYawOffset);
}
tagCompound.setTag("entity", entityCompound);
NBTExtras.storeTagSafely(tagCompound, "entity", entityCompound);
tagCompound.setInteger("timer", timer);
tagCompound.setInteger("lifetime", lifetime);
tagCompound.setBoolean("isIce", isIce);
@@ -8,7 +8,10 @@ import net.minecraft.entity.monster.*;
import net.minecraft.util.DamageSource;
import net.minecraft.util.EntityDamageSource;
import java.util.*;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Map;
// A note on the use of the vanilla damagesources:
// When using indirect damage sources, the SECOND argument is the original entity (i.e. the caster), and the
@@ -51,8 +51,8 @@ public final class NBTExtras {
for(Map.Entry<K, V> entry : map.entrySet()){
NBTTagCompound mapping = new NBTTagCompound();
mapping.setTag(keyTagName, keyFunction.apply(entry.getKey()));
mapping.setTag(valueTagName, valueFunction.apply(entry.getValue()));
NBTExtras.storeTagSafely(mapping, keyTagName, keyFunction.apply(entry.getKey()));
NBTExtras.storeTagSafely(mapping, valueTagName, valueFunction.apply(entry.getValue()));
tagList.appendTag(mapping);
}
@@ -205,6 +205,51 @@ public final class NBTExtras {
tag.removeTag(key + "Least");
}
/**
* Stores the given NBT tag inside the given NBT tag compound using the given key. Under normal circumstances, this
* is equivalent to {@link NBTTagCompound#setTag(String, NBTBase)}, but this method performs safety checks to
* prevent circular references. If storing the given tag would cause a circular reference, the tag is not stored
* and an error is printed to the console.
* @param compound The {@link NBTTagCompound} in which to store the tag.
* @param key The key to store the tag under.
* @param tag The tag to store.
*/
// This is a catch-all fix for issue #299.
public static void storeTagSafely(NBTTagCompound compound, String key, NBTBase tag){
if(compound == tag || deepContains(tag, compound)){
Wizardry.logger.error("Cannot store tag of type {} under key '{}' as it would result in a circular reference!",
NBTBase.getTypeName(tag.getId()), key);
}else{
compound.setTag(key, tag);
}
}
/**
* Recursively searches within the first NBT tag for the second NBT tag. This handles both compound and list tags.
* @param toSearch The NBT tag to search inside. If this is not a compound or list tag, this method will always
* return false.
* @param searchFor The NBT tag to search for.
* @return True if the second tag appears anywhere within the NBT tree contained within the first tag, false if not.
*/
public static boolean deepContains(NBTBase toSearch, NBTBase searchFor){
if(toSearch instanceof NBTTagCompound){
for(String subKey : ((NBTTagCompound)toSearch).getKeySet()){
NBTBase subTag = ((NBTTagCompound)toSearch).getTag(subKey);
if(subTag == searchFor || deepContains(subTag, searchFor)) return true;
}
}else if(toSearch instanceof NBTTagList){
for(NBTBase subTag : (NBTTagList)toSearch){
if(subTag == searchFor || deepContains(subTag, searchFor)) return true;
}
}
return false;
}
/**
* Returns an NBTTagCompound which contains only the given UUID, stored using
* {@link NBTTagCompound#setUniqueId(String, UUID)}. Allows for neater storage to NBTTagLists.
@@ -398,7 +398,7 @@ public final class WandHelper {
if(wand.getTagCompound() == null) wand.setTagCompound((new NBTTagCompound()));
if(!wand.getTagCompound().hasKey(UPGRADES_KEY))
wand.getTagCompound().setTag(UPGRADES_KEY, new NBTTagCompound());
NBTExtras.storeTagSafely(wand.getTagCompound(), UPGRADES_KEY, new NBTTagCompound());
NBTTagCompound upgrades = wand.getTagCompound().getCompoundTag(UPGRADES_KEY);
@@ -406,7 +406,7 @@ public final class WandHelper {
if(key != null) upgrades.setInteger(key, upgrades.getInteger(key) + 1);
wand.getTagCompound().setTag(UPGRADES_KEY, upgrades);
NBTExtras.storeTagSafely(wand.getTagCompound(), UPGRADES_KEY, upgrades);
}
/** Returns true if the given item is a valid special wand upgrade. */
@@ -3,6 +3,7 @@ package electroblob.wizardry.worldgen;
import com.google.common.math.Quantiles;
import electroblob.wizardry.Wizardry;
import electroblob.wizardry.registry.WizardryAdvancementTriggers;
import electroblob.wizardry.util.NBTExtras;
import electroblob.wizardry.util.WizardryUtilities;
import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap;
@@ -256,7 +257,7 @@ public abstract class WorldGenSurfaceStructure implements IWorldGenerator {
NBTTagCompound tag = new NBTTagCompound();
tag.setInteger("ChunkX", chunkX);
tag.setInteger("ChunkZ", chunkZ);
tag.setTag("BB", settings.getBoundingBox().toNBTTagIntArray());
NBTExtras.storeTagSafely(tag, "BB", settings.getBoundingBox().toNBTTagIntArray());
structureData.writeInstance(tag, chunkX, chunkZ);
structureData.markDirty();
}