From bca080be7c9765b90fe696f5570d91410f6883d3 Mon Sep 17 00:00:00 2001 From: Adam Date: Mon, 15 Jul 2024 10:01:42 -0400 Subject: [PATCH] Restrict view-only further (#226) * Top inventory now rejects edits entirely * This prevents potential issues with plugins ignoring cancelled event status and then doing a strange mix of mismatched queries (like getItem into setContents). * It is not feasible/worthwhile to do this with the bottom inventory; many plugins don't use `InventoryView#getBottomInventory`. They use `Player#getInventory` instead. --- .../v1_21_R1/container/OpenContainerMenu.java | 31 +++- .../v1_21_R1/container/OpenEnderChest.java | 4 + .../container/OpenEnderChestMenu.java | 64 ++++--- .../v1_21_R1/container/OpenInventoryMenu.java | 53 +++--- .../container/OpenPlayerInventory.java | 2 +- .../v1_21_R1/container/OpenViewInventory.java | 157 ++++++++++++++++++ 6 files changed, 263 insertions(+), 48 deletions(-) create mode 100644 internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenViewInventory.java diff --git a/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenContainerMenu.java b/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenContainerMenu.java index 347cce48..64a42e8c 100644 --- a/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenContainerMenu.java +++ b/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenContainerMenu.java @@ -2,9 +2,13 @@ import com.google.common.base.Suppliers; import com.lishid.openinv.internal.v1_21_R1.container.slot.SlotPlaceholder; +import com.lishid.openinv.internal.v1_21_R1.container.slot.SlotViewOnly; import it.unimi.dsi.fastutil.ints.IntArrayList; import it.unimi.dsi.fastutil.ints.IntList; +import net.minecraft.server.level.ServerPlayer; +import net.minecraft.world.entity.player.Player; import net.minecraft.world.inventory.AbstractContainerMenu; +import net.minecraft.world.inventory.ClickType; import net.minecraft.world.inventory.ContainerData; import net.minecraft.world.inventory.ContainerListener; import net.minecraft.world.inventory.ContainerSynchronizer; @@ -24,6 +28,10 @@ */ public abstract class OpenContainerMenu extends AbstractContainerMenu { + protected final ServerPlayer viewer; + protected final boolean viewOnly; + protected final boolean ownContainer; + // Syncher fields private @Nullable ContainerSynchronizer synchronizer; private final List dataSlots = new ArrayList<>(); private final IntList remoteDataSlots = new IntArrayList(); @@ -31,10 +39,15 @@ public abstract class OpenContainerMenu extends AbstractContainerMenu { private ItemStack remoteCarried = ItemStack.EMPTY; private boolean suppressRemoteUpdates; - protected OpenContainerMenu(@Nullable MenuType containers, int containerCounter) { + protected OpenContainerMenu(@Nullable MenuType containers, int containerCounter,ServerPlayer owner, ServerPlayer viewer) { super(containers, containerCounter); + this.viewer = viewer; + ownContainer = owner.equals(viewer); + viewOnly = checkViewOnly(); } + protected abstract boolean checkViewOnly(); + static @NotNull MenuType getContainers(int inventorySize) { return switch (inventorySize) { case 9 -> MenuType.GENERIC_9x1; @@ -128,16 +141,30 @@ private static boolean addToExistingStack(ItemStack itemStack, Slot slot) { return true; } - // Overrides from here on are purely to modify the sync process to send placeholder items. + @Override + public void clicked(int i, int j, ClickType clickType, Player player) { + if (viewOnly) { + if (clickType == ClickType.QUICK_CRAFT) { + sendAllDataToRemote(); + } + return; + } + super.clicked(i, j, clickType, player); + } + @Override protected Slot addSlot(Slot slot) { slot.index = this.slots.size(); + if (viewOnly && !(slot instanceof SlotViewOnly)) { + slot = SlotViewOnly.wrap(slot); + } this.slots.add(slot); this.lastSlots.add(ItemStack.EMPTY); this.remoteSlots.add(ItemStack.EMPTY); return slot; } + // Overrides from here on are purely to modify the sync process to send placeholder items. @Override protected DataSlot addDataSlot(DataSlot dataSlot) { this.dataSlots.add(dataSlot); diff --git a/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenEnderChest.java b/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenEnderChest.java index 6959e8f4..fa038e7e 100644 --- a/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenEnderChest.java +++ b/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenEnderChest.java @@ -38,6 +38,10 @@ public OpenEnderChest(@NotNull org.bukkit.entity.Player player) { this.items = owner.getEnderChestInventory().items; } + public @NotNull ServerPlayer getOwnerHandle() { + return owner; + } + @Override public @NotNull org.bukkit.inventory.Inventory getBukkitInventory() { if (inventory == null) { diff --git a/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenEnderChestMenu.java b/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenEnderChestMenu.java index 7bfb4b3a..cbd27eb9 100644 --- a/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenEnderChestMenu.java +++ b/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenEnderChestMenu.java @@ -8,24 +8,21 @@ import net.minecraft.world.inventory.Slot; import net.minecraft.world.item.ItemStack; import org.bukkit.craftbukkit.v1_21_R1.inventory.CraftInventoryView; +import org.bukkit.event.inventory.InventoryType; +import org.bukkit.inventory.Inventory; +import org.bukkit.inventory.InventoryView; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; class OpenEnderChestMenu extends OpenContainerMenu { private final OpenEnderChest enderChest; - private final boolean viewOnly; - private final ServerPlayer viewer; private final int topSize; private CraftInventoryView view; OpenEnderChestMenu(OpenEnderChest enderChest, ServerPlayer viewer, int containerId) { - super(getMenuType(enderChest), containerId); + super(getMenuType(enderChest), containerId, enderChest.getOwnerHandle(), viewer); this.enderChest = enderChest; - this.viewer = viewer; - var bukkitViewer = viewer.getBukkitEntity(); - viewOnly = !(bukkitViewer.equals(enderChest.getPlayer()) - ? Permissions.ENDERCHEST_EDIT_SELF - : Permissions.ENDERCHEST_OPEN_OTHER) - .hasPermission(bukkitViewer); int upperRows = (int) Math.ceil(enderChest.getContainerSize() / 9.0); topSize = upperRows * 9; @@ -70,24 +67,47 @@ private static MenuType getMenuType(OpenEnderChest enderChest) { } @Override - public CraftInventoryView getBukkitView() { - if (view == null) { - view = new CraftInventoryView(viewer.getBukkitEntity(), enderChest.getBukkitInventory(), this); - } - return view; + protected boolean checkViewOnly() { + return !(ownContainer ? Permissions.ENDERCHEST_EDIT_SELF : Permissions.ENDERCHEST_OPEN_OTHER) + .hasPermission(viewer.getBukkitEntity()); } @Override - protected Slot addSlot(Slot slot) { - slot = super.addSlot(slot); + public CraftInventoryView getBukkitView() { + if (view == null) { + Inventory top; + if (viewOnly) { + top = new OpenViewInventory(enderChest); + } else { + top = enderChest.getBukkitInventory(); + } + view = new CraftInventoryView(viewer.getBukkitEntity(), top, this) { + @Override + public @Nullable Inventory getInventory(int rawSlot) { + if (viewOnly) { + return null; + } + return super.getInventory(rawSlot); + } - // If view-only and slot is in upper inventory, wrap it. - if (viewOnly && slot.index < enderChest.getContainerSize()) { - slot = SlotViewOnly.wrap(slot); - slots.set(slot.index, slot); - } + @Override + public int convertSlot(int rawSlot) { + if (viewOnly) { + return InventoryView.OUTSIDE; + } + return super.convertSlot(rawSlot); + } - return slot; + @Override + public @NotNull InventoryType.SlotType getSlotType(int slot) { + if (viewOnly) { + return InventoryType.SlotType.OUTSIDE; + } + return super.getSlotType(slot); + } + }; + } + return view; } @Override diff --git a/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenInventoryMenu.java b/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenInventoryMenu.java index 02317ec3..15bc17a8 100644 --- a/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenInventoryMenu.java +++ b/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenInventoryMenu.java @@ -21,20 +21,16 @@ public class OpenInventoryMenu extends OpenContainerMenu { private final OpenInventory inventory; - private final ServerPlayer viewer; private final int topSize; private final int offset; - private final boolean ownInv; private CraftInventoryView bukkitEntity; protected OpenInventoryMenu(OpenInventory inventory, ServerPlayer viewer, int i) { - super(getMenuType(inventory, viewer), i); + super(getMenuType(inventory, viewer), i, inventory.getOwnerHandle(), viewer); this.inventory = inventory; - this.viewer = viewer; - ownInv = inventory.getOwnerHandle().equals(viewer); int upperRows; - if (ownInv) { + if (ownContainer) { // Disallow duplicate access to own main inventory contents. offset = viewer.getInventory().items.size(); upperRows = ((int) Math.ceil((inventory.getContainerSize() - offset) / 9.0)); @@ -43,9 +39,6 @@ protected OpenInventoryMenu(OpenInventory inventory, ServerPlayer viewer, int i) upperRows = inventory.getContainerSize() / 9; } - boolean viewOnly = !(ownInv ? Permissions.INVENTORY_EDIT_SELF : Permissions.INVENTORY_EDIT_OTHER) - .hasPermission(viewer.getBukkitEntity()); - // View's upper inventory - our container for (int row = 0; row < upperRows; ++row) { for (int col = 0; col < 9; ++col) { @@ -61,7 +54,7 @@ protected OpenInventoryMenu(OpenInventory inventory, ServerPlayer viewer, int i) continue; } - Slot slot = getUpperSlot(index, x, y, ownInv, viewOnly); + Slot slot = getUpperSlot(index, x, y); addSlot(slot); } @@ -86,7 +79,17 @@ protected OpenInventoryMenu(OpenInventory inventory, ServerPlayer viewer, int i) this.topSize = slots.size() - 36; } - private Slot getUpperSlot(int index, int x, int y, boolean ownInv, boolean viewOnly) { + private static MenuType getMenuType(OpenInventory inventory, ServerPlayer viewer) { + int size = inventory.getContainerSize(); + if (inventory.getOwnerHandle().equals(viewer)) { + size -= viewer.getInventory().items.size(); + size = ((int) Math.ceil(size / 9.0)) * 9; + } + + return OpenContainerMenu.getContainers(size); + } + + private Slot getUpperSlot(int index, int x, int y) { Slot slot = inventory.getMenuSlot(index, x, y); // If the slot is cannot be interacted with there's nothing to configure. @@ -124,7 +127,7 @@ private Slot getUpperSlot(int index, int x, int y, boolean ownInv, boolean viewO } // When viewing own inventory, only allow access to equipment and drop slots (equipment allowed above). - if (ownInv && !(slot instanceof ContentDrop.SlotDrop)) { + if (ownContainer && !(slot instanceof ContentDrop.SlotDrop)) { return new SlotViewOnly(inventory, index, x, y); } @@ -135,21 +138,19 @@ private Slot getUpperSlot(int index, int x, int y, boolean ownInv, boolean viewO return slot; } - private static MenuType getMenuType(OpenInventory inventory, ServerPlayer viewer) { - int size = inventory.getContainerSize(); - if (inventory.getOwnerHandle().equals(viewer)) { - size -= viewer.getInventory().items.size(); - size = ((int) Math.ceil(size / 9.0)) * 9; - } - - return OpenContainerMenu.getContainers(size); + @Override + protected boolean checkViewOnly() { + return !(ownContainer ? Permissions.INVENTORY_EDIT_SELF : Permissions.INVENTORY_EDIT_OTHER) + .hasPermission(viewer.getBukkitEntity()); } @Override public CraftInventoryView getBukkitView() { if (bukkitEntity == null) { org.bukkit.inventory.Inventory bukkitInventory; - if (ownInv) { + if (viewOnly) { + bukkitInventory = new OpenViewInventory(inventory); + } else if (ownContainer) { bukkitInventory = new OpenPlayerInventorySelf(inventory, offset); } else { bukkitInventory = inventory.getBukkitInventory(); @@ -158,7 +159,7 @@ public CraftInventoryView getBukkitView() { bukkitEntity = new CraftInventoryView(viewer.getBukkitEntity(), bukkitInventory, this) { @Override public org.bukkit.inventory.ItemStack getItem(int index) { - if (index < 0) { + if (viewOnly || index < 0) { return null; } @@ -173,6 +174,9 @@ public boolean isInTop(int rawSlot) { @Override public @Nullable Inventory getInventory(int rawSlot) { + if (viewOnly) { + return null; + } if (rawSlot < 0) { return super.getInventory(rawSlot); } @@ -188,6 +192,9 @@ public boolean isInTop(int rawSlot) { @Override public int convertSlot(int rawSlot) { + if (viewOnly) { + return InventoryView.OUTSIDE; + } if (rawSlot < 0) { return rawSlot; } @@ -203,7 +210,7 @@ public int convertSlot(int rawSlot) { @Override public @NotNull InventoryType.SlotType getSlotType(int slot) { - if (slot < 0) { + if (viewOnly || slot < 0) { return InventoryType.SlotType.OUTSIDE; } if (slot >= topSize) { diff --git a/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenPlayerInventory.java b/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenPlayerInventory.java index 5295a625..196ee7d1 100644 --- a/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenPlayerInventory.java +++ b/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenPlayerInventory.java @@ -20,7 +20,7 @@ public OpenPlayerInventory(@NotNull OpenInventory inventory) { } @Override - public OpenInventory getInventory() { + public @NotNull OpenInventory getInventory() { return (OpenInventory) super.getInventory(); } diff --git a/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenViewInventory.java b/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenViewInventory.java new file mode 100644 index 00000000..b714a287 --- /dev/null +++ b/internal/v1_21_R1/src/main/java/com/lishid/openinv/internal/v1_21_R1/container/OpenViewInventory.java @@ -0,0 +1,157 @@ +package com.lishid.openinv.internal.v1_21_R1.container; + +import net.minecraft.world.Container; +import org.bukkit.Material; +import org.bukkit.craftbukkit.v1_21_R1.inventory.CraftInventory; +import org.bukkit.inventory.ItemStack; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Collections; +import java.util.HashMap; +import java.util.ListIterator; + +/** + * A locked down "empty" inventory that rejects plugin interaction. + */ +public class OpenViewInventory extends CraftInventory { + + public OpenViewInventory(Container inventory) { + super(inventory); + } + + @Override + public @Nullable ItemStack getItem(int index) { + return null; + } + + @Override + public void setItem(int index, @Nullable ItemStack item) { + + } + + @Override + public @NotNull HashMap addItem(@NotNull ItemStack... items) throws IllegalArgumentException { + return arrayToHashMap(items); + } + + @Override + public @NotNull HashMap removeItem(@NotNull ItemStack... items) throws IllegalArgumentException { + return arrayToHashMap(items); + } + + private static @NotNull HashMap arrayToHashMap(@NotNull ItemStack[] items) { + HashMap ignored = new HashMap<>(); + for (int index = 0; index < items.length; ++index) { + ignored.put(index, items[index]); + } + return ignored; + } + + @Override + public ItemStack[] getContents() { + return new ItemStack[getSize()]; + } + + @Override + public void setContents(@NotNull ItemStack[] items) throws IllegalArgumentException { + + } + + @Override + public @NotNull ItemStack[] getStorageContents() { + return new ItemStack[getSize()]; + } + + @Override + public void setStorageContents(@NotNull ItemStack[] items) throws IllegalArgumentException { + + } + + @Override + public boolean contains(@NotNull Material material) throws IllegalArgumentException { + return false; + } + + @Override + public boolean contains(@Nullable ItemStack item) { + return false; + } + + @Override + public boolean contains(@NotNull Material material, int amount) throws IllegalArgumentException { + return false; + } + + @Override + public boolean contains(@Nullable ItemStack item, int amount) { + return false; + } + + @Override + public boolean containsAtLeast(@Nullable ItemStack item, int amount) { + return false; + } + + @Override + public @NotNull HashMap all( + @NotNull Material material) throws IllegalArgumentException { + return new HashMap<>(); + } + + @Override + public @NotNull HashMap all(@Nullable ItemStack item) { + return new HashMap<>(); + } + + @Override + public int first(@NotNull Material material) throws IllegalArgumentException { + return -1; + } + + @Override + public int first(@NotNull ItemStack item) { + return -1; + } + + @Override + public int firstEmpty() { + return -1; + } + + @Override + public boolean isEmpty() { + return true; + } + + @Override + public void remove(@NotNull Material material) throws IllegalArgumentException { + + } + + @Override + public void remove(@NotNull ItemStack item) { + + } + + @Override + public void clear(int index) { + + } + + @Override + public void clear() { + + } + + @Override + public @NotNull ListIterator iterator() { + return Collections.emptyListIterator(); + } + + @Override + public @NotNull ListIterator iterator(int index) { + return Collections.emptyListIterator(); + } + +}