R3944Realms/PlayerSync
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Final audit: fix ghost-online, SQL injection, resource leak, NPE

Browse Source 
CRITICAL-1/2: Remove duplicate online=1 writes from doPlayerJoin.
The synchronous onPlayerLoggedInKickCheck already sets online=1.
The background thread writes raced with logout's online=0, permanently
locking players as "online" after crash-disconnect during join.

HIGH-1: Startup SQL uses PreparedStatement for server_id (was string concat).
HIGH-2: update() method now uses try-with-resources for PreparedStatement.
HIGH-3: NPE guard in RS2 data file logging when getRS2DataFile returns null.

Vyrriox
This commit is contained in:
laforetbrut 2026-03-26 18:33:00 +01:00
parent b1563cc9ae
commit 484f1a8c05
4 changed files with 9 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/main/java/vip/fubuki/playersync/PlayerSync.java
View File

@ -204,7 +204,7 @@ public class PlayerSync {
);
try {
JDBCsetUp.executeUpdate("UPDATE player_data SET online=0 WHERE last_server=" + JdbcConfig.SERVER_ID.get() +" AND online=1 LIMIT 1000");
JDBCsetUp.executePreparedUpdate("UPDATE player_data SET online=0 WHERE last_server=? AND online=1 LIMIT 1000", JdbcConfig.SERVER_ID.get());
} catch (Exception e) {
LOGGER.error("An exception occurred while trying change wrong player-status\n" + e.getMessage());
}

10
src/main/java/vip/fubuki/playersync/sync/VanillaSync.java
View File

@ -269,12 +269,13 @@ public class VanillaSync {
}
if (!playerExists) {
// FIX CRITICAL-1/2: online=1 is already set by onPlayerLoggedInKickCheck (synchronous).
// Do NOT write online=1 again from background/queued threads - if the player disconnects
// quickly, the background write races with logout's online=0 and permanently locks the player.
server.execute(() -> {
try {
new ModsSupport().doCuriosRestore(serverPlayer);
store(serverPlayer, true);
JDBCsetUp.executePreparedUpdate("UPDATE server_info SET last_update=? WHERE id=?", System.currentTimeMillis(), JdbcConfig.SERVER_ID.get());
JDBCsetUp.executePreparedUpdate("UPDATE player_data SET online=1, last_server=? WHERE uuid=?", JdbcConfig.SERVER_ID.get(), player_uuid);
store(serverPlayer, true); // INSERT with online=1 handled by store() init path
serverPlayer.addTag("player_synced");
} catch (Exception e) {
PlayerSync.LOGGER.error("Error initializing new player {}", player_uuid, e);
@ -285,8 +286,7 @@ public class VanillaSync {
return;
}
JDBCsetUp.executePreparedUpdate("UPDATE server_info SET last_update=? WHERE id=?", System.currentTimeMillis(), JdbcConfig.SERVER_ID.get());
JDBCsetUp.executePreparedUpdate("UPDATE player_data SET online=1, last_server=? WHERE uuid=?", JdbcConfig.SERVER_ID.get(), player_uuid);
// online=1 already set by onPlayerLoggedInKickCheck - no duplicate write here
// Read all DB data into local variables (background thread - safe)
final int health, foodLevel, xp, score;

2
src/main/java/vip/fubuki/playersync/sync/addons/ModsSupport.java
View File

@ -462,7 +462,7 @@ public class ModsSupport {
// Read the .dat file directly (getDataFile is private, use reflection)
java.io.File datFile = getRS2DataFile(sp);
if (datFile == null || !datFile.exists()) {
PlayerSync.LOGGER.warn("RS2 storage data file not found: {}", datFile.getAbsolutePath());
PlayerSync.LOGGER.warn("RS2 storage data file not found: {}", datFile != null ? datFile.getAbsolutePath() : "<null>");
return;
}

4
src/main/java/vip/fubuki/playersync/util/JDBCsetUp.java
View File

@ -90,8 +90,8 @@ public class JDBCsetUp {
*/
public static void update(String sql, String... argument) throws SQLException {
LOGGER.trace(sql);
try (Connection connection = getConnection()) { // With database selected
PreparedStatement updateStatement = connection.prepareStatement(sql);
try (Connection connection = getConnection();
PreparedStatement updateStatement = connection.prepareStatement(sql)) {
for (int i = 0; i < argument.length; i++) {
updateStatement.setString(i + 1, argument[i]);
}