From 4bee15956abb6c17831984e59fbf603686f18137 Mon Sep 17 00:00:00 2001 From: Josh Roy <10731363+JRoy@users.noreply.github.com> Date: Sat, 28 Aug 2021 09:32:45 -0700 Subject: [PATCH] Fix economy usernames being unsanitized in some places (#4484) Co-authored-by: MD <1917406+mdcfe@users.noreply.github.com> This PR fixes various issues with NPC accounts: - Fixes some NPC account names not being sanitised - Fixes wrong keys being used when manually generating a NPC account file - Adds some debug logging to `UserMap` name lookups --- .../java/com/earth2me/essentials/UserData.java | 2 +- .../java/com/earth2me/essentials/UserMap.java | 16 +++++++++++++++- .../com/earth2me/essentials/api/Economy.java | 6 +++--- .../config/EssentialsUserConfiguration.java | 2 +- .../economy/vault/VaultEconomyProvider.java | 5 +++-- 5 files changed, 23 insertions(+), 8 deletions(-) diff --git a/Essentials/src/main/java/com/earth2me/essentials/UserData.java b/Essentials/src/main/java/com/earth2me/essentials/UserData.java index f2128b5ad..313b984cc 100644 --- a/Essentials/src/main/java/com/earth2me/essentials/UserData.java +++ b/Essentials/src/main/java/com/earth2me/essentials/UserData.java @@ -69,7 +69,7 @@ public abstract class UserData extends PlayerExtension implements IConf { if (config.getUsername() != null) { ess.getUserMap().removeUser(config.getUsername()); if (isNPC()) { - final String uuid = UUID.nameUUIDFromBytes(("NPC:" + config.getUsername()).getBytes(Charsets.UTF_8)).toString(); + final String uuid = UUID.nameUUIDFromBytes(("NPC:" + StringUtil.safeString(config.getUsername())).getBytes(Charsets.UTF_8)).toString(); ess.getUserMap().removeUserUUID(uuid); } } diff --git a/Essentials/src/main/java/com/earth2me/essentials/UserMap.java b/Essentials/src/main/java/com/earth2me/essentials/UserMap.java index 895401f94..35f5eefe6 100644 --- a/Essentials/src/main/java/com/earth2me/essentials/UserMap.java +++ b/Essentials/src/main/java/com/earth2me/essentials/UserMap.java @@ -92,13 +92,21 @@ public class UserMap extends CacheLoader implements IConf { } public User getUser(final String name) { + final String sanitizedName = StringUtil.safeString(name); try { - final String sanitizedName = StringUtil.safeString(name); + if (ess.getSettings().isDebug()) { + ess.getLogger().warning("Looking up username " + name + " (" + sanitizedName + ") ..."); + } + if (names.containsKey(sanitizedName)) { final UUID uuid = names.get(sanitizedName); return getUser(uuid); } + if (ess.getSettings().isDebug()) { + ess.getLogger().warning(name + "(" + sanitizedName + ") has no known usermap entry"); + } + final File userFile = getUserFileFromString(sanitizedName); if (userFile.exists()) { ess.getLogger().info("Importing user " + name + " to usermap."); @@ -108,6 +116,9 @@ public class UserMap extends CacheLoader implements IConf { } return null; } catch (final UncheckedExecutionException ex) { + if (ess.getSettings().isDebug()) { + ess.getLogger().log(Level.WARNING, ex, () -> String.format("Exception while getting user for %s (%s)", name, sanitizedName)); + } return null; } } @@ -120,6 +131,9 @@ public class UserMap extends CacheLoader implements IConf { return legacyCacheGet(uuid); } } catch (final ExecutionException | UncheckedExecutionException ex) { + if (ess.getSettings().isDebug()) { + ess.getLogger().log(Level.WARNING, ex, () -> "Exception while getting user for " + uuid); + } return null; } } diff --git a/Essentials/src/main/java/com/earth2me/essentials/api/Economy.java b/Essentials/src/main/java/com/earth2me/essentials/api/Economy.java index d0a12b77a..dd7ae19f7 100644 --- a/Essentials/src/main/java/com/earth2me/essentials/api/Economy.java +++ b/Essentials/src/main/java/com/earth2me/essentials/api/Economy.java @@ -52,7 +52,7 @@ public class Economy { } } final UUID npcUUID = UUID.nameUUIDFromBytes(("NPC:" + name).getBytes(Charsets.UTF_8)); - final File npcFile = new File(folder, npcUUID.toString() + ".yml"); + final File npcFile = new File(folder, npcUUID + ".yml"); if (npcFile.exists()) { LOGGER.log(Level.SEVERE, MessageFormat.format(WARN_NPC_RECREATE_1, name, npcUUID.toString()), new RuntimeException()); LOGGER.log(Level.SEVERE, WARN_NPC_RECREATE_2); @@ -60,7 +60,7 @@ public class Economy { final EssentialsUserConfiguration npcConfig = new EssentialsUserConfiguration(name, npcUUID, npcFile); npcConfig.load(); npcConfig.setProperty("npc", true); - npcConfig.setProperty("lastAccountName", name); + npcConfig.setProperty("last-account-name", name); npcConfig.setProperty("money", ess.getSettings().getStartingBalance()); npcConfig.blockingSave(); ess.getUserMap().trackUUID(npcUUID, name, false); @@ -96,7 +96,7 @@ public class Economy { } if (user == null) { - user = getUserByUUID(UUID.nameUUIDFromBytes(("NPC:" + name).getBytes(Charsets.UTF_8))); + user = getUserByUUID(UUID.nameUUIDFromBytes(("NPC:" + StringUtil.safeString(name)).getBytes(Charsets.UTF_8))); } return user; diff --git a/Essentials/src/main/java/com/earth2me/essentials/config/EssentialsUserConfiguration.java b/Essentials/src/main/java/com/earth2me/essentials/config/EssentialsUserConfiguration.java index 2eb9ab2cb..798b4da37 100644 --- a/Essentials/src/main/java/com/earth2me/essentials/config/EssentialsUserConfiguration.java +++ b/Essentials/src/main/java/com/earth2me/essentials/config/EssentialsUserConfiguration.java @@ -46,7 +46,7 @@ public class EssentialsUserConfiguration extends EssentialsConfiguration { LOGGER.log(Level.WARNING, "Failed to migrate user: " + username, ex); } - setProperty("lastAccountName", username); + setProperty("last-account-name", username); } private File getAltFile() { diff --git a/Essentials/src/main/java/com/earth2me/essentials/economy/vault/VaultEconomyProvider.java b/Essentials/src/main/java/com/earth2me/essentials/economy/vault/VaultEconomyProvider.java index 054c95462..4ba9e66ca 100644 --- a/Essentials/src/main/java/com/earth2me/essentials/economy/vault/VaultEconomyProvider.java +++ b/Essentials/src/main/java/com/earth2me/essentials/economy/vault/VaultEconomyProvider.java @@ -5,6 +5,7 @@ import com.earth2me.essentials.api.NoLoanPermittedException; import com.earth2me.essentials.api.UserDoesNotExistException; import com.earth2me.essentials.config.EssentialsUserConfiguration; import com.earth2me.essentials.utils.NumberUtil; +import com.earth2me.essentials.utils.StringUtil; import com.google.common.base.Charsets; import net.ess3.api.MaxMoneyException; import net.milkbowl.vault.economy.Economy; @@ -80,7 +81,7 @@ public class VaultEconomyProvider implements Economy { return true; } // We may not have the player name in the usermap, let's double check an NPC account with this name doesn't exist. - return com.earth2me.essentials.api.Economy.playerExists(UUID.nameUUIDFromBytes(("NPC:" + playerName).getBytes(Charsets.UTF_8))); + return com.earth2me.essentials.api.Economy.playerExists(UUID.nameUUIDFromBytes(("NPC:" + StringUtil.safeString(playerName)).getBytes(Charsets.UTF_8))); } @Override @@ -306,7 +307,7 @@ public class VaultEconomyProvider implements Economy { final EssentialsUserConfiguration npcConfig = new EssentialsUserConfiguration(player.getName(), player.getUniqueId(), npcFile); npcConfig.load(); npcConfig.setProperty("npc", true); - npcConfig.setProperty("lastAccountName", player.getName()); + npcConfig.setProperty("last-account-name", player.getName()); npcConfig.setProperty("money", ess.getSettings().getStartingBalance()); npcConfig.blockingSave(); ess.getUserMap().trackUUID(player.getUniqueId(), player.getName(), false);