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.

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() {

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;
+    }
 }