From fb1492907598ad041b876c4864b2833d8f39b1e9 Mon Sep 17 00:00:00 2001 From: A248 Date: Thu, 17 Aug 2023 19:06:37 -0500 Subject: [PATCH] Three fixes relating to specific setups, including for #229 * Solve incoming logins using wrong UUID on Sponge when player info forwarding is enabled (#229). * The 'show-applicable-for-history' option was turning /history into /warns. Extend the option to similarly infuse /warns with applicability, but don't take over the command. * Initialize the H2 database driver if possible. Use data sources and direct instantiation to avoid registering the Driver if possible. --- .../core/commands/ListCommands.java | 62 +++++++++++-------- .../libertybans/core/config/MainConfig.java | 4 +- .../libertybans/core/database/JdbcDriver.java | 25 +++++--- .../core/importing/JdbcDetails.java | 43 ++++++++----- .../env/sponge/ConnectionListener.java | 18 +++--- 5 files changed, 88 insertions(+), 64 deletions(-) diff --git a/bans-core/src/main/java/space/arim/libertybans/core/commands/ListCommands.java b/bans-core/src/main/java/space/arim/libertybans/core/commands/ListCommands.java index 740cc2caa..26f5ae749 100644 --- a/bans-core/src/main/java/space/arim/libertybans/core/commands/ListCommands.java +++ b/bans-core/src/main/java/space/arim/libertybans/core/commands/ListCommands.java @@ -23,6 +23,7 @@ import jakarta.inject.Singleton; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.ComponentLike; +import org.checkerframework.checker.nullness.qual.NonNull; import space.arim.api.jsonchat.adventure.util.ComponentText; import space.arim.libertybans.api.CompositeVictim; import space.arim.libertybans.api.PunishmentType; @@ -47,6 +48,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Function; import java.util.function.UnaryOperator; import java.util.stream.Stream; @@ -121,34 +123,25 @@ public ReactionStage execute() { target = command().next(); if (config().commands().showApplicableForHistory()) { - yield argumentParser().parsePlayer(sender(), target).thenCompose((uuidAndAddress) -> { - if (uuidAndAddress == null) { - return completedFuture(null); - } - return parsePageThenExecute( - selector.selectionByApplicabilityBuilder( - uuidAndAddress.uuid(), uuidAndAddress.address() - ).selectAll() - ); - }); - } - SelectionOrderBuilder selectionOrderBuilder = selector.selectionBuilder(); - if (listType == ListType.HISTORY) { - selectionOrderBuilder.selectAll(); + yield historyOrWarns( + argumentParser().parsePlayer(sender(), target), + (uuidAndAddress) -> selector.selectionByApplicabilityBuilder( + uuidAndAddress.uuid(), uuidAndAddress.address() + ) + ); } else { - selectionOrderBuilder.type(PunishmentType.WARN); + yield historyOrWarns( + argumentParser().parseVictim( + sender(), target, ParseVictim.ofPreferredType(Victim.VictimType.PLAYER) + ), + (victim) -> { + // Select punishments made against this user OR against the composite user + CompositeVictim compositeWildcard = new AsCompositeWildcard().apply(victim); + SelectionPredicate victimSelection = matchingAnyOf(victim, compositeWildcard); + return selector.selectionBuilder().victims(victimSelection); + } + ); } - yield argumentParser().parseVictim( - sender(), target, ParseVictim.ofPreferredType(Victim.VictimType.PLAYER) - ).thenCompose((victim) -> { - if (victim == null) { - return completedFuture(null); - } - // Select punishments made against this user OR against the composite user - CompositeVictim compositeWildcard = new AsCompositeWildcard().apply(victim); - SelectionPredicate victimSelection = matchingAnyOf(victim, compositeWildcard); - return parsePageThenExecute(selectionOrderBuilder.victims(victimSelection)); - }); } case BLAME -> { if (!command().hasNext()) { @@ -171,6 +164,23 @@ yield argumentParser().parseOperator( }; } + private ReactionStage historyOrWarns(CentralisedFuture parseVictim, + Function<@NonNull V, SelectionBuilderBase> createSelection) { + return parseVictim.thenCompose((victim) -> { + if (victim == null) { + return completedFuture(null); + } + SelectionBuilderBase selectionBuilder = createSelection.apply(victim); + if (listType == ListType.HISTORY) { + selectionBuilder.selectAll(); + } else { + assert listType == ListType.WARNS; + selectionBuilder.type(PunishmentType.WARN); + } + return parsePageThenExecute(selectionBuilder); + }); + } + private ReactionStage parsePageThenExecute(SelectionBuilderBase selectionBuilder) { int selectedPage = parsePage(); if (selectedPage <= 0) { diff --git a/bans-core/src/main/java/space/arim/libertybans/core/config/MainConfig.java b/bans-core/src/main/java/space/arim/libertybans/core/config/MainConfig.java index 6f345c135..79271957d 100644 --- a/bans-core/src/main/java/space/arim/libertybans/core/config/MainConfig.java +++ b/bans-core/src/main/java/space/arim/libertybans/core/config/MainConfig.java @@ -138,8 +138,8 @@ interface Commands { @ConfKey("show-applicable-for-history") @ConfComments({ - "By default, /history shows all active punishments for the player requested.", - "Enabling this option will make /history scan all punishments which would apply to the player", + "By default, /history and /warns show punishments specifically for the player requested.", + "Enabling this option will make /history and /warn scan punishments which would apply to the player", "", "For example, an IP ban may apply to a player, but it will not be in the player's /history unless this is enabled" }) diff --git a/bans-core/src/main/java/space/arim/libertybans/core/database/JdbcDriver.java b/bans-core/src/main/java/space/arim/libertybans/core/database/JdbcDriver.java index bcedb0733..ba283f757 100644 --- a/bans-core/src/main/java/space/arim/libertybans/core/database/JdbcDriver.java +++ b/bans-core/src/main/java/space/arim/libertybans/core/database/JdbcDriver.java @@ -1,6 +1,6 @@ /* * LibertyBans - * Copyright © 2021 Anand Beh + * Copyright © 2023 Anand Beh * * LibertyBans is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as @@ -24,19 +24,22 @@ import java.util.Map; public enum JdbcDriver { - HSQLDB("org.hsqldb.jdbc.JDBCDriver", "org.hsqldb.jdbc.JDBCDataSource", ';', ';'), - MARIADB_CONNECTOR("org.mariadb.jdbc.Driver", "org.mariadb.jdbc.MariaDbDataSource", '?', '&'), - PG_JDBC("org.postgresql.Driver", "org.postgresql.ds.PGSimpleDataSource", '?', '&'), + H2("org.h2.Driver", "org.h2.jdbcx.JdbcDataSource", "h2", ';', ';'), + HSQLDB("org.hsqldb.jdbc.JDBCDriver", "org.hsqldb.jdbc.JDBCDataSource", "hsqldb", ';', ';'), + MARIADB_CONNECTOR("org.mariadb.jdbc.Driver", "org.mariadb.jdbc.MariaDbDataSource", "mariadb", '?', '&'), + PG_JDBC("org.postgresql.Driver", "org.postgresql.ds.PGSimpleDataSource", "postgresql", '?', '&'), ; private final String driverClassName; private final String dataSourceClassName; + private final String urlPrefix; private final char urlPropertyPrefix; private final char urlPropertySeparator; - private JdbcDriver(String driverClassName, String dataSourceClassName, - char urlPropertyPrefix, char urlPropertySeparator) { + JdbcDriver(String driverClassName, String dataSourceClassName, + String urlPrefix, char urlPropertyPrefix, char urlPropertySeparator) { + this.urlPrefix = urlPrefix; this.driverClassName = driverClassName; this.dataSourceClassName = dataSourceClassName; @@ -44,20 +47,24 @@ private JdbcDriver(String driverClassName, String dataSourceClassName, this.urlPropertySeparator = urlPropertySeparator; } - public String driverClassName() { + String driverClassName() { return driverClassName; } - String dataSourceClassName() { + public String dataSourceClassName() { return dataSourceClassName; } + public boolean ownsUrl(String jdbcUrl) { + return jdbcUrl.startsWith("jdbc:" + urlPrefix); + } + String formatConnectionProperties(Map properties) { if (properties.isEmpty()) { return ""; } List connectProps = new ArrayList<>(properties.size()); - properties.forEach((key, value) -> connectProps.add(key + "=" + value)); + properties.forEach((key, value) -> connectProps.add(key + '=' + value)); return urlPropertyPrefix + String.join(Character.toString(urlPropertySeparator), connectProps); } diff --git a/bans-core/src/main/java/space/arim/libertybans/core/importing/JdbcDetails.java b/bans-core/src/main/java/space/arim/libertybans/core/importing/JdbcDetails.java index 76440ddbf..0c6909ce6 100644 --- a/bans-core/src/main/java/space/arim/libertybans/core/importing/JdbcDetails.java +++ b/bans-core/src/main/java/space/arim/libertybans/core/importing/JdbcDetails.java @@ -1,6 +1,6 @@ /* * LibertyBans - * Copyright © 2020 Anand Beh + * Copyright © 2023 Anand Beh * * LibertyBans is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as @@ -21,6 +21,7 @@ import space.arim.libertybans.core.database.JdbcDriver; +import javax.sql.DataSource; import java.sql.Connection; import java.sql.DriverManager; import java.sql.SQLException; @@ -40,25 +41,35 @@ public JdbcDetails(String jdbcUrl, String username, String password) { @Override public Connection openConnection() throws SQLException { - // Workaround DriverManager not supporting drivers not loaded through system ClassLoader - if (jdbcUrl.startsWith("jdbc:hsqldb")) { - initDriver(JdbcDriver.HSQLDB); - } - if (jdbcUrl.startsWith("jdbc:mariadb")) { - initDriver(JdbcDriver.MARIADB_CONNECTOR); - } - if (jdbcUrl.startsWith("jdbc:postgresql")) { - initDriver(JdbcDriver.PG_JDBC); + // Connect manually if possible, for two reasons + // 1. Workaround DriverManager not supporting drivers not loaded through system ClassLoader + // 2. Avoid polluting the driver registry if we don't have to. Helps the ecosystem + for (JdbcDriver driver : JdbcDriver.values()) { + if (driver.ownsUrl(jdbcUrl)) { + try { + return manualConnect(driver); + } catch (ReflectiveOperationException ex) { + throw new ImportException("Failed to initialize JDBC data source for " + driver, ex); + } + } } + // Scan the global driver registry. This is potentially susceptible to classloading chaos, + // however, this url was requested for importing purposes, so we must serve the request return DriverManager.getConnection(jdbcUrl, username, password); } - private void initDriver(JdbcDriver driver) { - try { - Class.forName(driver.driverClassName()); - } catch (ClassNotFoundException ex) { - throw new ImportException("Failed to initialize JDBC driver " + driver, ex); - } + private Connection manualConnect(JdbcDriver driver) throws SQLException, ReflectiveOperationException { + // Instantiate data source + DataSource dataSource = Class.forName(driver.dataSourceClassName()) + .asSubclass(DataSource.class) + .getDeclaredConstructor() + .newInstance(); + // Set url. All implementations have this method + dataSource.getClass() + .getMethod("setUrl", String.class) + .invoke(dataSource, jdbcUrl); + // Retrieve connection + return dataSource.getConnection(username, password); } @Override diff --git a/bans-env/sponge/src/main/java/space/arim/libertybans/env/sponge/ConnectionListener.java b/bans-env/sponge/src/main/java/space/arim/libertybans/env/sponge/ConnectionListener.java index d11ddfecd..1808b1b17 100644 --- a/bans-env/sponge/src/main/java/space/arim/libertybans/env/sponge/ConnectionListener.java +++ b/bans-env/sponge/src/main/java/space/arim/libertybans/env/sponge/ConnectionListener.java @@ -33,7 +33,7 @@ import java.util.UUID; @Singleton -public final class ConnectionListener extends ParallelisedListener { +public final class ConnectionListener extends ParallelisedListener { private final RegisterListeners registerListeners; private final Guardian guardian; @@ -55,11 +55,7 @@ public void unregister() { } @Listener(order = Order.EARLY) - public void onConnectEarly(ServerSideConnectionEvent.Auth event) { - if (event.isCancelled()) { - debugPrematurelyDenied(event); - return; - } + public void onConnectEarly(ServerSideConnectionEvent.Handshake event) { UUID uuid = event.profile().uniqueId(); String name = event.profile().name().orElseThrow(() -> new IllegalStateException("No name found")); InetAddress address = event.connection().address().getAddress(); @@ -67,19 +63,19 @@ public void onConnectEarly(ServerSideConnectionEvent.Auth event) { } @Override - protected boolean isAllowed(ServerSideConnectionEvent.Auth event) { - return !event.isCancelled(); + protected boolean isAllowed(ServerSideConnectionEvent.Handshake event) { + // No way to check if the connection has been closed by someone else + return true; } @Listener(order = Order.LATE) - public void onConnectLate(ServerSideConnectionEvent.Auth event) { + public void onConnectLate(ServerSideConnectionEvent.Handshake event) { Component message = withdraw(event); if (message == null) { debugResultPermitted(event); return; } - event.setCancelled(true); - event.setMessage(message); + event.connection().close(message); } }