Change ItemFrame to actually provide a defensive copy. Fixes BUKKIT-2784

If a defensive copy is not used in the API, changes to the item are
reflected in memory, but never updated to the client. It also goes
against the general contract provided in Bukkit, where setItem should be
the only way to change the underlying item frame.

2 files changed, 14 insertions, 1 deletions

diff --git a/src/main/java/org/bukkit/craftbukkit/entity/CraftItemFrame.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftItemFrame.java
index 94c4a12e..e6958b0f 100644
--- a/src/main/java/org/bukkit/craftbukkit/entity/CraftItemFrame.java
+++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftItemFrame.java
@@ -27,7 +27,7 @@ public class CraftItemFrame extends CraftHanging implements ItemFrame {
 
     public org.bukkit.inventory.ItemStack getItem() {
         ItemStack i = getHandle().i();
-        return i == null ? new org.bukkit.inventory.ItemStack(Material.AIR) : new CraftItemStack(i);
+        return i == null ? new org.bukkit.inventory.ItemStack(Material.AIR) : CraftItemStack.asBukkitStack(i);
     }
 
     public Rotation getRotation() {
diff --git a/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java b/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java
index 9f7f0045..5d5ae069 100644
--- a/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java
+++ b/src/main/java/org/bukkit/craftbukkit/inventory/CraftItemStack.java
@@ -169,6 +169,10 @@ public class CraftItemStack extends ItemStack {
 
     @Override
     public Map<Enchantment, Integer> getEnchantments() {
+        return getEnchantments(item);
+    }
+
+    public static Map<Enchantment, Integer> getEnchantments(net.minecraft.server.ItemStack item) {
         Map<Enchantment, Integer> result = new HashMap<Enchantment, Integer>();
         NBTTagList list = (item == null) ? null : item.getEnchantments();
 
@@ -233,4 +237,13 @@ public class CraftItemStack extends ItemStack {
         }
         return new CraftItemStack(original).getHandle();
     }
+
+    /**
+     * Copies the NMS stack to return as a strictly-Bukkit stack
+     */
+    public static ItemStack asBukkitStack(net.minecraft.server.ItemStack original) {
+        ItemStack stack = new ItemStack(original.id, original.count, (short) original.getData());
+        stack.addUnsafeEnchantments(getEnchantments(original));
+        return stack;
+    }
 }