From 6c33e62dc4f196d610d96bf18df2e3996cc3c611 Mon Sep 17 00:00:00 2001 From: Mykhailo Alipa <6442572+strobil@users.noreply.github.com> Date: Thu, 5 Nov 2026 06:57:42 +0100 Subject: [PATCH] Fix bypass cache never expiring for active players hasBypass moved to Cache#asMap().computeIfAbsent in #2325. Guava implements that through compute, which records a write on every hit, so expireAfterWrite restarted each time. hasBypass runs on every block crossing, so an active player kept a stale result until they stayed idle for two seconds or changed worlds. A revoked bypass permission kept working in protected regions, and a newly granted one was ignored. Use getIfPresent and put, which only set the write time on a miss. Co-Authored-By: Claude Opus 5.5 --- .../session/AbstractSessionManager.java | 9 +- .../session/AbstractSessionManagerTest.java | 155 ++++++++++++++++++ 2 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 worldguard-core/src/test/java/com/sk89q/worldguard/session/AbstractSessionManagerTest.java diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java b/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java index 628f923fa..1f7f17a13 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java @@ -142,7 +142,14 @@ public boolean hasBypass(LocalPlayer player, World world) { } var bypassCacheKey = new BypassCacheKey(world.getName(), player.getUniqueId()); - return bypassCache.asMap().computeIfAbsent(bypassCacheKey, (_) -> BYPASS_PERMISSION_TEST.test(world, player)); + // Not asMap().computeIfAbsent: it renews the write time on every hit, so the + // result would never expire for a player who keeps being checked. + Boolean bypass = bypassCache.getIfPresent(bypassCacheKey); + if (bypass == null) { + bypass = BYPASS_PERMISSION_TEST.test(world, player); + bypassCache.put(bypassCacheKey, bypass); + } + return bypass; } @Override diff --git a/worldguard-core/src/test/java/com/sk89q/worldguard/session/AbstractSessionManagerTest.java b/worldguard-core/src/test/java/com/sk89q/worldguard/session/AbstractSessionManagerTest.java new file mode 100644 index 000000000..d332e1890 --- /dev/null +++ b/worldguard-core/src/test/java/com/sk89q/worldguard/session/AbstractSessionManagerTest.java @@ -0,0 +1,155 @@ +/* + * WorldGuard, a suite of tools for Minecraft + * Copyright (C) sk89q + * Copyright (C) WorldGuard team and contributors + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as published by the + * Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + */ + +package com.sk89q.worldguard.session; + +import com.sk89q.worldedit.util.Location; +import com.sk89q.worldedit.world.World; +import com.sk89q.worldguard.TestPlayer; +import com.sk89q.worldguard.WorldGuard; +import com.sk89q.worldguard.config.ConfigurationManager; +import com.sk89q.worldguard.config.WorldConfiguration; +import com.sk89q.worldguard.internal.platform.WorldGuardPlatform; +import com.sk89q.worldguard.protection.ApplicableRegionSet; +import com.sk89q.worldguard.protection.PermissiveRegionSet; +import com.sk89q.worldguard.protection.managers.RegionManager; +import com.sk89q.worldguard.protection.regions.RegionContainer; +import com.sk89q.worldguard.protection.regions.RegionQuery; +import org.junit.jupiter.api.Test; + +import java.io.File; +import java.lang.reflect.Proxy; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class AbstractSessionManagerTest { + + @Test + public void testCachedBypassExpiresWhileBeingChecked() throws Exception { + ConfigurationManager config = new TestConfigurationManager(); + WorldGuard.getInstance().setPlatform(stub(WorldGuardPlatform.class, Map.of( + "getGlobalStateManager", config, + "getRegionContainer", new TestRegionContainer()))); + + World world = stub(World.class, Map.of("getName", "world")); + TestBypassPlayer player = new TestBypassPlayer(world); + SessionManager manager = new TestSessionManager(); + manager.get(player); + assertTrue(manager.hasBypass(player, world)); + + player.bypass = false; + + // Moving players are checked on every block crossing, which is far more + // often than the two second expiry of the cached result. + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + boolean bypass = true; + while (bypass && System.nanoTime() < deadline) { + Thread.sleep(50); + bypass = manager.hasBypass(player, world); + } + assertFalse(bypass, "Repeated checks kept a revoked bypass cached"); + } + + private static T stub(Class type, Map answers) { + return type.cast(Proxy.newProxyInstance( + type.getClassLoader(), new Class[] {type}, + (proxy, method, args) -> { + Object answer = answers.get(method.getName()); + if (answer == null) { + throw new UnsupportedOperationException(method.toString()); + } + return answer; + })); + } + + private static final class TestSessionManager extends AbstractSessionManager { + @Override + public void resetAllStates() { + } + } + + private static final class TestBypassPlayer extends TestPlayer { + private final World world; + private volatile boolean bypass = true; + + private TestBypassPlayer(World world) { + super("test"); + this.world = world; + } + + @Override + public boolean hasPermission(String perm) { + return bypass; + } + + @Override + public World getWorld() { + return world; + } + + @Override + public Location getLocation() { + return new Location(world); + } + } + + private static final class TestConfigurationManager extends ConfigurationManager { + @Override + public File getDataFolder() { + return null; + } + + @Override + public void load() { + } + + @Override + public void unload() { + } + + @Override + public WorldConfiguration get(World world) { + return null; + } + + @Override + public void disableUuidMigration() { + } + } + + private static final class TestRegionContainer extends RegionContainer { + @Override + protected RegionManager load(World world) { + return null; + } + + @Override + public RegionQuery createQuery() { + return new RegionQuery(cache) { + @Override + public ApplicableRegionSet getApplicableRegions(Location location) { + return PermissiveRegionSet.getInstance(); + } + }; + } + } +}