From 3e27ced8cea66387463ed71bd3878ea51b98adb9 Mon Sep 17 00:00:00 2001 From: Jake Potrebic <jake.m.potrebic@gmail.com> Date: Wed, 20 Mar 2024 13:42:29 -0700 Subject: [PATCH] Clone mutable types in events when changes are discarded (#10333) --- patches/api/Add-StructuresLocateEvent.patch | 2 +- ...-constructor-and-getChangedBlockData.patch | 2 +- patches/api/Add-worldborder-events.patch | 2 +- patches/api/AsyncTabCompleteEvent.patch | 4 +- patches/api/BlockDestroyEvent.patch | 5 +- ...utables-to-prevent-unexpected-issues.patch | 151 ++++++++++++++++++ patches/api/EntityPathfindEvent.patch | 2 +- patches/api/PreCreatureSpawnEvent.patch | 2 +- patches/api/PreSpawnerSpawnEvent.patch | 2 +- patches/api/Turtle-API.patch | 4 +- 10 files changed, 163 insertions(+), 13 deletions(-) create mode 100644 patches/api/Clone-mutables-to-prevent-unexpected-issues.patch diff --git a/patches/api/Add-StructuresLocateEvent.patch b/patches/api/Add-StructuresLocateEvent.patch index 0a3724585f..e67d4f1dbb 100644 --- a/patches/api/Add-StructuresLocateEvent.patch +++ b/patches/api/Add-StructuresLocateEvent.patch @@ -241,7 +241,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + * @return {@link Location} where search begins + */ + public @NotNull Location getOrigin() { -+ return this.origin; ++ return this.origin.clone(); + } + + /** diff --git a/patches/api/Add-source-block-constructor-and-getChangedBlockData.patch b/patches/api/Add-source-block-constructor-and-getChangedBlockData.patch index e098261ce0..790a6ca02b 100644 --- a/patches/api/Add-source-block-constructor-and-getChangedBlockData.patch +++ b/patches/api/Add-source-block-constructor-and-getChangedBlockData.patch @@ -46,7 +46,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + */ + @NotNull + public BlockData getChangedBlockData() { -+ return changed; ++ return changed.clone(); + } + // Paper end + diff --git a/patches/api/Add-worldborder-events.patch b/patches/api/Add-worldborder-events.patch index 930584fce3..273da4b0e9 100644 --- a/patches/api/Add-worldborder-events.patch +++ b/patches/api/Add-worldborder-events.patch @@ -242,7 +242,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + */ + @NotNull + public Location getOldCenter() { -+ return this.oldCenter; ++ return this.oldCenter.clone(); + } + + /** diff --git a/patches/api/AsyncTabCompleteEvent.patch b/patches/api/AsyncTabCompleteEvent.patch index f3ff39f75e..106bd0371c 100644 --- a/patches/api/AsyncTabCompleteEvent.patch +++ b/patches/api/AsyncTabCompleteEvent.patch @@ -208,7 +208,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + */ + @Nullable + public Location getLocation() { -+ return this.location; ++ return this.location != null ? this.location.clone() : null; + } + + /** @@ -570,7 +570,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + */ + @org.jetbrains.annotations.Nullable + public org.bukkit.Location getLocation() { -+ return loc; ++ return this.loc != null ? this.loc.clone() : null; + } + // Paper end + diff --git a/patches/api/BlockDestroyEvent.patch b/patches/api/BlockDestroyEvent.patch index b34e128ad2..d9a5a744a4 100644 --- a/patches/api/BlockDestroyEvent.patch +++ b/patches/api/BlockDestroyEvent.patch @@ -79,9 +79,8 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + /** + * @return The new state of this block (Air, or a Fluid type) + */ -+ @NotNull -+ public BlockData getNewState() { -+ return this.newState; ++ public @NotNull BlockData getNewState() { ++ return this.newState.clone(); + } + + /** diff --git a/patches/api/Clone-mutables-to-prevent-unexpected-issues.patch b/patches/api/Clone-mutables-to-prevent-unexpected-issues.patch new file mode 100644 index 0000000000..172c7dfef5 --- /dev/null +++ b/patches/api/Clone-mutables-to-prevent-unexpected-issues.patch @@ -0,0 +1,151 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Jake Potrebic <jake.m.potrebic@gmail.com> +Date: Sat, 16 Mar 2024 11:10:48 -0700 +Subject: [PATCH] Clone mutables to prevent unexpected issues + +There are lots of locations in the API where mutable +types are not cloned, either on return or when passed +as a parameter and assigned to a field, which can cause +unexpected behaviors. Let this be a lesson to use +immutable types for simple things Location, Vector, and +others. + +diff --git a/src/main/java/org/bukkit/event/block/BlockCanBuildEvent.java b/src/main/java/org/bukkit/event/block/BlockCanBuildEvent.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/org/bukkit/event/block/BlockCanBuildEvent.java ++++ b/src/main/java/org/bukkit/event/block/BlockCanBuildEvent.java +@@ -0,0 +0,0 @@ public class BlockCanBuildEvent extends BlockEvent { + */ + @NotNull + public BlockData getBlockData() { +- return blockData; ++ return blockData.clone(); // Paper - clone because mutation isn't used + } + + /** +diff --git a/src/main/java/org/bukkit/event/entity/EntityChangeBlockEvent.java b/src/main/java/org/bukkit/event/entity/EntityChangeBlockEvent.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/org/bukkit/event/entity/EntityChangeBlockEvent.java ++++ b/src/main/java/org/bukkit/event/entity/EntityChangeBlockEvent.java +@@ -0,0 +0,0 @@ public class EntityChangeBlockEvent extends EntityEvent implements Cancellable { + */ + @NotNull + public BlockData getBlockData() { +- return to; ++ return to.clone(); // Paper - clone because mutation isn't used + } + + @NotNull +diff --git a/src/main/java/org/bukkit/event/entity/EntityExplodeEvent.java b/src/main/java/org/bukkit/event/entity/EntityExplodeEvent.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/org/bukkit/event/entity/EntityExplodeEvent.java ++++ b/src/main/java/org/bukkit/event/entity/EntityExplodeEvent.java +@@ -0,0 +0,0 @@ public class EntityExplodeEvent extends EntityEvent implements Cancellable { + */ + @NotNull + public Location getLocation() { +- return location; ++ return location.clone(); // Paper - clone to avoid changes + } + + /** +diff --git a/src/main/java/org/bukkit/event/entity/EntityPortalEnterEvent.java b/src/main/java/org/bukkit/event/entity/EntityPortalEnterEvent.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/org/bukkit/event/entity/EntityPortalEnterEvent.java ++++ b/src/main/java/org/bukkit/event/entity/EntityPortalEnterEvent.java +@@ -0,0 +0,0 @@ public class EntityPortalEnterEvent extends EntityEvent { + */ + @NotNull + public Location getLocation() { +- return location; ++ return location.clone(); // Paper - clone to avoid changes + } + + @NotNull +diff --git a/src/main/java/org/bukkit/event/entity/ItemDespawnEvent.java b/src/main/java/org/bukkit/event/entity/ItemDespawnEvent.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/org/bukkit/event/entity/ItemDespawnEvent.java ++++ b/src/main/java/org/bukkit/event/entity/ItemDespawnEvent.java +@@ -0,0 +0,0 @@ public class ItemDespawnEvent extends EntityEvent implements Cancellable { + */ + @NotNull + public Location getLocation() { +- return location; ++ return location.clone(); // Paper - clone to avoid changes + } + + @NotNull +diff --git a/src/main/java/org/bukkit/event/vehicle/VehicleBlockCollisionEvent.java b/src/main/java/org/bukkit/event/vehicle/VehicleBlockCollisionEvent.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/org/bukkit/event/vehicle/VehicleBlockCollisionEvent.java ++++ b/src/main/java/org/bukkit/event/vehicle/VehicleBlockCollisionEvent.java +@@ -0,0 +0,0 @@ public class VehicleBlockCollisionEvent extends VehicleCollisionEvent { + */ + @NotNull + public org.bukkit.util.Vector getVelocity() { +- return velocity; ++ return velocity.clone(); + } + // Paper end + +diff --git a/src/main/java/org/bukkit/event/vehicle/VehicleMoveEvent.java b/src/main/java/org/bukkit/event/vehicle/VehicleMoveEvent.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/org/bukkit/event/vehicle/VehicleMoveEvent.java ++++ b/src/main/java/org/bukkit/event/vehicle/VehicleMoveEvent.java +@@ -0,0 +0,0 @@ public class VehicleMoveEvent extends VehicleEvent { + */ + @NotNull + public Location getFrom() { +- return from; ++ return from.clone(); // Paper - clone to avoid changes + } + + /** +@@ -0,0 +0,0 @@ public class VehicleMoveEvent extends VehicleEvent { + */ + @NotNull + public Location getTo() { +- return to; ++ return to.clone(); // Paper - clone to avoid changes + } + + +diff --git a/src/main/java/org/bukkit/event/world/GenericGameEvent.java b/src/main/java/org/bukkit/event/world/GenericGameEvent.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/org/bukkit/event/world/GenericGameEvent.java ++++ b/src/main/java/org/bukkit/event/world/GenericGameEvent.java +@@ -0,0 +0,0 @@ public class GenericGameEvent extends WorldEvent implements Cancellable { + */ + @NotNull + public Location getLocation() { +- return location; ++ return location.clone(); // Paper - clone to avoid changes + } + + /** +diff --git a/src/main/java/org/bukkit/event/world/SpawnChangeEvent.java b/src/main/java/org/bukkit/event/world/SpawnChangeEvent.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/org/bukkit/event/world/SpawnChangeEvent.java ++++ b/src/main/java/org/bukkit/event/world/SpawnChangeEvent.java +@@ -0,0 +0,0 @@ public class SpawnChangeEvent extends WorldEvent { + */ + @NotNull + public Location getPreviousLocation() { +- return previousLocation; ++ return previousLocation.clone(); // Paper - clone to avoid changes + } + + @NotNull +diff --git a/src/main/java/org/bukkit/event/world/StructureGrowEvent.java b/src/main/java/org/bukkit/event/world/StructureGrowEvent.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/org/bukkit/event/world/StructureGrowEvent.java ++++ b/src/main/java/org/bukkit/event/world/StructureGrowEvent.java +@@ -0,0 +0,0 @@ public class StructureGrowEvent extends WorldEvent implements Cancellable { + */ + @NotNull + public Location getLocation() { +- return location; ++ return location.clone(); // Paper - clone to avoid changes + } + + /** diff --git a/patches/api/EntityPathfindEvent.patch b/patches/api/EntityPathfindEvent.patch index ebbdee4fd7..bc48f8deb6 100644 --- a/patches/api/EntityPathfindEvent.patch +++ b/patches/api/EntityPathfindEvent.patch @@ -74,7 +74,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + */ + @NotNull + public Location getLoc() { -+ return this.location; ++ return this.location.clone(); + } + + @Override diff --git a/patches/api/PreCreatureSpawnEvent.patch b/patches/api/PreCreatureSpawnEvent.patch index 163c736bbb..682486ea45 100644 --- a/patches/api/PreCreatureSpawnEvent.patch +++ b/patches/api/PreCreatureSpawnEvent.patch @@ -64,7 +64,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + */ + @NotNull + public Location getSpawnLocation() { -+ return this.location; ++ return this.location.clone(); + } + + /** diff --git a/patches/api/PreSpawnerSpawnEvent.patch b/patches/api/PreSpawnerSpawnEvent.patch index 2e38fd0699..8b692cb5c2 100644 --- a/patches/api/PreSpawnerSpawnEvent.patch +++ b/patches/api/PreSpawnerSpawnEvent.patch @@ -42,6 +42,6 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + + @NotNull + public Location getSpawnerLocation() { -+ return this.spawnerLocation; ++ return this.spawnerLocation.clone(); + } +} diff --git a/patches/api/Turtle-API.patch b/patches/api/Turtle-API.patch index 5a14e1413d..10462b727d 100644 --- a/patches/api/Turtle-API.patch +++ b/patches/api/Turtle-API.patch @@ -116,7 +116,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + */ + @NotNull + public Location getLocation() { -+ return this.location; ++ return this.location.clone(); + } + + /** @@ -210,7 +210,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + */ + @NotNull + public Location getLocation() { -+ return this.location; ++ return this.location.clone(); + } + + @Override