From a6818c1e175bbe7895c902c3e52764406bd3ca91 Mon Sep 17 00:00:00 2001 From: drdev Date: Sat, 6 Feb 2016 19:30:00 +0000 Subject: [PATCH] Add better validation for Planar Conquest readers --- .../forge/util/collect/FCollectionReader.java | 15 +++++- .../forge/util/storage/StorageReaderFile.java | 54 ++++++------------- .../src/main/java/forge/model/CardBlock.java | 2 +- .../forge/planarconquest/ConquestEvent.java | 19 +++++-- .../forge/planarconquest/ConquestPlane.java | 19 +++++-- .../forge/planarconquest/ConquestRegion.java | 19 +++++-- .../src/main/java/forge/quest/QuestWorld.java | 50 ++++++++++------- 7 files changed, 106 insertions(+), 72 deletions(-) diff --git a/forge-core/src/main/java/forge/util/collect/FCollectionReader.java b/forge-core/src/main/java/forge/util/collect/FCollectionReader.java index 034a87090de..f12efa56fa9 100644 --- a/forge-core/src/main/java/forge/util/collect/FCollectionReader.java +++ b/forge-core/src/main/java/forge/util/collect/FCollectionReader.java @@ -12,13 +12,24 @@ public abstract class FCollectionReader { } void readAll(FCollection collection) { - for (final String line : FileUtil.readFile(file)) { - final T item = read(line); + for (String line : FileUtil.readFile(file)) { + line = line.trim(); + if (line.isEmpty()) { + continue; //ignore blank or whitespace lines + } + + T item = read(line); if (item != null) { collection.add(item); } } } + protected void alertInvalidLine(String line, String message) { + System.err.println(message); + System.err.println(line); + System.err.println(file.getPath()); + } + protected abstract T read(String line); } diff --git a/forge-core/src/main/java/forge/util/storage/StorageReaderFile.java b/forge-core/src/main/java/forge/util/storage/StorageReaderFile.java index 036a6d79cca..93d9a75368f 100644 --- a/forge-core/src/main/java/forge/util/storage/StorageReaderFile.java +++ b/forge-core/src/main/java/forge/util/storage/StorageReaderFile.java @@ -36,22 +36,10 @@ import java.util.TreeMap; public abstract class StorageReaderFile extends StorageReaderBase { protected final File file; - /** - * Instantiates a new storage reader file. - * - * @param pathname the pathname - * @param keySelector0 the key selector0 - */ public StorageReaderFile(final String pathname, final Function keySelector0) { this(new File(pathname), keySelector0); } - /** - * Instantiates a new storage reader file. - * - * @param file0 the file0 - * @param keySelector0 the key selector0 - */ public StorageReaderFile(final File file0, final Function keySelector0) { super(keySelector0); file = file0; @@ -62,23 +50,24 @@ public abstract class StorageReaderFile extends StorageReaderBase { return file.getPath(); } - /* (non-Javadoc) - * @see forge.util.IItemReader#readAll() - */ @Override public Map readAll() { final Map result = new TreeMap(); int idx = 0; - for (final String s : FileUtil.readFile(file)) { - if (!lineContainsObject(s)) { + for (String line : FileUtil.readFile(file)) { + line = line.trim(); + if (line.isEmpty()) { + continue; //ignore blank or whitespace lines + } + + if (!lineContainsObject(line)) { continue; } - final T item = read(s, idx); - if (null == item) { - final String msg = "An object stored in " + file.getPath() + " failed to load.\nPlease submit this as a bug with the mentioned file attached."; - throw new RuntimeException(msg); + T item = read(line, idx); + if (item == null) { + continue; } idx++; @@ -92,31 +81,20 @@ public abstract class StorageReaderFile extends StorageReaderBase { return result; } - /** - * TODO: Write javadoc for this method. - * - * @param line - * the line - * @return the t - */ protected abstract T read(String line, int idx); - /** - * Line contains object. - * - * @param line - * the line - * @return true, if successful - */ protected boolean lineContainsObject(final String line) { return !StringUtils.isBlank(line) && !line.trim().startsWith("#"); } - /* (non-Javadoc) - * @see forge.util.IItemReader#getItemKey(java.lang.Object) - */ @Override public String getItemKey(final T item) { return keySelector.apply(item); } + + protected void alertInvalidLine(String line, String message) { + System.err.println(message); + System.err.println(line); + System.err.println(file.getPath()); + } } diff --git a/forge-gui/src/main/java/forge/model/CardBlock.java b/forge-gui/src/main/java/forge/model/CardBlock.java index 9aeb7c55eb6..730516590b3 100644 --- a/forge-gui/src/main/java/forge/model/CardBlock.java +++ b/forge-gui/src/main/java/forge/model/CardBlock.java @@ -238,7 +238,7 @@ public final class CardBlock implements Comparable { */ @Override protected CardBlock read(String line, int i) { - final String[] sParts = TextUtil.splitWithParenthesis(line.trim(), ',', 3); + final String[] sParts = TextUtil.splitWithParenthesis(line, ',', 3); String name = sParts[0]; String[] numbers = sParts[1].trim().split("/"); diff --git a/forge-gui/src/main/java/forge/planarconquest/ConquestEvent.java b/forge-gui/src/main/java/forge/planarconquest/ConquestEvent.java index d39ce93edc5..9fa043442b5 100644 --- a/forge-gui/src/main/java/forge/planarconquest/ConquestEvent.java +++ b/forge-gui/src/main/java/forge/planarconquest/ConquestEvent.java @@ -103,11 +103,19 @@ public class ConquestEvent { String avatar = null; String description = null; - String[] pieces = line.trim().split("\\|"); + String key, value; + String[] pieces = line.split("\\|"); for (String piece : pieces) { - String[] kv = piece.split(":", 2); - String key = kv[0].trim().toLowerCase(); - String value = kv[1].trim(); + int idx = piece.indexOf(':'); + if (idx != -1) { + key = piece.substring(0, idx).trim().toLowerCase(); + value = piece.substring(idx + 1).trim(); + } + else { + alertInvalidLine(line, "Invalid event definition."); + key = piece.trim().toLowerCase(); + value = ""; + } switch(key) { case "name": name = value; @@ -133,6 +141,9 @@ public class ConquestEvent { case "desc": description = value; break; + default: + alertInvalidLine(line, "Invalid event definition."); + break; } } if (deck == null) { diff --git a/forge-gui/src/main/java/forge/planarconquest/ConquestPlane.java b/forge-gui/src/main/java/forge/planarconquest/ConquestPlane.java index b6c2f088c7c..b4b8aaa527c 100644 --- a/forge-gui/src/main/java/forge/planarconquest/ConquestPlane.java +++ b/forge-gui/src/main/java/forge/planarconquest/ConquestPlane.java @@ -243,11 +243,19 @@ public class ConquestPlane { String description = null; boolean unreachable = false; - String[] pieces = line.trim().split("\\|"); + String key, value; + String[] pieces = line.split("\\|"); for (String piece : pieces) { - String[] kv = piece.split(":", 2); - String key = kv[0].trim().toLowerCase(); - String value = kv[1].trim(); + int idx = piece.indexOf(':'); + if (idx != -1) { + key = piece.substring(0, idx).trim().toLowerCase(); + value = piece.substring(idx + 1).trim(); + } + else { + alertInvalidLine(line, "Invalid plane definition."); + key = piece.trim().toLowerCase(); + value = ""; + } switch(key) { case "name": name = value; @@ -266,6 +274,9 @@ public class ConquestPlane { case "desc": description = value; break; + default: + alertInvalidLine(line, "Invalid plane definition."); + break; } } return new ConquestPlane(name, description, regionSize, unreachable); diff --git a/forge-gui/src/main/java/forge/planarconquest/ConquestRegion.java b/forge-gui/src/main/java/forge/planarconquest/ConquestRegion.java index e37cfcb93ae..98be760d76e 100644 --- a/forge-gui/src/main/java/forge/planarconquest/ConquestRegion.java +++ b/forge-gui/src/main/java/forge/planarconquest/ConquestRegion.java @@ -90,11 +90,19 @@ public class ConquestRegion { ColorSet colorSet = ColorSet.ALL_COLORS; Predicate pred = Predicates.alwaysTrue(); - String[] pieces = line.trim().split("\\|"); + String key, value; + String[] pieces = line.split("\\|"); for (String piece : pieces) { - String[] kv = piece.split(":", 2); - String key = kv[0].trim().toLowerCase(); - String value = kv[1].trim(); + int idx = piece.indexOf(':'); + if (idx != -1) { + key = piece.substring(0, idx).trim().toLowerCase(); + value = piece.substring(idx + 1).trim(); + } + else { + alertInvalidLine(line, "Invalid plane definition."); + key = piece.trim().toLowerCase(); + value = ""; + } switch(key) { case "name": name = value; @@ -129,6 +137,9 @@ public class ConquestRegion { } }; break; + default: + alertInvalidLine(line, "Invalid region definition."); + break; } } return new ConquestRegion(plane, name, artCardName, colorSet, pred); diff --git a/forge-gui/src/main/java/forge/quest/QuestWorld.java b/forge-gui/src/main/java/forge/quest/QuestWorld.java index be009346f65..bb796e3d914 100644 --- a/forge-gui/src/main/java/forge/quest/QuestWorld.java +++ b/forge-gui/src/main/java/forge/quest/QuestWorld.java @@ -18,7 +18,6 @@ package forge.quest; import com.google.common.base.Function; - import forge.deck.Deck; import forge.game.GameFormat; import forge.item.PaperCard; @@ -131,28 +130,41 @@ public class QuestWorld implements Comparable{ final List sets = new ArrayList(); final List bannedCards = new ArrayList(); // if both empty, no format - // This is what you need to use here => - // FileSection.parse(line, ":", "|"); - - final String[] sParts = line.trim().split("\\|"); - - for (final String sPart : sParts) { - - final String[] kv = sPart.split(":", 2); - final String key = kv[0].toLowerCase(); - if ("name".equals(key)) { - useName = kv[1]; - } else if ("dir".equals(key)) { - useDir = kv[1]; - } else if ("sets".equals(key)) { - sets.addAll(Arrays.asList(kv[1].split(", "))); - } else if ("banned".equals(key)) { - bannedCards.addAll(Arrays.asList(kv[1].split("; "))); + String key, value; + String[] pieces = line.split("\\|"); + for (String piece : pieces) { + int idx = piece.indexOf(':'); + if (idx != -1) { + key = piece.substring(0, idx).trim().toLowerCase(); + value = piece.substring(idx + 1).trim(); + } + else { + alertInvalidLine(line, "Invalid plane definition."); + key = piece.trim().toLowerCase(); + value = ""; + } + switch(key) { + case "name": + useName = value; + break; + case "dir": + useDir = value; + break; + case "sets": + sets.addAll(Arrays.asList(value.split(", "))); + break; + case "banned": + bannedCards.addAll(Arrays.asList(value.split("; "))); + break; + default: + alertInvalidLine(line, "Invalid quest world definition."); + break; } } if (useName == null) { - throw new RuntimeException("A Quest World must have a name! Check worlds.txt file"); + alertInvalidLine(line, "Quest world must have a name."); + return null; } if (!sets.isEmpty() || !bannedCards.isEmpty()) {