Fix getting white list, ban list, and op list. Fixes BUKKIT-5538

The getEntries methods on these return player names instead of UUIDs.
As we need the UUIDs for our API we add a getValues method to get at
the data we need. To further ensure we get the most data possible we
also add a way to get at the stored GameProfile to ensure we always
have both the UUID and the name from the list.

9 files changed, 78 insertions, 51 deletions

diff --git a/src/main/java/net/minecraft/server/JsonList.java b/src/main/java/net/minecraft/server/JsonList.java
index 36df9888..36dbcf62 100644
--- a/src/main/java/net/minecraft/server/JsonList.java
+++ b/src/main/java/net/minecraft/server/JsonList.java
@@ -79,6 +79,12 @@ public class JsonList {
         return (String[]) this.d.keySet().toArray(new String[this.d.size()]);
     }
 
+    // CraftBukkit start
+    public Collection<JsonListEntry> getValues() {
+        return this.d.values();
+    }
+    // CraftBukkit end
+
     public boolean d() {
         return this.d.size() < 1;
     }
@@ -120,7 +126,7 @@ public class JsonList {
         return this.d;
     }
 
-    public void save() {
+    public void save() throws IOException { // CraftBukkit - Added throws
         Collection collection = this.d.values();
         String s = this.b.toJson(collection);
         BufferedWriter bufferedwriter = null;
@@ -133,7 +139,7 @@ public class JsonList {
         }
     }
 
-    public void load() {
+    public void load() throws IOException { // CraftBukkit - Added throws
         Collection collection = null;
         BufferedReader bufferedreader = null;
 
diff --git a/src/main/java/net/minecraft/server/JsonListEntry.java b/src/main/java/net/minecraft/server/JsonListEntry.java
index 3a7e2c38..e853c564 100644
--- a/src/main/java/net/minecraft/server/JsonListEntry.java
+++ b/src/main/java/net/minecraft/server/JsonListEntry.java
@@ -14,7 +14,7 @@ public class JsonListEntry {
         this.a = object;
     }
 
-    Object f() {
+    public Object f() { // CraftBukkit -> package private -> public
         return this.a;
     }
 
diff --git a/src/main/java/net/minecraft/server/JsonListEntrySerializer.java b/src/main/java/net/minecraft/server/JsonListEntrySerializer.java
index b6aba6a6..f4eee668 100644
--- a/src/main/java/net/minecraft/server/JsonListEntrySerializer.java
+++ b/src/main/java/net/minecraft/server/JsonListEntrySerializer.java
@@ -1,5 +1,7 @@
 package net.minecraft.server;
 
+// CraftBukkit - Imported for package private use in JsonList
+
 import java.lang.reflect.Type;
 
 import net.minecraft.util.com.google.gson.JsonDeserializationContext;
diff --git a/src/main/java/net/minecraft/server/NameReferencingFileConverter.java b/src/main/java/net/minecraft/server/NameReferencingFileConverter.java
index 45561e9b..a4724312 100644
--- a/src/main/java/net/minecraft/server/NameReferencingFileConverter.java
+++ b/src/main/java/net/minecraft/server/NameReferencingFileConverter.java
@@ -77,14 +77,12 @@ public class NameReferencingFileConverter {
 
         if (b.exists() && b.isFile()) {
             if (gameprofilebanlist.c().exists()) {
-                /* CraftBukkit start - Exception is never thrown
                 try {
                     gameprofilebanlist.load();
-                } catch (FileNotFoundException filenotfoundexception) {
-                    e.warn("Could not load existing file " + gameprofilebanlist.c().getName(), filenotfoundexception);
+                // CraftBukkit start - FileNotFoundException -> IOException, don't print stacetrace
+                } catch (IOException filenotfoundexception) {
+                    e.warn("Could not load existing file " + gameprofilebanlist.c().getName() + ", " + filenotfoundexception.getMessage());
                 }
-                */
-                gameprofilebanlist.load();
                 // CraftBukkit end
             }
 
@@ -115,14 +113,12 @@ public class NameReferencingFileConverter {
 
         if (a.exists() && a.isFile()) {
             if (ipbanlist.c().exists()) {
-                /* CraftBukkit start - Exception is never thrown
                 try {
                     ipbanlist.load();
-                } catch (FileNotFoundException filenotfoundexception) {
-                    e.warn("Could not load existing file " + ipbanlist.c().getName(), filenotfoundexception);
+                // CraftBukkit start - FileNotFoundException -> IOException, don't print stacetrace
+                } catch (IOException filenotfoundexception) {
+                    e.warn("Could not load existing file " + ipbanlist.c().getName() + ", " + filenotfoundexception.getMessage());
                 }
-                */
-                ipbanlist.load();
                 // CraftBukkit end
             }
 
@@ -160,14 +156,12 @@ public class NameReferencingFileConverter {
 
         if (c.exists() && c.isFile()) {
             if (oplist.c().exists()) {
-                /* CraftBukkit start - Exception is never thrown
                 try {
                     oplist.load();
-                } catch (FileNotFoundException filenotfoundexception) {
-                    e.warn("Could not load existing file " + oplist.c().getName(), filenotfoundexception);
+                // CraftBukkit start - FileNotFoundException -> IOException, don't print stacetrace
+                } catch (IOException filenotfoundexception) {
+                    e.warn("Could not load existing file " + oplist.c().getName() + ", " + filenotfoundexception.getMessage());
                 }
-                */
-                oplist.load();
                 // CraftBukkit end
             }
 
@@ -196,14 +190,12 @@ public class NameReferencingFileConverter {
 
         if (d.exists() && d.isFile()) {
             if (whitelist.c().exists()) {
-                /* CraftBukkit start - Exception is never thrown
                 try {
                     whitelist.load();
-                } catch (FileNotFoundException filenotfoundexception) {
-                    e.warn("Could not load existing file " + whitelist.c().getName(), filenotfoundexception);
+                // CraftBukkit start - FileNotFoundException -> IOException, don't print stacetrace
+                } catch (IOException filenotfoundexception) {
+                    e.warn("Could not load existing file " + whitelist.c().getName() + ", " + filenotfoundexception.getMessage());
                 }
-                */
-                whitelist.load();
                 // CraftBukkit end
             }
 
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftIpBanEntry.java b/src/main/java/org/bukkit/craftbukkit/CraftIpBanEntry.java
index 583f9ab6..42085f8d 100644
--- a/src/main/java/org/bukkit/craftbukkit/CraftIpBanEntry.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftIpBanEntry.java
@@ -2,7 +2,9 @@ package org.bukkit.craftbukkit;
 
 import net.minecraft.server.IpBanEntry;
 import net.minecraft.server.IpBanList;
+import net.minecraft.server.MinecraftServer;
 
+import java.io.IOException;
 import java.util.Date;
 
 public final class CraftIpBanEntry implements org.bukkit.BanEntry {
@@ -75,6 +77,10 @@ public final class CraftIpBanEntry implements org.bukkit.BanEntry {
     public void save() {
         IpBanEntry entry = new IpBanEntry(target, this.created, this.source, this.expiration, this.reason);
         this.list.add(entry);
-        this.list.save();
+        try {
+            this.list.save();
+        } catch (IOException ex) {
+            MinecraftServer.getLogger().error("Failed to save banned-ips.json, " + ex.getMessage());
+        }
     }
 }
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftIpBanList.java b/src/main/java/org/bukkit/craftbukkit/CraftIpBanList.java
index 75dc31bb..feb679c1 100644
--- a/src/main/java/org/bukkit/craftbukkit/CraftIpBanList.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftIpBanList.java
@@ -1,11 +1,13 @@
 package org.bukkit.craftbukkit;
 
+import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.util.Date;
 import java.util.Set;
 
 import net.minecraft.server.IpBanEntry;
 import net.minecraft.server.IpBanList;
+import net.minecraft.server.MinecraftServer;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.Validate;
 
@@ -39,7 +41,13 @@ public class CraftIpBanList implements org.bukkit.BanList {
                 StringUtils.isBlank(reason) ? null : reason);
 
         list.add(entry);
-        list.save();
+
+        try {
+            list.save();
+        } catch (IOException ex) {
+            MinecraftServer.getLogger().error("Failed to save banned-ips.json, " + ex.getMessage());
+        }
+
         return new CraftIpBanEntry(target, entry, list);
     }
 
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftProfileBanEntry.java b/src/main/java/org/bukkit/craftbukkit/CraftProfileBanEntry.java
index 499c5aac..c4491970 100644
--- a/src/main/java/org/bukkit/craftbukkit/CraftProfileBanEntry.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftProfileBanEntry.java
@@ -2,8 +2,10 @@ package org.bukkit.craftbukkit;
 
 import net.minecraft.server.GameProfileBanEntry;
 import net.minecraft.server.GameProfileBanList;
+import net.minecraft.server.MinecraftServer;
 import net.minecraft.util.com.mojang.authlib.GameProfile;
 
+import java.io.IOException;
 import java.util.Date;
 
 public final class CraftProfileBanEntry implements org.bukkit.BanEntry {
@@ -76,6 +78,10 @@ public final class CraftProfileBanEntry implements org.bukkit.BanEntry {
     public void save() {
         GameProfileBanEntry entry = new GameProfileBanEntry(profile, this.created, this.source, this.expiration, this.reason);
         this.list.add(entry);
-        this.list.save();
+        try {
+            this.list.save();
+        } catch (IOException ex) {
+            MinecraftServer.getLogger().error("Failed to save banned-players.json, " + ex.getMessage());
+        }
     }
 }
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftProfileBanList.java b/src/main/java/org/bukkit/craftbukkit/CraftProfileBanList.java
index c077daa1..5679a036 100644
--- a/src/main/java/org/bukkit/craftbukkit/CraftProfileBanList.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftProfileBanList.java
@@ -1,11 +1,14 @@
 package org.bukkit.craftbukkit;
 
+import java.io.IOException;
 import java.util.Date;
 import java.util.Set;
 import java.util.UUID;
 
 import net.minecraft.server.GameProfileBanEntry;
 import net.minecraft.server.GameProfileBanList;
+import net.minecraft.server.JsonListEntry;
+import net.minecraft.server.MinecraftServer;
 import net.minecraft.util.com.mojang.authlib.GameProfile;
 
 import org.apache.commons.lang.StringUtils;
@@ -47,18 +50,22 @@ public class CraftProfileBanList implements org.bukkit.BanList {
                 StringUtils.isBlank(reason) ? null : reason);
 
         list.add(entry);
-        list.save();
+
+        try {
+            list.save();
+        } catch (IOException ex) {
+            MinecraftServer.getLogger().error("Failed to save banned-players.json, " + ex.getMessage());
+        }
+
         return new CraftProfileBanEntry(profile, entry, list);
     }
 
     @Override
     public Set<org.bukkit.BanEntry> getBanEntries() {
         ImmutableSet.Builder<org.bukkit.BanEntry> builder = ImmutableSet.builder();
-        for (String target : list.getEntries()) {
-            UUID id = UUID.fromString(target);
-            GameProfile profile = new GameProfile(id, null);
-
-            builder.add(new CraftProfileBanEntry(profile, (GameProfileBanEntry) list.get(profile), list));
+        for (JsonListEntry entry : list.getValues()) {
+            GameProfile profile = (GameProfile) entry.f(); // Should be getKey
+            builder.add(new CraftProfileBanEntry(profile, (GameProfileBanEntry) entry, list));
         }
 
         return builder.build();
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftServer.java b/src/main/java/org/bukkit/craftbukkit/CraftServer.java
index 27eafd90..e33576d9 100644
--- a/src/main/java/org/bukkit/craftbukkit/CraftServer.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftServer.java
@@ -76,7 +76,10 @@ import net.minecraft.server.EntityTracker;
 import net.minecraft.server.EnumDifficulty;
 import net.minecraft.server.EnumGamemode;
 import net.minecraft.server.ExceptionWorldConflict;
+import net.minecraft.server.GameProfileBanEntry;
+import net.minecraft.server.GameProfileBanList;
 import net.minecraft.server.Items;
+import net.minecraft.server.JsonListEntry;
 import net.minecraft.server.PlayerList;
 import net.minecraft.server.RecipesFurnace;
 import net.minecraft.server.MinecraftServer;
@@ -737,8 +740,16 @@ public final class CraftServer implements Server {
         chunkGCLoadThresh = configuration.getInt("chunk-gc.load-threshold");
         loadIcon();
 
-        playerList.getIPBans().load();
-        playerList.getProfileBans().load();
+        try {
+            playerList.getIPBans().load();
+        } catch (IOException ex) {
+            logger.log(Level.WARNING, "Failed to load banned-ips.json, " + ex.getMessage());
+        }
+        try {
+            playerList.getProfileBans().load();
+        } catch (IOException ex) {
+            logger.log(Level.WARNING, "Failed to load banned-players.json, " + ex.getMessage());
+        }
 
         for (WorldServer world : console.worlds) {
             world.difficulty = difficulty;
@@ -1330,12 +1341,8 @@ public final class CraftServer implements Server {
     public Set<OfflinePlayer> getBannedPlayers() {
         Set<OfflinePlayer> result = new HashSet<OfflinePlayer>();
 
-        for (String id : playerList.getProfileBans().getEntries()) {
-            try {
-                result.add(getOfflinePlayer(UUID.fromString(id)));
-            } catch (IllegalArgumentException ex) {
-                // This shouldn't happen
-            }
+        for (JsonListEntry entry : playerList.getProfileBans().getValues()) {
+            result.add(getOfflinePlayer((GameProfile) entry.f())); // Should be getKey
         }
 
         return result;
@@ -1364,11 +1371,8 @@ public final class CraftServer implements Server {
     public Set<OfflinePlayer> getWhitelistedPlayers() {
         Set<OfflinePlayer> result = new LinkedHashSet<OfflinePlayer>();
 
-        for (Object name : playerList.getWhitelisted()) {
-            if (((String)name).length() == 0 || ((String)name).startsWith("#")) {
-                continue;
-            }
-            result.add(getOfflinePlayer((String) name));
+        for (JsonListEntry entry : playerList.getWhitelist().getValues()) {
+            result.add(getOfflinePlayer((GameProfile) entry.f())); // Should be getKey
         }
 
         return result;
@@ -1377,12 +1381,8 @@ public final class CraftServer implements Server {
     public Set<OfflinePlayer> getOperators() {
         Set<OfflinePlayer> result = new HashSet<OfflinePlayer>();
 
-        for (String id : playerList.getOPs().getEntries()) {
-            try {
-                result.add(getOfflinePlayer(UUID.fromString(id)));
-            } catch (IllegalArgumentException ex) {
-                // This shouldn't ever happen
-            }
+        for (JsonListEntry entry : playerList.getOPs().getValues()) {
+            result.add(getOfflinePlayer((GameProfile) entry.f())); // Should be getKey
         }
 
         return result;