Clone mutable types in events when changes are discarded (#10333)

This commit is contained in:
Jake Potrebic 2024-03-20 13:42:29 -07:00
parent 71a2187c20
commit 3e27ced8ce
10 changed files with 163 additions and 13 deletions

View file

@ -241,7 +241,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
+ * @return {@link Location} where search begins
+ */
+ public @NotNull Location getOrigin() {
+ return this.origin;
+ return this.origin.clone();
+ }
+
+ /**

View file

@ -46,7 +46,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
+ */
+ @NotNull
+ public BlockData getChangedBlockData() {
+ return changed;
+ return changed.clone();
+ }
+ // Paper end
+

View file

@ -242,7 +242,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
+ */
+ @NotNull
+ public Location getOldCenter() {
+ return this.oldCenter;
+ return this.oldCenter.clone();
+ }
+
+ /**

View file

@ -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
+

View file

@ -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();
+ }
+
+ /**

View file

@ -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
}
/**

View file

@ -74,7 +74,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
+ */
+ @NotNull
+ public Location getLoc() {
+ return this.location;
+ return this.location.clone();
+ }
+
+ @Override

View file

@ -64,7 +64,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
+ */
+ @NotNull
+ public Location getSpawnLocation() {
+ return this.location;
+ return this.location.clone();
+ }
+
+ /**

View file

@ -42,6 +42,6 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
+
+ @NotNull
+ public Location getSpawnerLocation() {
+ return this.spawnerLocation;
+ return this.spawnerLocation.clone();
+ }
+}

View file

@ -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