From aa0e21a2dc24904b1358ff84b2224ab050d7a31a Mon Sep 17 00:00:00 2001 From: Nassim Jahnke Date: Sat, 12 Feb 2022 19:29:41 +0100 Subject: [PATCH] Fix PlayerProfile BukkitObject serialization, deprecate setName and setId for removal (#7471) Having a modifiable hash here is a bit flawed and most developers should never need these methods --- patches/api/Basic-PlayerProfile-API.patch | 2 + patches/server/Basic-PlayerProfile-API.patch | 128 ++++++++++++++++--- 2 files changed, 112 insertions(+), 18 deletions(-) diff --git a/patches/api/Basic-PlayerProfile-API.patch b/patches/api/Basic-PlayerProfile-API.patch index e977b2b6d9..7d141b8a1e 100644 --- a/patches/api/Basic-PlayerProfile-API.patch +++ b/patches/api/Basic-PlayerProfile-API.patch @@ -39,6 +39,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + * @return The previous Name + */ + @NotNull ++ @Deprecated(forRemoval = true) + String setName(@Nullable String name); + + /** @@ -53,6 +54,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + * @return The previous UUID + */ + @Nullable ++ @Deprecated(forRemoval = true) + UUID setId(@Nullable UUID uuid); + + /** diff --git a/patches/server/Basic-PlayerProfile-API.patch b/patches/server/Basic-PlayerProfile-API.patch index 16d131aeee..2854bdb93d 100644 --- a/patches/server/Basic-PlayerProfile-API.patch +++ b/patches/server/Basic-PlayerProfile-API.patch @@ -23,6 +23,8 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 +import net.minecraft.server.MinecraftServer; +import net.minecraft.server.players.GameProfileCache; +import org.apache.commons.lang3.Validate; ++import org.bukkit.configuration.serialization.SerializableAs; ++import org.bukkit.craftbukkit.configuration.ConfigSerializationUtil; +import org.bukkit.craftbukkit.entity.CraftPlayer; +import org.bukkit.craftbukkit.profile.CraftPlayerTextures; +import org.bukkit.craftbukkit.profile.CraftProfileProperty; @@ -34,6 +36,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 +import java.util.*; +import java.util.concurrent.CompletableFuture; + ++@SerializableAs("PlayerProfile") +public class CraftPlayerProfile implements PlayerProfile, SharedPlayerProfile { + + private GameProfile profile; @@ -92,6 +95,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + } + + @Override ++ @Deprecated(forRemoval = true) + public UUID setId(@Nullable UUID uuid) { + GameProfile prev = this.profile; + this.profile = new GameProfile(uuid, prev.getName()); @@ -111,6 +115,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + } + + @Override ++ @Deprecated(forRemoval = true) + public String setName(@Nullable String name) { + GameProfile prev = this.profile; + this.profile = new GameProfile(prev.getId(), name); @@ -163,24 +168,6 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + } + + @Override -+ public boolean equals(Object o) { -+ if (this == o) return true; -+ if (o == null || getClass() != o.getClass()) return false; -+ CraftPlayerProfile that = (CraftPlayerProfile) o; -+ return Objects.equals(profile, that.profile); -+ } -+ -+ @Override -+ public int hashCode() { -+ return profile.hashCode(); -+ } -+ -+ @Override -+ public String toString() { -+ return profile.toString(); -+ } -+ -+ @Override + public CraftPlayerProfile clone() { + CraftPlayerProfile clone = new CraftPlayerProfile(this.getId(), this.getName()); + clone.setProperties(getProperties()); @@ -328,6 +315,46 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + return map; + } + ++ public static CraftPlayerProfile deserialize(Map map) { ++ UUID uniqueId = ConfigSerializationUtil.getUuid(map, "uniqueId", true); ++ String name = ConfigSerializationUtil.getString(map, "name", true); ++ ++ // This also validates the deserialized unique id and name (ensures that not both are null): ++ CraftPlayerProfile profile = new CraftPlayerProfile(uniqueId, name); ++ ++ if (map.containsKey("properties")) { ++ for (Object propertyData : (List) map.get("properties")) { ++ if (!(propertyData instanceof Map)) { ++ throw new IllegalArgumentException("Property data (" + propertyData + ") is not a valid Map"); ++ } ++ Property property = CraftProfileProperty.deserialize((Map) propertyData); ++ profile.profile.getProperties().put(property.getName(), property); ++ } ++ } ++ ++ return profile; ++ } ++ ++ @Override ++ public boolean equals(Object obj) { ++ if (this == obj) return true; ++ if (!(obj instanceof CraftPlayerProfile otherProfile)) return false; ++ return Objects.equals(this.profile, otherProfile.profile); ++ } ++ ++ @Override ++ public String toString() { ++ return "CraftPlayerProfile [uniqueId=" + getId() + ++ ", name=" + getName() + ++ ", properties=" + org.bukkit.craftbukkit.profile.CraftPlayerProfile.toString(this.profile.getProperties()) + ++ "]"; ++ } ++ ++ @Override ++ public int hashCode() { ++ return this.profile.hashCode(); ++ } ++ + private class PropertySet extends AbstractSet { + + @Override @@ -603,6 +630,14 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 public final class CraftServer implements Server { private final String serverName = "Paper"; // Paper private final String serverVersion; +@@ -0,0 +0,0 @@ public final class CraftServer implements Server { + static { + ConfigurationSerialization.registerClass(CraftOfflinePlayer.class); + ConfigurationSerialization.registerClass(CraftPlayerProfile.class); ++ ConfigurationSerialization.registerClass(com.destroystokyo.paper.profile.CraftPlayerProfile.class); // Paper + CraftItemFactory.instance(); + } + @@ -0,0 +0,0 @@ public final class CraftServer implements Server { public boolean suggestPlayerNamesWhenNullTabCompletions() { return com.destroystokyo.paper.PaperConfig.suggestPlayersWhenNullTabCompletions; @@ -668,6 +703,63 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 } void rebuildDirtyProperties() { +@@ -0,0 +0,0 @@ public final class CraftPlayerProfile implements PlayerProfile { + return builder.toString(); + } + +- private static String toString(@Nonnull PropertyMap propertyMap) { ++ public static String toString(@Nonnull PropertyMap propertyMap) { // Paper - public + StringBuilder builder = new StringBuilder(); + builder.append("{"); + propertyMap.asMap().forEach((propertyName, properties) -> { +@@ -0,0 +0,0 @@ public final class CraftPlayerProfile implements PlayerProfile { + return true; + } + +- private static boolean equals(@Nonnull PropertyMap propertyMap, @Nonnull PropertyMap other) { ++ public static boolean equals(@Nonnull PropertyMap propertyMap, @Nonnull PropertyMap other) { // Paper - public + if (propertyMap.size() != other.size()) return false; + // We take the order of properties into account here, because it is + // also relevant in the serialized and NBT forms of GameProfiles. +@@ -0,0 +0,0 @@ public final class CraftPlayerProfile implements PlayerProfile { + return result; + } + +- private static int hashCode(PropertyMap propertyMap) { ++ public static int hashCode(PropertyMap propertyMap) { // Paper - public + int result = 1; + for (Property property : propertyMap.values()) { + result = 31 * result + CraftProfileProperty.hashCode(property); +@@ -0,0 +0,0 @@ public final class CraftPlayerProfile implements PlayerProfile { + + @Override + public Map serialize() { ++ // Paper - diff on change + Map map = new LinkedHashMap<>(); + if (this.uniqueId != null) { + map.put("uniqueId", this.uniqueId.toString()); +@@ -0,0 +0,0 @@ public final class CraftPlayerProfile implements PlayerProfile { + }); + map.put("properties", propertiesData); + } ++ // Paper - diff on change + return map; + } + + public static CraftPlayerProfile deserialize(Map map) { ++ // Paper - diff on change + UUID uniqueId = ConfigSerializationUtil.getUuid(map, "uniqueId", true); + String name = ConfigSerializationUtil.getString(map, "name", true); + +@@ -0,0 +0,0 @@ public final class CraftPlayerProfile implements PlayerProfile { + profile.properties.put(property.getName(), property); + } + } +- ++ // Paper - diff on change + return profile; + } + } diff --git a/src/main/java/org/bukkit/craftbukkit/profile/CraftPlayerTextures.java b/src/main/java/org/bukkit/craftbukkit/profile/CraftPlayerTextures.java index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 --- a/src/main/java/org/bukkit/craftbukkit/profile/CraftPlayerTextures.java