Prevent internal NPE on ItemStack#damage (#10836)

ItemStack#damage internally uses ItemStack#hurtAndBreak, which
previously would call a Consumer in case the item broke.
Since 1.20.5 the break game event logic however resides in said method
and was using the equipment slot passed, which is null in the case of
the API ItemStack#damage method.

This commit prevents the NPE by first null checking the slot.
Addittionally, hurtAndBreak also now checks if the player has infinite
materials, e.g. is in creative mode, to prevent damaging the item.

As such as filter is undesirable for API calls, this commit also skips
this logic in case of an API invocation.
This commit is contained in:
Bjarne Koll 2024-06-10 08:24:52 +02:00 committed by GitHub
parent bd5867a96f
commit 906df69f05
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 38 additions and 10 deletions

View File

@ -10,8 +10,36 @@ the logic associated with damaging them
== AT ==
public net.minecraft.world.entity.LivingEntity entityEventForEquipmentBreak(Lnet/minecraft/world/entity/EquipmentSlot;)B
diff --git a/src/main/java/net/minecraft/world/item/ItemStack.java b/src/main/java/net/minecraft/world/item/ItemStack.java
index acce1b72185d4687ef5a72dd4014d87149f4b092..dc473b46394b41d951b68ed0eb6993e6807a0d29 100644
--- a/src/main/java/net/minecraft/world/item/ItemStack.java
+++ b/src/main/java/net/minecraft/world/item/ItemStack.java
@@ -713,7 +713,14 @@ public final class ItemStack implements DataComponentHolder {
return;
}
}
+ // Paper start - ItemStack damage API - split hurtAndBreak to skip pre-checks like creative mode
+ this.hurtAndBreakWithoutChecks(amount, entity, slot);
+ }
+ }
+ public void hurtAndBreakWithoutChecks(int amount, LivingEntity entity, @org.checkerframework.checker.nullness.qual.Nullable EquipmentSlot slot) {
+ {
+ // Paper end - ItemStack damage API - split hurtAndBreak to skip pre-checks like creative mode
RandomSource randomsource = entity.getRandom();
ServerPlayer entityplayer;
@@ -726,7 +733,7 @@ public final class ItemStack implements DataComponentHolder {
}
this.hurtAndBreak(amount, randomsource, entity, () -> { // Paper - Add EntityDamageItemEvent
- entity.broadcastBreakEvent(slot);
+ if (slot != null) entity.broadcastBreakEvent(slot); // Paper - ItemStack damage API - slot is nullable
Item item = this.getItem();
// CraftBukkit start - Check for item breaking
if (this.count == 1 && entity instanceof net.minecraft.world.entity.player.Player) {
diff --git a/src/main/java/org/bukkit/craftbukkit/entity/CraftLivingEntity.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftLivingEntity.java
index 2537d4d4ec1243f3b80ec13a6b863ec6de619919..f866a3c2f5ec5954bab9de5f8d3e2a8c55824805 100644
index 2537d4d4ec1243f3b80ec13a6b863ec6de619919..6cf60ff71e398dbf6110f94a00e9bb973916a7f8 100644
--- a/src/main/java/org/bukkit/craftbukkit/entity/CraftLivingEntity.java
+++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftLivingEntity.java
@@ -1103,4 +1103,48 @@ public class CraftLivingEntity extends CraftEntity implements LivingEntity {
@ -59,7 +87,7 @@ index 2537d4d4ec1243f3b80ec13a6b863ec6de619919..f866a3c2f5ec5954bab9de5f8d3e2a8c
+ }
+
+ private void damageItemStack0(final net.minecraft.world.item.ItemStack nmsStack, final int amount, final net.minecraft.world.entity.EquipmentSlot slot) {
+ nmsStack.hurtAndBreak(amount, this.getHandle(), slot);
+ nmsStack.hurtAndBreakWithoutChecks(amount, this.getHandle(), slot);
+ }
+ // Paper end - ItemStack damage API
}

View File

@ -5,7 +5,7 @@ Subject: [PATCH] Friction API
diff --git a/src/main/java/net/minecraft/world/entity/LivingEntity.java b/src/main/java/net/minecraft/world/entity/LivingEntity.java
index d3a48cdb8a6f56cefe824fac814cef58c1697c10..8a4977bf4d5b87ca30e048d749b6a878b1a17911 100644
index d736a53a6ea2a20a950096cd89df178864e644f4..fecbef1ffcc942c71e3a1daaad4c16e0c774be49 100644
--- a/src/main/java/net/minecraft/world/entity/LivingEntity.java
+++ b/src/main/java/net/minecraft/world/entity/LivingEntity.java
@@ -273,6 +273,7 @@ public abstract class LivingEntity extends Entity implements Attackable {
@ -133,11 +133,11 @@ index 1a291dd8a287db30e71dcb315599fc4b038764c4..30d62ee4d5cd2ddacb8783b5bbbf475d
public int getHealth() {
return this.getHandle().health;
diff --git a/src/main/java/org/bukkit/craftbukkit/entity/CraftLivingEntity.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftLivingEntity.java
index f866a3c2f5ec5954bab9de5f8d3e2a8c55824805..8f987c87032fad49835d51cd747f40df2bb5aa32 100644
index 6cf60ff71e398dbf6110f94a00e9bb973916a7f8..112d54e1789d30ef89b34f53a60b0124538d3cda 100644
--- a/src/main/java/org/bukkit/craftbukkit/entity/CraftLivingEntity.java
+++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftLivingEntity.java
@@ -1147,4 +1147,18 @@ public class CraftLivingEntity extends CraftEntity implements LivingEntity {
nmsStack.hurtAndBreak(amount, this.getHandle(), slot);
nmsStack.hurtAndBreakWithoutChecks(amount, this.getHandle(), slot);
}
// Paper end - ItemStack damage API
+

View File

@ -62,10 +62,10 @@ index 4f076eae3a9c597e41f4520dae8378ec429d9f69..810b6099e734a5524e696beb3f25d6ca
this.profiler.push(() -> {
String s = String.valueOf(worldserver);
diff --git a/src/main/java/net/minecraft/world/item/ItemStack.java b/src/main/java/net/minecraft/world/item/ItemStack.java
index 3e9caee7322d7ffdb93fd7d063675dddc4c71226..8e2b3dd109dca3089cbce82cd3788874613a3230 100644
index ea5a36a192258b2f147b4a820f5cc988eb0e1180..86dcad62f59f68d2eefe2d5df1cdaee0955dc6e3 100644
--- a/src/main/java/net/minecraft/world/item/ItemStack.java
+++ b/src/main/java/net/minecraft/world/item/ItemStack.java
@@ -794,10 +794,16 @@ public final class ItemStack implements DataComponentHolder {
@@ -801,10 +801,16 @@ public final class ItemStack implements DataComponentHolder {
}
public ItemStack copy() {
@ -85,7 +85,7 @@ index 3e9caee7322d7ffdb93fd7d063675dddc4c71226..8e2b3dd109dca3089cbce82cd3788874
itemstack.setPopTime(this.getPopTime());
return itemstack;
diff --git a/src/main/java/net/minecraft/world/level/block/entity/BlockEntity.java b/src/main/java/net/minecraft/world/level/block/entity/BlockEntity.java
index d2939e12449ae6b2b57beff7e689a0d39212161d..65170cbb50d8d5030fc5e33b6389c554aec6ae31 100644
index 6207c6063cd11ccb1177fe7016c49c02a3416990..139cc0123921bf981d10334d9bd7378d19ec5f3b 100644
--- a/src/main/java/net/minecraft/world/level/block/entity/BlockEntity.java
+++ b/src/main/java/net/minecraft/world/level/block/entity/BlockEntity.java
@@ -38,6 +38,7 @@ import co.aikar.timings.MinecraftTimings; // Paper

View File

@ -10,7 +10,7 @@ public org/bukkit/craftbukkit/block/CraftBlockStates getBlockState(Lorg/bukkit/W
public net/minecraft/world/level/block/entity/BlockEntity saveId(Lnet/minecraft/nbt/CompoundTag;)V
diff --git a/src/main/java/net/minecraft/world/item/ItemStack.java b/src/main/java/net/minecraft/world/item/ItemStack.java
index 8e2b3dd109dca3089cbce82cd3788874613a3230..893efb2c4a07c33d41e934279dd914a9dbd4ef79 100644
index 86dcad62f59f68d2eefe2d5df1cdaee0955dc6e3..1bd673336f13f12a875210acd23bd8496b5773ae 100644
--- a/src/main/java/net/minecraft/world/item/ItemStack.java
+++ b/src/main/java/net/minecraft/world/item/ItemStack.java
@@ -414,7 +414,7 @@ public final class ItemStack implements DataComponentHolder {
@ -22,7 +22,7 @@ index 8e2b3dd109dca3089cbce82cd3788874613a3230..893efb2c4a07c33d41e934279dd914a9
int newCount = this.getCount();
this.setCount(oldCount);
this.restorePatch(oldData);
@@ -1251,6 +1251,11 @@ public final class ItemStack implements DataComponentHolder {
@@ -1258,6 +1258,11 @@ public final class ItemStack implements DataComponentHolder {
public void setItem(Item item) {
this.bukkitStack = null; // Paper
this.item = item;