diff --git a/dataprovider-platform-paper/src/main/java/nl/hauntedmc/dataprovider/platform/bukkit/identity/BukkitCallerContextResolver.java b/dataprovider-platform-paper/src/main/java/nl/hauntedmc/dataprovider/platform/bukkit/identity/BukkitCallerContextResolver.java index 5c31752..3405627 100644 --- a/dataprovider-platform-paper/src/main/java/nl/hauntedmc/dataprovider/platform/bukkit/identity/BukkitCallerContextResolver.java +++ b/dataprovider-platform-paper/src/main/java/nl/hauntedmc/dataprovider/platform/bukkit/identity/BukkitCallerContextResolver.java @@ -10,7 +10,10 @@ import org.bukkit.plugin.Plugin; import java.util.List; +import java.util.Locale; import java.util.Objects; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; /** @@ -19,6 +22,7 @@ public final class BukkitCallerContextResolver implements CallerContextResolver { private final PluginIdentityRegistry identities = new PluginIdentityRegistry(); + private final Set installedPluginIds = ConcurrentHashMap.newKeySet(); private final Supplier> callerChain; public BukkitCallerContextResolver(ClassLoader ownClassLoader) { @@ -58,23 +62,38 @@ public CallerContext resolveCallerIfPresent() { @Override public boolean isKnownPlugin(String pluginId) { - return identities.isKnownPlugin(pluginId); + if (pluginId == null || pluginId.isBlank()) { + return false; + } + return installedPluginIds.contains(normalizePluginId(pluginId)); } - /** Called from Paper's lifecycle thread before APIs are handed to plugins. */ + /** + * Called from Paper's lifecycle thread before APIs are handed to plugins. + * + *

All installed plugin names must be known here, not only plugins that have already completed + * {@code onEnable}. Access policies are configuration declarations and may legitimately name a + * plugin that enables later in Paper's dependency order. Only enabled plugins receive an active + * lifecycle identity; disabled plugins are known solely for configuration validation.

+ */ public void synchronizePlugins() { - for (Plugin plugin : Bukkit.getPluginManager().getPlugins()) { + synchronizePlugins(List.of(Bukkit.getPluginManager().getPlugins())); + } + + void synchronizePlugins(Iterable plugins) { + Objects.requireNonNull(plugins, "Plugins cannot be null."); + for (Plugin plugin : plugins) { + rememberInstalled(plugin); if (plugin.isEnabled()) { - register(plugin); + registerActiveIdentity(plugin); } } } public PluginIdentity register(Plugin plugin) { Objects.requireNonNull(plugin, "Plugin cannot be null."); - ClassLoader classLoader = plugin.getClass().getClassLoader(); - PluginIdentity existing = identities.find(classLoader); - return existing != null ? existing : identities.register(plugin.getName(), classLoader); + rememberInstalled(plugin); + return registerActiveIdentity(plugin); } public void invalidate(Plugin plugin) { @@ -89,6 +108,7 @@ public PluginIdentity find(Plugin plugin) { public void invalidateAll() { identities.invalidateAll(); + installedPluginIds.clear(); } @Override @@ -102,7 +122,7 @@ public PluginIdentity issueIdentity(Object platformPlugin) { // Bukkit fires PluginEnableEvent after JavaPlugin.onEnable. Register here as // well so a plugin can bind the API from its own onEnable callback. PluginIdentity identity = register(plugin); - if (identity == null || !identity.pluginId().equals(plugin.getName().trim().toLowerCase(java.util.Locale.ROOT))) { + if (!identity.pluginId().equals(normalizePluginId(plugin.getName()))) { throw new SecurityException("Bukkit plugin is not active in DataProvider's identity registry."); } requireBindingCaller(identity); @@ -114,10 +134,39 @@ public boolean isIdentityActive(PluginIdentity identity) { return identities.isActive(identity); } + private PluginIdentity registerActiveIdentity(Plugin plugin) { + String pluginId = normalizePluginId(plugin.getName()); + ClassLoader classLoader = plugin.getClass().getClassLoader(); + PluginIdentity existing = identities.find(classLoader); + PluginIdentity identity = existing != null ? existing : identities.register(pluginId, classLoader); + if (!identity.pluginId().equals(pluginId)) { + throw new IllegalStateException( + "Cannot securely distinguish Bukkit plugins '" + identity.pluginId() + "' and '" + + pluginId + "' because they share one class loader." + ); + } + return identity; + } + + private void rememberInstalled(Plugin plugin) { + Objects.requireNonNull(plugin, "Plugin cannot be null."); + installedPluginIds.add(normalizePluginId(plugin.getName())); + } + private void requireBindingCaller(PluginIdentity identity) { CallerContext caller = resolveCaller(); if (!identity.pluginId().equals(caller.pluginId()) || identity.classLoader() != caller.classLoader()) { throw new SecurityException("A Bukkit plugin can bind DataProvider only to its own plugin instance."); } } + + private static String normalizePluginId(String pluginId) { + String normalized = Objects.requireNonNull(pluginId, "Plugin id cannot be null.") + .trim() + .toLowerCase(Locale.ROOT); + if (normalized.isEmpty()) { + throw new IllegalArgumentException("Plugin id cannot be blank."); + } + return normalized; + } } diff --git a/dataprovider-platform-paper/src/test/java/nl/hauntedmc/dataprovider/platform/bukkit/identity/BukkitCallerContextResolverTest.java b/dataprovider-platform-paper/src/test/java/nl/hauntedmc/dataprovider/platform/bukkit/identity/BukkitCallerContextResolverTest.java index 4b580d8..a608725 100644 --- a/dataprovider-platform-paper/src/test/java/nl/hauntedmc/dataprovider/platform/bukkit/identity/BukkitCallerContextResolverTest.java +++ b/dataprovider-platform-paper/src/test/java/nl/hauntedmc/dataprovider/platform/bukkit/identity/BukkitCallerContextResolverTest.java @@ -7,18 +7,19 @@ import java.util.List; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; class BukkitCallerContextResolverTest { @Test - void issuesAndInvalidatesLifecycleIdentityWithoutBukkitAccessDuringUse() { + void invalidatesLifecycleIdentityButRetainsInstalledPluginKnowledge() { Plugin plugin = mock(Plugin.class); BukkitCallerContextResolver resolver = resolverFor(plugin.getClass().getClassLoader()); when(plugin.getName()).thenReturn("Example"); @@ -34,10 +35,47 @@ void issuesAndInvalidatesLifecycleIdentityWithoutBukkitAccessDuringUse() { verifyNoInteractions(plugin); resolver.invalidate(plugin); + assertFalse(resolver.isIdentityActive(identity)); + assertTrue(resolver.isKnownPlugin("example")); + } + + @Test + void invalidateAllClearsLifecycleAndInstalledPluginKnowledge() { + Plugin plugin = mock(Plugin.class); + when(plugin.getName()).thenReturn("Example"); + BukkitCallerContextResolver resolver = resolverFor(plugin.getClass().getClassLoader()); + PluginIdentity identity = resolver.register(plugin); + + resolver.invalidateAll(); + assertFalse(resolver.isIdentityActive(identity)); assertFalse(resolver.isKnownPlugin("example")); } + @Test + void synchronizesInstalledPluginsBeforeTheyAreEnabled() { + Plugin dataRegistryDelegate = mock(Plugin.class); + Plugin serverFeaturesDelegate = mock(Plugin.class); + when(dataRegistryDelegate.getName()).thenReturn("DataRegistry"); + when(serverFeaturesDelegate.getName()).thenReturn("ServerFeatures"); + when(dataRegistryDelegate.isEnabled()).thenReturn(true); + when(serverFeaturesDelegate.isEnabled()).thenReturn(false); + ClassLoader dataRegistryLoader = new ClassLoader() { + }; + ClassLoader serverFeaturesLoader = new ClassLoader() { + }; + Plugin dataRegistry = pluginWithLoader(dataRegistryDelegate, dataRegistryLoader); + Plugin serverFeatures = pluginWithLoader(serverFeaturesDelegate, serverFeaturesLoader); + + BukkitCallerContextResolver resolver = resolverFor(dataRegistryLoader); + resolver.synchronizePlugins(List.of(dataRegistry, serverFeatures)); + + assertTrue(resolver.isKnownPlugin("dataregistry")); + assertTrue(resolver.isKnownPlugin("serverfeatures")); + assertNull(resolver.find(serverFeatures)); + assertThrows(SecurityException.class, () -> resolver.issueIdentity(serverFeatures)); + } + @Test void issuesAnIdentityDuringPluginEnableBeforeTheLifecycleEventIsFired() { Plugin plugin = mock(Plugin.class);