ban expiration date in banned-players.json isn't functional / banned-ips experation date doesn't fully work

Can confirm (See attached picture img.png)

Bug is still in 1.9

Already in the affected versions...

Please link to this comment in the description

The following is based on decompiled version of Minecraft 1.9 using MCP 9.24 beta.

The reason why the entry from the banned-players.json file is not removed is because the method net.minecraft.server.management.UserList.removeExpired() uses the wrong key to remove the entry. It uses the value of the entry to remove it, instead it should uses the method net.minecraft.server.management.UserList.getObjectKey(K) to create the key based on the entry value.

/**
 * Removes expired bans from the list. See {@link BanEntry#hasBanExpired}
 */
private void removeExpired()
{
    List<K> list = Lists.<K>newArrayList();

    for (V v : this.values.values())
    {
        if (v.hasBanExpired())
        {
            list.add(v.getValue());
        }
    }

    for (K k : list)
    {
        // Replaced this
        //this.values.remove(k);
        this.values.remove(this.getObjectKey(k));
    }
}

The reason why an exception is thrown for the first time is because the method net.minecraft.server.management.PlayerList.allowUserToConnect(SocketAddress, GameProfile) first tests if the GameProfile is banned and if this is the case it tries to get the ban entry. The problem is that the method net.minecraft.server.management.UserList.getEntry(K) first removes expired ban entries. This means that it returns null if the ban was removed, which then causes the NullPointerException.
Instead the methods net.minecraft.server.management.UserListIPBans.isBanned(SocketAddress), net.minecraft.server.management.UserListBans.isBanned(GameProfile) and net.minecraft.server.management.UserListBans.isUsernameBanned(String) should remove expired entries first before testing if the IP, GameProfile or username is banned.

Seems to be working fine in 1.13.1
(Yes it unbanned me after)

"created": "2018-08-28 09:34:53 -0400"
"expires": "2018-08-28 09:37:00 -0400"

Can confirm fixed.

Please re-open this issue, it's still present in 1.14.4. Reproduction steps are the same - set a ban that will expire, wait until expire time, and then join. The exception will only occur once though.

I have submitted a patch to Paper that Mojang is free to use - https://github.com/PaperMC/Paper/pull/2458

(the code is a bit different due to CraftBukkit changes but the same logic applies)

The relevant code is just this one line (spigot mappings), marked // NPE in PlayerList:

if (this.k.isBanned(gameprofile)) {
            GameProfileBanEntry gameprofilebanentry = (GameProfileBanEntry) this.k.get(gameprofile);

            chatmessage = new ChatMessage("multiplayer.disconnect.banned.reason", new Object[]{gameprofilebanentry.getReason()}); // NPE
            if (gameprofilebanentry.getExpires() != null) {
                chatmessage.addSibling(new ChatMessage("multiplayer.disconnect.banned.expiration", new Object[]{PlayerList.g.format(gameprofilebanentry.getExpires())}));
            }

the get() call will remove the relevant entry if it has expired, and then return null.  Then it will clearly NPE on the marked line.

The stacktrace of this occurring on the vanilla server:

[06:30:45] [Server thread/WARN]: Failed to handle packet for /127.0.0.1:50701
java.lang.NullPointerException: null
        at xv.a(SourceFile:357) ~[minecraft_server.jar:?]
        at we.c(SourceFile:97) ~[minecraft_server.jar:?]
        at we.b(SourceFile:63) ~[minecraft_server.jar:?]
        at jc.a(SourceFile:230) ~[minecraft_server.jar:?]
        at wb.c(SourceFile:171) [minecraft_server.jar:?]
        at net.minecraft.server.MinecraftServer.b(SourceFile:847) [minecraft_server.jar:?]
        at uk.b(SourceFile:343) [minecraft_server.jar:?]
        at net.minecraft.server.MinecraftServer.a(SourceFile:774) [minecraft_server.jar:?]
        at net.minecraft.server.MinecraftServer.run(SourceFile:642) [minecraft_server.jar:?]
        at java.lang.Thread.run(Thread.java:748) [?:1.8.0_222]
[06:30:45] [Server thread/INFO]: com.mojang.authlib.GameProfile@664932ca[id=9a9949cc-1987-3d04-b56b-cf3286adfe51,name=searchndstroy,properties={},legacy=false] (/127.0.0.1:50701) lost connection: Internal server error

@unknown, could you please create a new ticket that we can mark as a clone of this ticket? I'd rather have a new ticket than reusing this one.

https://bugs.mojang.com/browse/MC-158900

Thanks! I've linked the tickets accordingly.