From 0e9fe0c3ecf102bdaa6535b67d82c9f125ad0040 Mon Sep 17 00:00:00 2001 From: Ben Woo <30431861+benwoo1110@users.noreply.github.com> Date: Sun, 16 May 2021 21:12:10 +0800 Subject: [PATCH 1/3] Add configurable option to allow unsafe worldname. --- .../MultiverseCoreConfiguration.java | 17 +++++++++++++++-- .../api/MultiverseCoreConfig.java | 4 ++++ .../MultiverseCore/utils/WorldManager.java | 16 +++++++++------- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/onarandombox/MultiverseCore/MultiverseCoreConfiguration.java b/src/main/java/com/onarandombox/MultiverseCore/MultiverseCoreConfiguration.java index a5fa94791..adaa96b87 100644 --- a/src/main/java/com/onarandombox/MultiverseCore/MultiverseCoreConfiguration.java +++ b/src/main/java/com/onarandombox/MultiverseCore/MultiverseCoreConfiguration.java @@ -1,7 +1,5 @@ package com.onarandombox.MultiverseCore; -import java.util.Map; - import com.dumptruckman.minecraft.util.Logging; import com.onarandombox.MultiverseCore.api.MultiverseCoreConfig; import com.onarandombox.MultiverseCore.event.MVDebugModeEvent; @@ -10,6 +8,8 @@ import me.main__.util.SerializationConfig.SerializationConfig; import org.bukkit.Bukkit; +import java.util.Map; + /** * Our configuration. */ @@ -77,6 +77,8 @@ public static MultiverseCoreConfiguration getInstance() { private volatile boolean autopurge; @Property private volatile boolean idonotwanttodonate; + @Property + private volatile boolean allowunsafeworldname; public MultiverseCoreConfiguration() { super(); @@ -111,6 +113,7 @@ protected void setDefaults() { portalsearchradius = 128; autopurge = true; idonotwanttodonate = false; + allowunsafeworldname = false; // END CHECKSTYLE-SUPPRESSION: MagicNumberCheck } @@ -382,4 +385,14 @@ public boolean isShowingDonateMessage() { public void setShowDonateMessage(boolean showDonateMessage) { this.idonotwanttodonate = !showDonateMessage; } + + @Override + public boolean isAllowUnsafeWorldName() { + return allowunsafeworldname; + } + + @Override + public void setAllowUnsafeWorldName(boolean allowUnsafeWorldName) { + this.allowunsafeworldname = allowUnsafeWorldName; + } } diff --git a/src/main/java/com/onarandombox/MultiverseCore/api/MultiverseCoreConfig.java b/src/main/java/com/onarandombox/MultiverseCore/api/MultiverseCoreConfig.java index 07b2bb91d..8cdb53eb9 100644 --- a/src/main/java/com/onarandombox/MultiverseCore/api/MultiverseCoreConfig.java +++ b/src/main/java/com/onarandombox/MultiverseCore/api/MultiverseCoreConfig.java @@ -239,4 +239,8 @@ public interface MultiverseCoreConfig extends ConfigurationSerializable { * @param idonotwanttodonate True if donation/patreon messages should be shown. */ void setShowDonateMessage(boolean idonotwanttodonate); + + boolean isAllowUnsafeWorldName(); + + void setAllowUnsafeWorldName(boolean allowUnsafeWorldName); } diff --git a/src/main/java/com/onarandombox/MultiverseCore/utils/WorldManager.java b/src/main/java/com/onarandombox/MultiverseCore/utils/WorldManager.java index 0b2ab0701..860c6df3f 100644 --- a/src/main/java/com/onarandombox/MultiverseCore/utils/WorldManager.java +++ b/src/main/java/com/onarandombox/MultiverseCore/utils/WorldManager.java @@ -48,7 +48,6 @@ import java.util.Stack; import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; -import java.util.regex.Pattern; import java.util.stream.Collectors; /** @@ -130,7 +129,11 @@ public boolean cloneWorld(String oldName, String newName) { } // Check for valid world name - if (!(WorldNameChecker.isValidWorldName(oldName) && WorldNameChecker.isValidWorldName(newName))) { + if (!this.plugin.getMVConfig().isAllowUnsafeWorldName() && !WorldNameChecker.isValidWorldName(newName)) { + Logging.warning("Unable to clone world as world name '" + newName + "' is invalid."); + Logging.warning("World name should not contain spaces or special characters!"); + Logging.warning("Enable allowunsafeworldname to bypass the check if you are absolutely sure this world is valid."); + Logging.warning("However, MV is not response of any damage to your world if you enable this option."); return false; } @@ -234,13 +237,12 @@ public boolean addWorld(String name, Environment env, String seedString, WorldTy @Override public boolean addWorld(String name, Environment env, String seedString, WorldType type, Boolean generateStructures, String generator, boolean useSpawnAdjust) { - if (name.equalsIgnoreCase("plugins") || name.equalsIgnoreCase("logs")) { - return false; - } - if (!WorldNameChecker.isValidWorldName(name)) { - Logging.warning("Invalid world name '" + name + "'"); + if (!this.plugin.getMVConfig().isAllowUnsafeWorldName() && !WorldNameChecker.isValidWorldName(name)) { + Logging.warning("Unable to add world as world name '" + name + "' is invalid."); Logging.warning("World name should not contain spaces or special characters!"); + Logging.warning("Enable allowunsafeworldname to bypass the check if you are absolutely sure this world is valid."); + Logging.warning("However, MV is not response of any damage to your world if you enable this option."); return false; } From a14b822de83950e50b9dbbb2146358e9cc8f451e Mon Sep 17 00:00:00 2001 From: Ben Woo <30431861+benwoo1110@users.noreply.github.com> Date: Sun, 16 May 2021 21:13:16 +0800 Subject: [PATCH 2/3] Disallow worldname to have . or / --- .../com/onarandombox/MultiverseCore/utils/WorldNameChecker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/onarandombox/MultiverseCore/utils/WorldNameChecker.java b/src/main/java/com/onarandombox/MultiverseCore/utils/WorldNameChecker.java index 7e4841675..7e7aa8bc8 100644 --- a/src/main/java/com/onarandombox/MultiverseCore/utils/WorldNameChecker.java +++ b/src/main/java/com/onarandombox/MultiverseCore/utils/WorldNameChecker.java @@ -18,7 +18,7 @@ */ public class WorldNameChecker { - private static final Pattern WORLD_NAME_PATTERN = Pattern.compile("[a-zA-Z0-9/._-]+"); + private static final Pattern WORLD_NAME_PATTERN = Pattern.compile("[a-zA-Z0-9_-]+"); private static final Set BLACKLIST_NAMES = Collections.unmodifiableSet(new HashSet() {{ add("plugins"); add("logs"); From c132d18ae33e1cfe387a9eeb61d87212d8a3e13a Mon Sep 17 00:00:00 2001 From: Ben Woo <30431861+benwoo1110@users.noreply.github.com> Date: Sun, 16 May 2021 21:42:45 +0800 Subject: [PATCH 3/3] Improve world name/folder checking for create and import command. Make use of the new WorldNameChecker class. --- .../commands/CreateCommand.java | 24 ++++----- .../commands/ImportCommand.java | 52 +++++++++---------- .../MultiverseCore/TestWorldStuff.java | 13 +++-- 3 files changed, 43 insertions(+), 46 deletions(-) diff --git a/src/main/java/com/onarandombox/MultiverseCore/commands/CreateCommand.java b/src/main/java/com/onarandombox/MultiverseCore/commands/CreateCommand.java index 6d8f3833b..9cbc904b2 100644 --- a/src/main/java/com/onarandombox/MultiverseCore/commands/CreateCommand.java +++ b/src/main/java/com/onarandombox/MultiverseCore/commands/CreateCommand.java @@ -9,6 +9,7 @@ import com.onarandombox.MultiverseCore.MultiverseCore; import com.onarandombox.MultiverseCore.api.MVWorldManager; +import com.onarandombox.MultiverseCore.utils.WorldNameChecker; import com.pneumaticraft.commandhandler.CommandHandler; import org.bukkit.ChatColor; import org.bukkit.World.Environment; @@ -17,7 +18,6 @@ import org.bukkit.command.CommandSender; import org.bukkit.permissions.PermissionDefault; -import java.io.File; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -46,16 +46,10 @@ public CreateCommand(MultiverseCore plugin) { this.addCommandExample("/mv create " + ChatColor.GOLD + "moonworld" + ChatColor.GREEN + " normal" + ChatColor.DARK_AQUA + " -g BukkitFullOfMoon"); this.worldManager = this.plugin.getMVWorldManager(); } - - private String trimWorldName(String userInput) { - // Removes relative paths. - return userInput.replaceAll("^[./\\\\]+", ""); - } - + @Override public void runCommand(CommandSender sender, List args) { - String worldName = trimWorldName(args.get(0)); - File worldFile = new File(this.plugin.getServer().getWorldContainer(), worldName); + String worldName = args.get(0); String env = args.get(1); String seed = CommandHandler.getFlag("-s", args); String generator = CommandHandler.getFlag("-g", args); @@ -71,20 +65,20 @@ public void runCommand(CommandSender sender, List args) { useSpawnAdjust = false; } } - - // Make sure the world name doesn't contain the words 'plugins' and '.dat' - if(worldName.contains("plugins")||worldName.contains(".dat")){ - sender.sendMessage(ChatColor.RED + "Multiverse cannot create a world that contains 'plugins' or '.dat'"); + + if(!this.plugin.getMVConfig().isAllowUnsafeWorldName() && !WorldNameChecker.isValidWorldName(worldName)) { + sender.sendMessage(ChatColor.RED + "Multiverse cannot create the world as the world name '" + + worldName + "' contains spaces or invalid characters!"); return; } - + if (this.worldManager.isMVWorld(worldName)) { sender.sendMessage(ChatColor.RED + "Multiverse cannot create " + ChatColor.GOLD + ChatColor.UNDERLINE + "another" + ChatColor.RESET + ChatColor.RED + " world named " + worldName); return; } - if (worldFile.exists()) { + if (WorldNameChecker.checkFolder(worldName) != WorldNameChecker.FolderStatus.DOES_NOT_EXIST) { sender.sendMessage(ChatColor.RED + "A Folder/World already exists with this name!"); sender.sendMessage(ChatColor.RED + "If you are confident it is a world you can import with /mvimport"); return; diff --git a/src/main/java/com/onarandombox/MultiverseCore/commands/ImportCommand.java b/src/main/java/com/onarandombox/MultiverseCore/commands/ImportCommand.java index 7a52d5a15..fc7bb0382 100644 --- a/src/main/java/com/onarandombox/MultiverseCore/commands/ImportCommand.java +++ b/src/main/java/com/onarandombox/MultiverseCore/commands/ImportCommand.java @@ -17,7 +17,6 @@ import org.bukkit.command.CommandSender; import org.bukkit.permissions.PermissionDefault; -import java.io.File; import java.util.Collection; import java.util.List; @@ -55,15 +54,10 @@ private String getPotentialWorldStrings() { return worldList.toString(); } - - private String trimWorldName(String userInput) { - // Removes relative paths. - return userInput.replaceAll("^[./\\\\]+", ""); - } @Override public void runCommand(CommandSender sender, List args) { - String worldName = trimWorldName(args.get(0)); + String worldName = args.get(0); if (worldName.toLowerCase().equals("--list") || worldName.toLowerCase().equals("-l")) { String worldList = this.getPotentialWorldStrings(); @@ -82,11 +76,11 @@ public void runCommand(CommandSender sender, List args) { return; } - // Make sure the world name doesn't contain the words 'plugins' and '.dat' - if(worldName.contains("plugins")||worldName.contains(".dat")){ - sender.sendMessage(ChatColor.RED + "Multiverse cannot create a world that contains 'plugins' or '.dat'"); + if(!this.plugin.getMVConfig().isAllowUnsafeWorldName() && !WorldNameChecker.isValidWorldName(worldName)) { + sender.sendMessage(ChatColor.RED + "Multiverse cannot import the world as the world name '" + + worldName + "' contains spaces or invalid characters!"); return; - } + } // Make sure we don't already know about this world. if (this.worldManager.isMVWorld(worldName)) { @@ -95,7 +89,18 @@ public void runCommand(CommandSender sender, List args) { return; } - File worldFile = new File(this.plugin.getServer().getWorldContainer(), worldName); + switch (WorldNameChecker.checkFolder(worldName)) { + case DOES_NOT_EXIST: + sender.sendMessage(ChatColor.RED + "FAILED."); + sender.sendMessage(ChatColor.RED + "That world folder does not exist. These look like worlds to me:"); + sender.sendMessage(this.getPotentialWorldStrings()); + return; + case NOT_A_WORLD: + sender.sendMessage(ChatColor.RED + "FAILED."); + sender.sendMessage(String.format("%s'%s' does not appear to be a world. It is lacking a .dat file.", + ChatColor.RED, worldName)); + return; + } String generator = CommandHandler.getFlag("-g", args); boolean useSpawnAdjust = true; @@ -113,25 +118,18 @@ public void runCommand(CommandSender sender, List args) { return; } - if (!worldFile.exists()) { - sender.sendMessage(ChatColor.RED + "FAILED."); - String worldList = this.getPotentialWorldStrings(); - sender.sendMessage("That world folder does not exist. These look like worlds to me:"); - sender.sendMessage(worldList); - } else if (!WorldNameChecker.isValidWorldFolder(worldFile)) { - sender.sendMessage(ChatColor.RED + "FAILED."); - sender.sendMessage(String.format("'%s' does not appear to be a world. It is lacking a .dat file.", - worldName)); - } else if (env == null) { + if (env == null) { sender.sendMessage(ChatColor.RED + "FAILED."); sender.sendMessage("That world environment did not exist."); sender.sendMessage("For a list of available world types, type: " + ChatColor.AQUA + "/mvenv"); + return; + } + + Command.broadcastCommandMessage(sender, String.format("Starting import of world '%s'...", worldName)); + if (this.worldManager.addWorld(worldName, environment, null, null, null, generator, useSpawnAdjust)) { + Command.broadcastCommandMessage(sender, ChatColor.GREEN + "Complete!"); } else { - Command.broadcastCommandMessage(sender, String.format("Starting import of world '%s'...", worldName)); - if (this.worldManager.addWorld(worldName, environment, null, null, null, generator, useSpawnAdjust)) - Command.broadcastCommandMessage(sender, ChatColor.GREEN + "Complete!"); - else - Command.broadcastCommandMessage(sender, ChatColor.RED + "Failed!"); + Command.broadcastCommandMessage(sender, ChatColor.RED + "Failed!"); } } } \ No newline at end of file diff --git a/src/test/java/com/onarandombox/MultiverseCore/TestWorldStuff.java b/src/test/java/com/onarandombox/MultiverseCore/TestWorldStuff.java index 05957c842..b2828d25b 100644 --- a/src/test/java/com/onarandombox/MultiverseCore/TestWorldStuff.java +++ b/src/test/java/com/onarandombox/MultiverseCore/TestWorldStuff.java @@ -30,7 +30,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentMatchers; -import org.mockito.Matchers; import org.mockito.internal.verification.VerificationModeFactory; import org.powermock.core.classloader.annotations.PowerMockIgnore; import org.powermock.core.classloader.annotations.PrepareForTest; @@ -38,8 +37,14 @@ import java.io.File; -import static junit.framework.Assert.*; -import static org.mockito.Mockito.*; +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertNotNull; +import static junit.framework.Assert.assertTrue; +import static junit.framework.Assert.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; @RunWith(PowerMockRunner.class) @PrepareForTest({ PluginManager.class, MultiverseCore.class, Permission.class, Bukkit.class, WorldManager.class, @@ -92,7 +97,7 @@ public void testWorldImportWithNoFolder() { // Import the first world. The world folder does not exist. plugin.onCommand(mockCommandSender, mockCommand, "", normalArgs); verify(mockCommandSender).sendMessage(ChatColor.RED + "FAILED."); - verify(mockCommandSender).sendMessage("That world folder does not exist. These look like worlds to me:"); + verify(mockCommandSender).sendMessage(ChatColor.RED + "That world folder does not exist. These look like worlds to me:"); // We should still have no worlds. assertEquals(0, creator.getCore().getMVWorldManager().getMVWorlds().size());