From 3bc1c42f33fb4682bf648a0b6bf96c97618d9186 Mon Sep 17 00:00:00 2001 From: Remy Duijsens Date: Mon, 27 Jul 2026 20:16:26 +0200 Subject: [PATCH 1/8] Contain broken placeholder expansions --- .../api/hook/PlaceholderAPIHook.java | 300 +++++++++++++++++- 1 file changed, 294 insertions(+), 6 deletions(-) diff --git a/serverfeatures-api/src/main/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHook.java b/serverfeatures-api/src/main/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHook.java index b90206a4..4da25b7c 100644 --- a/serverfeatures-api/src/main/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHook.java +++ b/serverfeatures-api/src/main/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHook.java @@ -2,18 +2,53 @@ import org.bukkit.Bukkit; import org.bukkit.entity.Player; +import org.bukkit.plugin.RegisteredServiceProvider; +import org.bukkit.plugin.java.JavaPlugin; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.TreeSet; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.BiConsumer; import java.util.function.BiFunction; +import java.util.function.BooleanSupplier; +import java.util.function.LongSupplier; import java.util.function.Predicate; +import java.util.logging.Level; +import java.util.regex.Matcher; +import java.util.regex.Pattern; -public class PlaceholderAPIHook { +/** Safe boundary around PlaceholderAPI and third-party expansions. */ +public final class PlaceholderAPIHook { + + private static final Pattern PLACEHOLDER_PATTERN = + Pattern.compile("%([A-Za-z0-9]+)(?:_[^%]*)?%"); + private static final Pattern VAULT_ECONOMY_PATTERN = + Pattern.compile("(?i)%vault_eco_[^%]+%"); + private static final long WARNING_INTERVAL_NANOS = TimeUnit.MINUTES.toNanos(5); + private static final int MAX_WARNING_KEYS = 2_048; + private static final ConcurrentMap LAST_WARNINGS = new ConcurrentHashMap<>(); + + private PlaceholderAPIHook() { + } public static String applyPlaceholders(String text, Player player) { return applyPlaceholders( text, player, pluginName -> Bukkit.getPluginManager().isPluginEnabled(pluginName), - me.clip.placeholderapi.PlaceholderAPI::setPlaceholders + me.clip.placeholderapi.PlaceholderAPI::setPlaceholders, + PlaceholderAPIHook::isVaultEconomyAvailable, + PlaceholderAPIHook::logWarning, + System::nanoTime ); } @@ -23,11 +58,264 @@ static String applyPlaceholders( Predicate pluginEnabled, BiFunction resolver ) { - String output = text; - if (player != null && pluginEnabled.test("PlaceholderAPI")) { - output = resolver.apply(player, text); + return applyPlaceholders( + text, + player, + pluginEnabled, + resolver, + () -> true, + (message, failure) -> { }, + System::nanoTime + ); + } + + static String applyPlaceholders( + String text, + Player player, + Predicate pluginEnabled, + BiFunction resolver, + BooleanSupplier vaultEconomyAvailable, + BiConsumer warningSink, + LongSupplier nanoTime + ) { + Objects.requireNonNull(pluginEnabled, "pluginEnabled"); + Objects.requireNonNull(resolver, "resolver"); + Objects.requireNonNull(vaultEconomyAvailable, "vaultEconomyAvailable"); + Objects.requireNonNull(warningSink, "warningSink"); + Objects.requireNonNull(nanoTime, "nanoTime"); + if (text == null || player == null) { + return text; + } + + try { + if (!pluginEnabled.test("PlaceholderAPI")) { + return text; + } + } catch (RuntimeException | LinkageError failure) { + warnRateLimited(text, player, "placeholderapi-state", failure, warningSink, nanoTime.getAsLong()); + return text; + } + + MaskedText input = MaskedText.unchanged(text); + if (VAULT_ECONOMY_PATTERN.matcher(text).find() && !safeEconomyAvailability(vaultEconomyAvailable)) { + input = MaskedText.maskVaultEconomy(text); + warnRateLimited( + text, + player, + "vault-economy-unavailable", + null, + warningSink, + nanoTime.getAsLong() + ); + } + + try { + String resolved = resolver.apply(player, input.masked()); + if (resolved == null) { + throw new IllegalStateException("PlaceholderAPI returned null."); + } + return input.restore(resolved); + } catch (RuntimeException | LinkageError failure) { + warnRateLimited(text, player, failure.getClass().getName(), failure, warningSink, nanoTime.getAsLong()); + return text; + } + } + + private static boolean safeEconomyAvailability(BooleanSupplier availability) { + try { + return availability.getAsBoolean(); + } catch (RuntimeException | LinkageError ignored) { + return false; + } + } + + private static boolean isVaultEconomyAvailable() { + try { + if (!Bukkit.getPluginManager().isPluginEnabled("Vault")) { + return false; + } + Class economyClass = Class.forName( + "net.milkbowl.vault.economy.Economy", + false, + PlaceholderAPIHook.class.getClassLoader() + ); + RegisteredServiceProvider registration = economyRegistration(economyClass); + if (registration == null || registration.getProvider() == null + || registration.getPlugin() == null || !registration.getPlugin().isEnabled()) { + return false; + } + Object provider = registration.getProvider(); + try { + JavaPlugin providingPlugin = JavaPlugin.getProvidingPlugin(provider.getClass()); + if (!providingPlugin.isEnabled()) { + return false; + } + } catch (IllegalArgumentException ignored) { + // Some providers are generated or loaded by a bridge class loader. The service owner check above remains valid. + } + return providerReportsEnabled(provider); + } catch (ClassNotFoundException | RuntimeException | LinkageError ignored) { + return false; } - return output; } + @SuppressWarnings({"rawtypes", "unchecked"}) + private static RegisteredServiceProvider economyRegistration(Class economyClass) { + return Bukkit.getServicesManager().getRegistration((Class) economyClass); + } + + private static boolean providerReportsEnabled(Object provider) { + try { + Method method = provider.getClass().getMethod("isEnabled"); + Object result = method.invoke(provider); + return !(result instanceof Boolean enabled) || enabled; + } catch (NoSuchMethodException ignored) { + return true; + } catch (IllegalAccessException | InvocationTargetException | RuntimeException | LinkageError ignored) { + return false; + } + } + + private static void warnRateLimited( + String text, + Player player, + String reason, + Throwable failure, + BiConsumer warningSink, + long now + ) { + WarningKey key = new WarningKey( + playerKey(player), + expansionKey(text), + text.hashCode(), + text.length(), + reason + ); + AtomicBoolean emit = new AtomicBoolean(); + LAST_WARNINGS.compute(key, (ignored, previous) -> { + if (previous == null || now - previous >= WARNING_INTERVAL_NANOS) { + emit.set(true); + return now; + } + return previous; + }); + trimWarningState(now); + if (!emit.get()) { + return; + } + + String playerName = safePlayerName(player); + String expansions = key.expansions(); + String message = failure == null + ? "Vault economy placeholders were left unresolved for player '" + playerName + + "' because no enabled economy provider is available (expansions: " + expansions + ")." + : "Placeholder resolution failed for player '" + playerName + "' (expansions: " + expansions + + "); keeping the original unresolved message."; + warningSink.accept(message, failure); + } + + private static void trimWarningState(long now) { + if (LAST_WARNINGS.size() <= MAX_WARNING_KEYS) { + return; + } + long staleBefore = now - WARNING_INTERVAL_NANOS * 2; + LAST_WARNINGS.entrySet().removeIf(entry -> entry.getValue() < staleBefore); + if (LAST_WARNINGS.size() <= MAX_WARNING_KEYS) { + return; + } + int toRemove = LAST_WARNINGS.size() - MAX_WARNING_KEYS; + for (WarningKey key : LAST_WARNINGS.keySet()) { + if (toRemove-- <= 0) { + break; + } + LAST_WARNINGS.remove(key); + } + } + + private static String expansionKey(String text) { + Set expansions = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + Matcher matcher = PLACEHOLDER_PATTERN.matcher(text); + while (matcher.find()) { + expansions.add(matcher.group(1).toLowerCase(java.util.Locale.ROOT)); + } + return expansions.isEmpty() ? "unknown" : String.join(",", expansions); + } + + private static String playerKey(Player player) { + try { + UUID uniqueId = player.getUniqueId(); + if (uniqueId != null) { + return uniqueId.toString(); + } + } catch (RuntimeException ignored) { + // Fall through to a stable best-effort key. + } + String name = safePlayerName(player); + return name + "@" + Integer.toUnsignedString(System.identityHashCode(player)); + } + + private static String safePlayerName(Player player) { + try { + String name = player.getName(); + if (name != null && !name.isBlank()) { + return name; + } + } catch (RuntimeException ignored) { + // Use a non-identifying fallback. + } + return "unknown"; + } + + private static void logWarning(String message, Throwable failure) { + if (failure == null) { + Bukkit.getLogger().warning("[ServerFeatures] " + message); + } else { + Bukkit.getLogger().log(Level.WARNING, "[ServerFeatures] " + message, failure); + } + } + + static void clearWarningStateForTests() { + LAST_WARNINGS.clear(); + } + + private record WarningKey( + String player, + String expansions, + int messageHash, + int messageLength, + String reason + ) { + } + + private record MaskedText(String masked, Map replacements) { + private static MaskedText unchanged(String text) { + return new MaskedText(text, Map.of()); + } + + private static MaskedText maskVaultEconomy(String text) { + Matcher matcher = VAULT_ECONOMY_PATTERN.matcher(text); + StringBuffer masked = new StringBuffer(text.length()); + Map replacements = new LinkedHashMap<>(); + int index = 0; + while (matcher.find()) { + String token; + do { + token = "__SERVERFEATURES_VAULT_ECONOMY_" + index++ + "_" + + Integer.toUnsignedString(text.hashCode()) + "__"; + } while (text.contains(token) || replacements.containsKey(token)); + replacements.put(token, matcher.group()); + matcher.appendReplacement(masked, Matcher.quoteReplacement(token)); + } + matcher.appendTail(masked); + return new MaskedText(masked.toString(), Map.copyOf(replacements)); + } + + private String restore(String resolved) { + String restored = resolved; + for (Map.Entry replacement : replacements.entrySet()) { + restored = restored.replace(replacement.getKey(), replacement.getValue()); + } + return restored; + } + } } From c90f1e86ec8164c6007fb31041d5ee7bf3887f63 Mon Sep 17 00:00:00 2001 From: Remy Duijsens Date: Mon, 27 Jul 2026 20:17:04 +0200 Subject: [PATCH 2/8] Test placeholder failure isolation and Vault guarding --- .../api/hook/PlaceholderAPIHookTest.java | 106 +++++++++++++++++- 1 file changed, 102 insertions(+), 4 deletions(-) diff --git a/serverfeatures-api/src/test/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHookTest.java b/serverfeatures-api/src/test/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHookTest.java index b849050f..bba4815e 100644 --- a/serverfeatures-api/src/test/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHookTest.java +++ b/serverfeatures-api/src/test/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHookTest.java @@ -2,10 +2,15 @@ import nl.hauntedmc.serverfeatures.util.InterfaceProxy; import org.bukkit.entity.Player; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.util.Map; +import java.util.UUID; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -13,6 +18,11 @@ class PlaceholderAPIHookTest { + @BeforeEach + void resetWarningLimiter() { + PlaceholderAPIHook.clearWarningStateForTests(); + } + @Test void returnsOriginalTextWhenPlayerIsNull() { AtomicBoolean resolverCalled = new AtomicBoolean(false); @@ -33,14 +43,14 @@ void returnsOriginalTextWhenPlayerIsNull() { @Test void returnsOriginalTextWhenPluginIsDisabled() { - Player player = InterfaceProxy.of(Player.class, Map.of()); + Player player = player("Remy"); AtomicBoolean resolverCalled = new AtomicBoolean(false); String output = PlaceholderAPIHook.applyPlaceholders( "hello", player, name -> false, - (p, text) -> { + (ignored, text) -> { resolverCalled.set(true); return "changed"; } @@ -52,16 +62,104 @@ void returnsOriginalTextWhenPluginIsDisabled() { @Test void appliesResolverWhenPluginIsEnabledAndPlayerExists() { - Player player = InterfaceProxy.of(Player.class, Map.of("getName", args -> "Remy")); + Player player = player("Remy"); String output = PlaceholderAPIHook.applyPlaceholders( "hello %player%", player, name -> "PlaceholderAPI".equals(name), - (p, text) -> text.replace("%player%", p.getName()) + (resolvedPlayer, text) -> text.replace("%player%", resolvedPlayer.getName()) ); assertEquals("hello Remy", output); assertTrue(output.contains("Remy")); } + + @Test + void preservesOriginalMessageAndRateLimitsExpansionFailures() { + Player player = player("Remy"); + AtomicInteger warnings = new AtomicInteger(); + AtomicLong now = new AtomicLong(); + String input = "Balance: %vault_eco_balance%"; + + for (int attempt = 0; attempt < 2; attempt++) { + assertEquals(input, PlaceholderAPIHook.applyPlaceholders( + input, + player, + name -> true, + (ignored, text) -> { + throw new IllegalStateException("broken expansion"); + }, + () -> true, + (message, failure) -> warnings.incrementAndGet(), + now::get + )); + } + assertEquals(1, warnings.get()); + + now.addAndGet(TimeUnit.MINUTES.toNanos(5)); + assertEquals(input, PlaceholderAPIHook.applyPlaceholders( + input, + player, + name -> true, + (ignored, text) -> { + throw new NoSuchMethodError("incompatible expansion"); + }, + () -> true, + (message, failure) -> warnings.incrementAndGet(), + now::get + )); + assertEquals(2, warnings.get()); + } + + @Test + void leavesVaultEconomyPlaceholdersUnresolvedWhenProviderIsUnavailable() { + Player player = player("Remy"); + AtomicBoolean resolverSawVaultPlaceholder = new AtomicBoolean(); + AtomicInteger warnings = new AtomicInteger(); + + String output = PlaceholderAPIHook.applyPlaceholders( + "%player_name% has %vault_eco_balance%", + player, + name -> true, + (ignored, text) -> { + resolverSawVaultPlaceholder.set(text.contains("%vault_eco_")); + return text.replace("%player_name%", "Remy"); + }, + () -> false, + (message, failure) -> warnings.incrementAndGet(), + System::nanoTime + ); + + assertEquals("Remy has %vault_eco_balance%", output); + assertFalse(resolverSawVaultPlaceholder.get()); + assertEquals(1, warnings.get()); + } + + @Test + void resolvesVaultEconomyPlaceholdersWhenProviderIsAvailable() { + Player player = player("Remy"); + + String output = PlaceholderAPIHook.applyPlaceholders( + "%player_name% has %vault_eco_balance%", + player, + name -> true, + (ignored, text) -> text + .replace("%player_name%", "Remy") + .replace("%vault_eco_balance%", "125.00"), + () -> true, + (message, failure) -> { }, + System::nanoTime + ); + + assertEquals("Remy has 125.00", output); + } + + private static Player player(String name) { + UUID uniqueId = UUID.nameUUIDFromBytes(name.getBytes(java.nio.charset.StandardCharsets.UTF_8)); + return InterfaceProxy.of(Player.class, Map.of( + "getName", arguments -> name, + "getUniqueId", arguments -> uniqueId + )); + } } From 04484cc1da0793f7d0e841886a3a478378056746 Mon Sep 17 00:00:00 2001 From: Remy Duijsens Date: Mon, 27 Jul 2026 20:18:09 +0200 Subject: [PATCH 3/8] Isolate scoreboard rendering failures per player and line --- .../internal/ScoreboardHandler.java | 190 ++++++++++++++---- 1 file changed, 155 insertions(+), 35 deletions(-) diff --git a/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/internal/ScoreboardHandler.java b/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/internal/ScoreboardHandler.java index 6a447c34..7f3b3ebd 100644 --- a/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/internal/ScoreboardHandler.java +++ b/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/internal/ScoreboardHandler.java @@ -12,17 +12,24 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.BiConsumer; +import java.util.function.Consumer; +import java.util.logging.Level; -/** - * Generates the localized lines each tick and delegates to ScoreboardManager. - */ +/** Generates localized scoreboard content and isolates third-party rendering failures. */ public class ScoreboardHandler { private static final int MAX_LINES = 15; + private static final long WARNING_INTERVAL_NANOS = TimeUnit.MINUTES.toNanos(5); private final Scoreboard feature; private final LocalizationHandler i18n; - private final Map> lastScoreboardLines = new ConcurrentHashMap<>(); + private final Map lastScoreboards = new ConcurrentHashMap<>(); + private final ConcurrentMap lastWarnings = new ConcurrentHashMap<>(); private final int refreshInterval; public ScoreboardHandler(Scoreboard feature) { @@ -31,54 +38,167 @@ public ScoreboardHandler(Scoreboard feature) { this.refreshInterval = (int) feature.getConfigHandler().get("refresh_interval"); } - /** - * Immediately recalculates and pushes the sidebar for one player. - */ + /** Immediately recalculates and pushes the sidebar for one player. */ public void updateScoreboardContent(Player player) { - Component title = i18n.getMessage("scoreboard.title").forAudience(player).build(); + Component title = renderMessageSafely("scoreboard.title", player); + if (title == null) { + title = Component.empty(); + } + List lines = new ArrayList<>(); - for (int i = 1; i <= MAX_LINES; i++) { - Component c = i18n.getMessage("scoreboard.line" + i).forAudience(player).build(); - String s = ComponentFormatter.serialize(c).format(ComponentFormatter.Serializer.Format.PLAIN).build(); - if (s.startsWith("")) break; - lines.add(c); + for (int lineNumber = 1; lineNumber <= MAX_LINES; lineNumber++) { + String messageKey = "scoreboard.line" + lineNumber; + Component line = renderMessageSafely(messageKey, player); + if (line == null) { + continue; + } + String plain; + try { + plain = ComponentFormatter.serialize(line) + .format(ComponentFormatter.Serializer.Format.PLAIN) + .build(); + } catch (RuntimeException | LinkageError failure) { + reportFailure(player, messageKey + ".serialize", failure); + continue; + } + if (plain.startsWith("")) { + break; + } + lines.add(line); } - List oldLines = lastScoreboardLines.get(player); - if (oldLines != null && oldLines.equals(lines)) { + UUID playerId = player.getUniqueId(); + ScoreboardSnapshot previous = lastScoreboards.get(playerId); + ScoreboardSnapshot next = new ScoreboardSnapshot(title, List.copyOf(lines)); + if (next.equals(previous)) { return; } - lastScoreboardLines.put(player, new ArrayList<>(lines)); - ScoreboardManager.updateSidebar(player, title, lines, oldLines); + + ScoreboardManager.updateSidebar( + player, + title, + lines, + previous == null ? null : previous.lines() + ); + lastScoreboards.put(playerId, next); } - /** - * Runs forceUpdate once every `refreshInterval` ticks for all online players - */ + /** Updates one player without allowing a rendering or provider failure to escape to an event. */ + public void updateScoreboardSafely(Player player) { + try { + updateScoreboardContent(player); + } catch (RuntimeException | LinkageError failure) { + reportFailure(player, "scoreboard.update", failure); + } + } + + /** Runs one independently guarded update for every online player. */ public void startUpdater() { feature.getLifecycleManager().getTaskManager() - .scheduleRepeatingTask(() -> - Bukkit.getOnlinePlayers().forEach(this::updateScoreboardContent), + .scheduleRepeatingTask(() -> updatePlayersIndependently( + Bukkit.getOnlinePlayers(), + this::updateScoreboardContent, + (player, failure) -> reportFailure(player, "scoreboard.update", failure) + ), BukkitTime.ticks(0L), BukkitTime.ticks(refreshInterval)); } - /** - * Removes the player's scoreboard from the handler. - * - * @param player the player to remove - */ + /** Removes the player's scoreboard from the handler. */ public void removePlayer(Player player) { - lastScoreboardLines.remove(player); - ScoreboardManager.removeSidebar(player); + UUID playerId = player.getUniqueId(); + lastScoreboards.remove(playerId); + lastWarnings.keySet().removeIf(key -> key.playerId().equals(playerId)); + try { + ScoreboardManager.removeSidebar(player); + } catch (RuntimeException | LinkageError failure) { + reportFailure(player, "scoreboard.remove", failure); + } } - /** - * Removes all players from scoreboard tracking and resets their scoreboards to the main scoreboard. - */ + /** Removes all tracked sidebars while isolating failures per player. */ public void removeAllPlayers() { - lastScoreboardLines.clear(); - for (Player player : Bukkit.getOnlinePlayers()) { - ScoreboardManager.removeSidebar(player); + lastScoreboards.clear(); + lastWarnings.clear(); + updatePlayersIndependently( + Bukkit.getOnlinePlayers(), + ScoreboardManager::removeSidebar, + (player, failure) -> reportFailure(player, "scoreboard.remove", failure) + ); + } + + private Component renderMessageSafely(String messageKey, Player player) { + try { + return i18n.getMessage(messageKey).forAudience(player).build(); + } catch (RuntimeException | LinkageError failure) { + reportFailure(player, messageKey, failure); + return null; + } + } + + private void reportFailure(Player player, String messageKey, Throwable failure) { + UUID playerId = safePlayerId(player); + FailureKey key = new FailureKey(playerId, messageKey, failure.getClass().getName()); + long now = System.nanoTime(); + AtomicBoolean emit = new AtomicBoolean(); + lastWarnings.compute(key, (ignored, previous) -> { + if (previous == null || now - previous >= WARNING_INTERVAL_NANOS) { + emit.set(true); + return now; + } + return previous; + }); + if (!emit.get()) { + return; + } + feature.getPlugin().getLogger().log( + Level.WARNING, + "[Scoreboard] Failed to render '" + messageKey + "' for player '" + + safePlayerName(player) + "'; the remaining scoreboard update will continue.", + failure + ); + } + + private static UUID safePlayerId(Player player) { + try { + UUID uniqueId = player.getUniqueId(); + if (uniqueId != null) { + return uniqueId; + } + } catch (RuntimeException ignored) { + // Use an instance-derived UUID solely for warning suppression. + } + return new UUID(0L, Integer.toUnsignedLong(System.identityHashCode(player))); + } + + private static String safePlayerName(Player player) { + try { + String name = player.getName(); + if (name != null && !name.isBlank()) { + return name; + } + } catch (RuntimeException ignored) { + // Use a non-identifying fallback. } + return "unknown"; + } + + static void updatePlayersIndependently( + Iterable players, + Consumer updater, + BiConsumer failureHandler + ) { + for (Player player : players) { + try { + updater.accept(player); + } catch (RuntimeException | LinkageError failure) { + failureHandler.accept(player, failure); + } + } + } + + private record ScoreboardSnapshot(Component title, List lines) { + } + + private record FailureKey(UUID playerId, String messageKey, String failureType) { } } From 9b2e1de9e223af9d0acbbbdd3456f54fa3bdae55 Mon Sep 17 00:00:00 2001 From: Remy Duijsens Date: Mon, 27 Jul 2026 20:18:18 +0200 Subject: [PATCH 4/8] Keep scoreboard failures out of player lifecycle events --- .../features/scoreboard/listener/PlayerJoinListener.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/listener/PlayerJoinListener.java b/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/listener/PlayerJoinListener.java index 81d22caf..3a876aed 100644 --- a/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/listener/PlayerJoinListener.java +++ b/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/listener/PlayerJoinListener.java @@ -17,12 +17,11 @@ public PlayerJoinListener(ScoreboardHandler scoreboardHandler) { @EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true) public void onPlayerJoin(PlayerJoinEvent event) { - scoreboardHandler.updateScoreboardContent(event.getPlayer()); + scoreboardHandler.updateScoreboardSafely(event.getPlayer()); } @EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true) public void onPlayerQuit(PlayerQuitEvent event) { scoreboardHandler.removePlayer(event.getPlayer()); } - } From 193f3c1963e7171826583e9986a8c0a9e5634cfd Mon Sep 17 00:00:00 2001 From: Remy Duijsens Date: Mon, 27 Jul 2026 20:18:34 +0200 Subject: [PATCH 5/8] Use guarded scoreboard updates during initialization --- .../features/scoreboard/Scoreboard.java | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/Scoreboard.java b/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/Scoreboard.java index cd143e75..7c5b24b9 100644 --- a/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/Scoreboard.java +++ b/serverfeatures-platform-paper/src/main/java/nl/hauntedmc/serverfeatures/features/scoreboard/Scoreboard.java @@ -1,9 +1,9 @@ package nl.hauntedmc.serverfeatures.features.scoreboard; -import nl.hauntedmc.serverfeatures.features.FeatureContext; import nl.hauntedmc.serverfeatures.api.io.config.ConfigMap; import nl.hauntedmc.serverfeatures.api.io.localization.MessageMap; import nl.hauntedmc.serverfeatures.features.BukkitBaseFeature; +import nl.hauntedmc.serverfeatures.features.FeatureContext; import nl.hauntedmc.serverfeatures.features.scoreboard.internal.ScoreboardHandler; import nl.hauntedmc.serverfeatures.features.scoreboard.listener.PlayerJoinListener; import nl.hauntedmc.serverfeatures.features.scoreboard.meta.Meta; @@ -23,7 +23,6 @@ public ConfigMap getDefaultConfig() { defaults.put("enabled", false); defaults.put("refresh_interval", 100); return defaults; - } @Override @@ -48,20 +47,18 @@ public MessageMap getDefaultMessages() { return messages; } - @Override public void initialize() { scoreboardHandler = new ScoreboardHandler(this); scoreboardHandler.startUpdater(); getLifecycleManager().getListenerManager().registerListener(new PlayerJoinListener(scoreboardHandler)); - // Initialize the scoreboard for all currently online players. - Bukkit.getOnlinePlayers().forEach(scoreboardHandler::updateScoreboardContent); + // Initialize each online player independently so one broken expansion cannot abort the feature. + Bukkit.getOnlinePlayers().forEach(scoreboardHandler::updateScoreboardSafely); } @Override public void disable() { scoreboardHandler.removeAllPlayers(); } - -} \ No newline at end of file +} From 733bd3735d749adaad84e9a190541d0fd39684fd Mon Sep 17 00:00:00 2001 From: Remy Duijsens Date: Mon, 27 Jul 2026 20:18:54 +0200 Subject: [PATCH 6/8] Test per-player scoreboard update isolation --- .../internal/ScoreboardHandlerTest.java | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 serverfeatures-platform-paper/src/test/java/nl/hauntedmc/serverfeatures/features/scoreboard/internal/ScoreboardHandlerTest.java diff --git a/serverfeatures-platform-paper/src/test/java/nl/hauntedmc/serverfeatures/features/scoreboard/internal/ScoreboardHandlerTest.java b/serverfeatures-platform-paper/src/test/java/nl/hauntedmc/serverfeatures/features/scoreboard/internal/ScoreboardHandlerTest.java new file mode 100644 index 00000000..027f5cc8 --- /dev/null +++ b/serverfeatures-platform-paper/src/test/java/nl/hauntedmc/serverfeatures/features/scoreboard/internal/ScoreboardHandlerTest.java @@ -0,0 +1,44 @@ +package nl.hauntedmc.serverfeatures.features.scoreboard.internal; + +import nl.hauntedmc.serverfeatures.util.InterfaceProxy; +import org.bukkit.entity.Player; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class ScoreboardHandlerTest { + + @Test + void continuesUpdatingOtherPlayersAfterRuntimeAndLinkageFailures() { + Player runtimeFailure = player("runtime"); + Player successful = player("successful"); + Player linkageFailure = player("linkage"); + AtomicInteger updates = new AtomicInteger(); + AtomicInteger failures = new AtomicInteger(); + + ScoreboardHandler.updatePlayersIndependently( + List.of(runtimeFailure, successful, linkageFailure), + player -> { + if (player == runtimeFailure) { + throw new IllegalStateException("broken expansion"); + } + if (player == linkageFailure) { + throw new NoSuchMethodError("incompatible expansion"); + } + updates.incrementAndGet(); + }, + (player, failure) -> failures.incrementAndGet() + ); + + assertEquals(1, updates.get()); + assertEquals(2, failures.get()); + } + + private static Player player(String name) { + return InterfaceProxy.of(Player.class, Map.of("getName", arguments -> name)); + } +} From 1c30ce340e7201021a0ff576f5cfa7084404721c Mon Sep 17 00:00:00 2001 From: Remy Duijsens Date: Mon, 27 Jul 2026 20:24:16 +0200 Subject: [PATCH 7/8] Rate-limit placeholder warnings strictly per message --- .../api/hook/PlaceholderAPIHook.java | 22 +++++-------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/serverfeatures-api/src/main/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHook.java b/serverfeatures-api/src/main/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHook.java index 4da25b7c..a0a96119 100644 --- a/serverfeatures-api/src/main/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHook.java +++ b/serverfeatures-api/src/main/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHook.java @@ -92,21 +92,14 @@ static String applyPlaceholders( return text; } } catch (RuntimeException | LinkageError failure) { - warnRateLimited(text, player, "placeholderapi-state", failure, warningSink, nanoTime.getAsLong()); + warnRateLimited(text, player, failure, warningSink, nanoTime.getAsLong()); return text; } MaskedText input = MaskedText.unchanged(text); if (VAULT_ECONOMY_PATTERN.matcher(text).find() && !safeEconomyAvailability(vaultEconomyAvailable)) { input = MaskedText.maskVaultEconomy(text); - warnRateLimited( - text, - player, - "vault-economy-unavailable", - null, - warningSink, - nanoTime.getAsLong() - ); + warnRateLimited(text, player, null, warningSink, nanoTime.getAsLong()); } try { @@ -116,7 +109,7 @@ static String applyPlaceholders( } return input.restore(resolved); } catch (RuntimeException | LinkageError failure) { - warnRateLimited(text, player, failure.getClass().getName(), failure, warningSink, nanoTime.getAsLong()); + warnRateLimited(text, player, failure, warningSink, nanoTime.getAsLong()); return text; } } @@ -151,7 +144,7 @@ private static boolean isVaultEconomyAvailable() { return false; } } catch (IllegalArgumentException ignored) { - // Some providers are generated or loaded by a bridge class loader. The service owner check above remains valid. + // Generated providers can use a bridge class loader; the service owner check above remains valid. } return providerReportsEnabled(provider); } catch (ClassNotFoundException | RuntimeException | LinkageError ignored) { @@ -179,7 +172,6 @@ private static boolean providerReportsEnabled(Object provider) { private static void warnRateLimited( String text, Player player, - String reason, Throwable failure, BiConsumer warningSink, long now @@ -188,8 +180,7 @@ private static void warnRateLimited( playerKey(player), expansionKey(text), text.hashCode(), - text.length(), - reason + text.length() ); AtomicBoolean emit = new AtomicBoolean(); LAST_WARNINGS.compute(key, (ignored, previous) -> { @@ -282,8 +273,7 @@ private record WarningKey( String player, String expansions, int messageHash, - int messageLength, - String reason + int messageLength ) { } From 686fb9ecbec50001d263a1e0398b11d25b1cdb3a Mon Sep 17 00:00:00 2001 From: Remy Duijsens Date: Mon, 27 Jul 2026 20:24:52 +0200 Subject: [PATCH 8/8] Cover strict warning windows and linkage failures --- .../api/hook/PlaceholderAPIHookTest.java | 51 +++++++++++++------ 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/serverfeatures-api/src/test/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHookTest.java b/serverfeatures-api/src/test/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHookTest.java index bba4815e..5f4336ee 100644 --- a/serverfeatures-api/src/test/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHookTest.java +++ b/serverfeatures-api/src/test/java/nl/hauntedmc/serverfeatures/api/hook/PlaceholderAPIHookTest.java @@ -83,22 +83,22 @@ void preservesOriginalMessageAndRateLimitsExpansionFailures() { String input = "Balance: %vault_eco_balance%"; for (int attempt = 0; attempt < 2; attempt++) { - assertEquals(input, PlaceholderAPIHook.applyPlaceholders( - input, - player, - name -> true, - (ignored, text) -> { - throw new IllegalStateException("broken expansion"); - }, - () -> true, - (message, failure) -> warnings.incrementAndGet(), - now::get - )); + assertEquals(input, failingResolution(input, player, warnings, now)); } assertEquals(1, warnings.get()); now.addAndGet(TimeUnit.MINUTES.toNanos(5)); - assertEquals(input, PlaceholderAPIHook.applyPlaceholders( + assertEquals(input, failingResolution(input, player, warnings, now)); + assertEquals(2, warnings.get()); + } + + @Test + void containsBinaryIncompatibilityErrorsFromExpansions() { + Player player = player("Remy"); + AtomicInteger warnings = new AtomicInteger(); + String input = "Balance: %vault_eco_balance%"; + + String output = PlaceholderAPIHook.applyPlaceholders( input, player, name -> true, @@ -107,9 +107,11 @@ void preservesOriginalMessageAndRateLimitsExpansionFailures() { }, () -> true, (message, failure) -> warnings.incrementAndGet(), - now::get - )); - assertEquals(2, warnings.get()); + System::nanoTime + ); + + assertEquals(input, output); + assertEquals(1, warnings.get()); } @Test @@ -155,6 +157,25 @@ void resolvesVaultEconomyPlaceholdersWhenProviderIsAvailable() { assertEquals("Remy has 125.00", output); } + private static String failingResolution( + String input, + Player player, + AtomicInteger warnings, + AtomicLong now + ) { + return PlaceholderAPIHook.applyPlaceholders( + input, + player, + name -> true, + (ignored, text) -> { + throw new IllegalStateException("broken expansion"); + }, + () -> true, + (message, failure) -> warnings.incrementAndGet(), + now::get + ); + } + private static Player player(String name) { UUID uniqueId = UUID.nameUUIDFromBytes(name.getBytes(java.nio.charset.StandardCharsets.UTF_8)); return InterfaceProxy.of(Player.class, Map.of(