From 5f7cff2da334643d6401a6b1eeeac09e7b5515e0 Mon Sep 17 00:00:00 2001 From: ThiagoROX <51332006+SrBedrock@users.noreply.github.com> Date: Mon, 28 Sep 2026 15:33:58 +0700 Subject: [PATCH 1/7] Harden REST endpoints and lifecycle (fixes #1) --- .github/workflows/build.yml | 18 ++ README.md | 67 +++-- build.gradle | 10 +- .../fredthedoggy/restpapi/RequestLimiter.java | 40 +++ .../me/fredthedoggy/restpapi/RestConfig.java | 45 +++ .../restpapi/RestPapiCommand.java | 33 +-- .../fredthedoggy/restpapi/RestPapiLoader.java | 96 +++++-- .../me/fredthedoggy/restpapi/Restpapi.java | 14 +- .../fredthedoggy/restpapi/SparkWrapper.java | 258 +++++++++++------- .../restpapi/RestSecurityTest.java | 80 ++++++ 10 files changed, 473 insertions(+), 188 deletions(-) create mode 100644 .github/workflows/build.yml create mode 100644 src/main/java/me/fredthedoggy/restpapi/RequestLimiter.java create mode 100644 src/main/java/me/fredthedoggy/restpapi/RestConfig.java create mode 100644 src/test/java/me/fredthedoggy/restpapi/RestSecurityTest.java diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 0000000..7e1c664 --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,18 @@ +name: Build + +on: + pull_request: + push: + branches: [master] + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: '17' + - uses: gradle/actions/setup-gradle@v4 + - run: bash gradlew test shadowJar --no-daemon diff --git a/README.md b/README.md index 86cb08e..51b0e39 100644 --- a/README.md +++ b/README.md @@ -1,34 +1,55 @@ -## What Is RestPlaceholderAPI? -RestPlaceholderAPI (RestPAPI) is a small lightweight plugin, that allows you to easily parse placeholders from an external application, like a Discord bot, or Forums. +# RestPlaceholderAPI -> :warning: **Warning:** RestPAPI Does nothing by itself, it just allows for external applications to parse placeholders via a simple Rest (http) API +RestPAPI exposes PlaceholderAPI values from each Bukkit/Paper backend over HTTP. Install PlaceholderAPI and this plugin on **each** backend you want to query. Velocity forwards Minecraft traffic, not these HTTP requests. -### Spigot Page / Downloads -https://www.spigotmc.org/resources/rest-placeholderapi.90266/ +## Configuration -### How Does it Work? -RestPAPI parses a specific placeholder, as a specific player when you make a Get request to your server, in the format: -http://backend.ip: port//, so an example would be http://example.com:8080/da8a8993-adfa-4d29-99b1-9d0f62fbb78d/player_name (returns json containing "Fredthedoggy") +On first start, `plugins/RestPAPI/config.yml` is created with two random UUID tokens. Keep them private. Example: -### Security: -RestPAPI has a List of "Tokens" in the config (It starts with 2 randomly generated Java UUIDs, but can be changed). You must send the header "Token" with the value of one of the tokens in the config, or you will get a 401 (unauthorized) message. +```yaml +port: 11001 +bind: 0.0.0.0 +tokens: + - "replace-with-a-long-random-secret" +timeout-ms: 3000 +max-concurrent: 16 +rate-limit: + requests: 60 + window-seconds: 60 +allowed-ips: [] +``` -### Plugin Support: -While it supports placeholderAPI, allowing it to support most PlaceholderAPI supported plugins, some placeholders will return an empty string, due to the fact that they cannot parse as an offline player, but will work when the player is online +Tokens must contain at least 16 characters, cannot be blank or padded with spaces, and must be unique. A missing or invalid token configuration prevents startup. To rotate tokens, temporarily include old and new tokens, run `/restpapi reload`, update clients, then remove the old token and reload again. Never print the tokens or put them in browser JavaScript. -Example Responses: +`bind` selects the interface **inside the container** (default `0.0.0.0`). `allowed-ips` is an exact match list of socket peer IPs; an empty list permits any peer with a valid token. Behind Nginx it will normally see the proxy address, not the original client. It deliberately ignores `X-Forwarded-For`. The rate limit is per socket peer and uses a fixed window; no more than 4096 distinct peers are tracked. The concurrency limit returns HTTP 503 instead of queuing unbounded lookups. Placeholder evaluation runs on the Minecraft main thread; clients receive HTTP 504 if it takes longer than `timeout-ms`. A lookup already running on the main thread cannot be interrupted. -```json -{"status":"401","message":"Unauthorized"} -``` -Without Authorization Header +The command `/restpapi reload` reads the file again, validates it before stopping the old listener, and attempts to restore the old listener if binding the new one fails. A failed rollback disables the plugin. A successful reload updates both the port and token set. + +## Requests -```json -{"status":"404","message":"Invalid URI"} +Both routes require the `Token` header and return JSON with string fields `status` and `message`: + +```bash +curl -H "Token: YOUR_SECRET" "http://127.0.0.1:11001/da8a8993-adfa-4d29-99b1-9d0f62fbb78d/player_name" +curl -H "Token: YOUR_SECRET" "http://127.0.0.1:11001/server/server_online" ``` -With The Wrong URL Scheme -```json -{"status": "200", "message":"Fredthedoggy"} +Use the placeholder name without percent signs. Unknown placeholders return 406, missing player data 400, bad UUID 400, unauthorized requests 401, blocked peers 403, unknown routes 404, excess requests 429, overload or shutdown 503, and timed out lookups 504. Empty resolved values are valid. Offline results depend on each PlaceholderAPI expansion's support for offline players. + +## Docker, Pterodactyl and external bots + +Allocate a dedicated HTTP port to **each backend** in Pterodactyl, separate from its Minecraft port. Set the plugin's `port` to that allocation's container port. For example, Minecraft may use `172.18.0.1:10001` while the same backend's REST service uses `172.18.0.1:11001`. The `172.18.0.1` gateway belongs to a Docker node and is not reachable from an external bot. The bot needs a routed private connection (for example VPN) or an HTTPS reverse proxy on the node. Do not publish the token over plain public HTTP. + +For a proxy on the same node, assign REST ports to a private/interface allocation reachable from the proxy. Confirm the effective Docker/host firewall rules restrict direct access. Example Nginx location within a TLS server: + +```nginx +location /survival/ { + proxy_pass http://172.18.0.1:11001/; +} ``` -Valid Placeholder (%player_name%) + +Then query `https://api.example.com/survival/UUID/player_name` with the `Token` header. Use separate allocations and tokens for other backends. The trailing slashes strip `/survival/` before forwarding. Restrict the proxy by the bot's IP and/or additional authentication and keep the per-backend token. Configure `allowed-ips` with the proxy's **socket peer** address if needed; when Docker publishes a port, verify which source IP reaches the container. + +## Build + +`bash gradlew test shadowJar`. The shaded plugin JAR is written under `build/libs`. diff --git a/build.gradle b/build.gradle index 10303fb..71c6731 100644 --- a/build.gradle +++ b/build.gradle @@ -24,6 +24,7 @@ tasks.jar { shadowJar { relocate 'org.bstats', 'me.fredthedoggy.restpapi.libs.bstats' relocate 'spark', 'me.fredthedoggy.restpapi.libs.spark' + relocate 'com.google.gson', 'me.fredthedoggy.restpapi.libs.gson' archiveBaseName.set('Restpapi') archiveVersion.set('1.0.5') archiveClassifier.set('') @@ -51,6 +52,13 @@ dependencies { compileOnly 'me.clip:placeholderapi:2.11.5' implementation 'org.bstats:bstats-bukkit:3.0.2' implementation 'com.sparkjava:spark-core:2.9.4' + implementation 'com.google.code.gson:gson:2.11.0' + testImplementation 'org.junit.jupiter:junit-jupiter:5.11.4' + testImplementation 'org.mockito:mockito-core:5.15.2' +} + +test { + useJUnitPlatform() } processResources { @@ -61,4 +69,4 @@ processResources { exclude 'plugin.yml' filter ReplaceTokens, tokens: [version: version] } -} \ No newline at end of file +} diff --git a/src/main/java/me/fredthedoggy/restpapi/RequestLimiter.java b/src/main/java/me/fredthedoggy/restpapi/RequestLimiter.java new file mode 100644 index 0000000..c44dc99 --- /dev/null +++ b/src/main/java/me/fredthedoggy/restpapi/RequestLimiter.java @@ -0,0 +1,40 @@ +package me.fredthedoggy.restpapi; + +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +final class RequestLimiter { + private final int limit; + private final long windowNanos; + private final Map windows = new HashMap<>(); + + RequestLimiter(int limit, int windowSeconds) { + this.limit = limit; + windowNanos = TimeUnit.SECONDS.toNanos(windowSeconds); + } + + synchronized boolean allow(String ip) { + long now = System.nanoTime(); + Window window = windows.get(ip); + if (window == null || now - window.start >= windowNanos) { + if (windows.size() >= 4096) { + Iterator iterator = windows.values().iterator(); + while (iterator.hasNext()) { + if (now - iterator.next().start >= windowNanos) iterator.remove(); + } + if (windows.size() >= 4096) return false; + } + windows.put(ip, new Window(now)); + return true; + } + return ++window.count <= limit; + } + + private static final class Window { + private final long start; + private int count = 1; + private Window(long start) { this.start = start; } + } +} diff --git a/src/main/java/me/fredthedoggy/restpapi/RestConfig.java b/src/main/java/me/fredthedoggy/restpapi/RestConfig.java new file mode 100644 index 0000000..fed3a46 --- /dev/null +++ b/src/main/java/me/fredthedoggy/restpapi/RestConfig.java @@ -0,0 +1,45 @@ +package me.fredthedoggy.restpapi; + +import org.bukkit.configuration.file.FileConfiguration; + +import java.util.HashSet; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +final class RestConfig { + private final int port, timeoutMillis, rateLimit, rateWindowSeconds, maxConcurrent; + private final String bind; + private final List tokens; + private final Set allowedIps; + + RestConfig(FileConfiguration yaml) { + port = yaml.getInt("port", 8080); + bind = yaml.getString("bind", "0.0.0.0"); + timeoutMillis = yaml.getInt("timeout-ms", 3000); + rateLimit = yaml.getInt("rate-limit.requests", 60); + rateWindowSeconds = yaml.getInt("rate-limit.window-seconds", 60); + maxConcurrent = yaml.getInt("max-concurrent", 16); + tokens = Collections.unmodifiableList(new ArrayList<>(yaml.getStringList("tokens"))); + allowedIps = Collections.unmodifiableSet(new HashSet<>(yaml.getStringList("allowed-ips"))); + if (port < 1 || port > 65535 || bind == null || bind.trim().isEmpty() + || timeoutMillis < 100 || timeoutMillis > 30000 + || rateLimit < 1 || rateLimit > 10000 || rateWindowSeconds < 1 + || rateWindowSeconds > 3600 || maxConcurrent < 1 || maxConcurrent > 24 + || tokens.isEmpty() || tokens.stream().anyMatch(t -> t == null || t.length() < 16 || !t.equals(t.trim())) + || new HashSet<>(tokens).size() != tokens.size() + || allowedIps.stream().anyMatch(ip -> ip == null || ip.trim().isEmpty() || !ip.equals(ip.trim()))) { + throw new IllegalArgumentException("Invalid REST configuration (port, bind, limits, tokens or allowed-ips)"); + } + } + + int port() { return port; } + String bind() { return bind; } + int timeoutMillis() { return timeoutMillis; } + int rateLimit() { return rateLimit; } + int rateWindowSeconds() { return rateWindowSeconds; } + int maxConcurrent() { return maxConcurrent; } + List tokens() { return tokens; } + Set allowedIps() { return allowedIps; } +} diff --git a/src/main/java/me/fredthedoggy/restpapi/RestPapiCommand.java b/src/main/java/me/fredthedoggy/restpapi/RestPapiCommand.java index a11b68c..a5f37ab 100644 --- a/src/main/java/me/fredthedoggy/restpapi/RestPapiCommand.java +++ b/src/main/java/me/fredthedoggy/restpapi/RestPapiCommand.java @@ -1,40 +1,23 @@ package me.fredthedoggy.restpapi; -import org.bukkit.Bukkit; import org.bukkit.ChatColor; import org.bukkit.command.Command; import org.bukkit.command.CommandExecutor; import org.bukkit.command.CommandSender; -import org.bukkit.entity.Player; -import java.util.logging.Level; +public final class RestPapiCommand implements CommandExecutor { + private final Restpapi plugin; -public class RestPapiCommand implements CommandExecutor { + RestPapiCommand(Restpapi plugin) { this.plugin = plugin; } - private final Restpapi restpapi; - - public RestPapiCommand(Restpapi restpapi) { - this.restpapi = restpapi; - } - - // This method is called, when somebody uses our command @Override public boolean onCommand(CommandSender sender, Command command, String label, String[] args) { - if (args.length >= 1 && args[0].equals("reload")) { - restpapi.webServer.destroy(); - this.restpapi.getLoader().loadWebServer(); - if (sender instanceof Player) { - Player player = (Player) sender; - player.sendMessage(ChatColor.GREEN + "RestPAPI Config is being reloaded."); - } - Bukkit.getLogger().log(Level.INFO, "[RestPAPI] Reloading Config"); + if (args.length == 1 && args[0].equalsIgnoreCase("reload")) { + boolean success = plugin.getLoader().reload(); + sender.sendMessage((success ? ChatColor.GREEN : ChatColor.RED) + + (success ? "RestPAPI configuration reloaded." : "RestPAPI reload failed; check console.")); } else { - if (sender instanceof Player) { - Player player = (Player) sender; - player.sendMessage(ChatColor.GREEN + "Run /restpapi reload to Reload RestPAPI"); - } else { - Bukkit.getLogger().log(Level.INFO, "[RestPAPI] Run /restpapi reload to Reload RestPAPI"); - } + sender.sendMessage(ChatColor.GREEN + "Usage: /" + label + " reload"); } return true; } diff --git a/src/main/java/me/fredthedoggy/restpapi/RestPapiLoader.java b/src/main/java/me/fredthedoggy/restpapi/RestPapiLoader.java index 378698a..33ca1cf 100644 --- a/src/main/java/me/fredthedoggy/restpapi/RestPapiLoader.java +++ b/src/main/java/me/fredthedoggy/restpapi/RestPapiLoader.java @@ -1,41 +1,87 @@ package me.fredthedoggy.restpapi; import org.bukkit.Bukkit; +import org.bukkit.configuration.file.FileConfiguration; +import java.io.File; import java.util.Arrays; -import java.util.List; import java.util.Objects; +import java.util.UUID; import java.util.logging.Level; -import static java.util.UUID.randomUUID; +public final class RestPapiLoader { + private final Restpapi plugin; + private SparkWrapper webServer; + private RestConfig runningConfig; -public class RestPapiLoader { - private final Restpapi parent; + RestPapiLoader(Restpapi plugin) { this.plugin = plugin; } - public RestPapiLoader(Restpapi parent) { - this.parent = parent; + void enable() { + if (!new File(plugin.getDataFolder(), "config.yml").exists()) { + FileConfiguration yaml = plugin.getConfig(); + yaml.set("port", 8080); + yaml.set("bind", "0.0.0.0"); + yaml.set("tokens", Arrays.asList(UUID.randomUUID().toString(), UUID.randomUUID().toString())); + yaml.set("timeout-ms", 3000); + yaml.set("max-concurrent", 16); + yaml.set("rate-limit.requests", 60); + yaml.set("rate-limit.window-seconds", 60); + yaml.set("allowed-ips", java.util.Collections.emptyList()); + plugin.saveConfig(); + } + Objects.requireNonNull(plugin.getCommand("restpapi")).setExecutor(new RestPapiCommand(plugin)); + Objects.requireNonNull(plugin.getCommand("rpapi")).setExecutor(new RestPapiCommand(plugin)); + try { + RestConfig config = new RestConfig(plugin.getConfig()); + SparkWrapper server = new SparkWrapper(plugin, config); + server.start(); + webServer = server; + runningConfig = config; + plugin.getLogger().info("REST listening on " + config.bind() + ":" + config.port()); + } catch (RuntimeException exception) { + plugin.getLogger().log(Level.SEVERE, "REST could not start; disabling plugin", exception); + Bukkit.getPluginManager().disablePlugin(plugin); + } } - public void enable() { - this.parent.webServer = new SparkWrapper(); - Objects.requireNonNull(this.parent.getCommand("restpapi")).setExecutor(new RestPapiCommand(this.parent)); - Objects.requireNonNull(this.parent.getCommand("rpapi")).setExecutor(new RestPapiCommand(this.parent)); - loadWebServer(); + synchronized boolean reload() { + plugin.reloadConfig(); + final RestConfig next; + try { + next = new RestConfig(plugin.getConfig()); + } catch (RuntimeException exception) { + plugin.getLogger().log(Level.WARNING, "Invalid REST config; previous service remains active", exception); + return false; + } + SparkWrapper previous = webServer; + if (previous != null) previous.stop(); + webServer = null; + SparkWrapper replacement = new SparkWrapper(plugin, next); + try { + replacement.start(); + webServer = replacement; + runningConfig = next; + plugin.getLogger().info("REST listening on " + next.bind() + ":" + next.port()); + return true; + } catch (RuntimeException exception) { + plugin.getLogger().log(Level.SEVERE, "REST reload failed; restoring previous service", exception); + if (runningConfig != null) { + try { + SparkWrapper restored = new SparkWrapper(plugin, runningConfig); + restored.start(); + webServer = restored; + } catch (RuntimeException rollbackException) { + plugin.getLogger().log(Level.SEVERE, "REST rollback failed; disabling plugin", rollbackException); + Bukkit.getPluginManager().disablePlugin(plugin); + } + } + return false; + } } - public void disable() { - this.parent.webServer.destroy(); - Bukkit.getLogger().log(Level.SEVERE,"[RestPAPI] Shutting down plugin."); - } - - public void loadWebServer() { - this.parent.config.addDefault("port", 8080); - List defaultTokens = Arrays.asList(randomUUID().toString(), randomUUID().toString()); - this.parent.config.addDefault("tokens", defaultTokens); - this.parent.config.options().copyDefaults(true); - this.parent.saveConfig(); - this.parent.webServer.create(this.parent.config.getInt("port"), this.parent.config.getStringList("tokens")); - Bukkit.getLogger().log(Level.INFO,"[RestPAPI] Enabled On Port " + this.parent.config.getInt("port")); + synchronized void disable() { + SparkWrapper server = webServer; + webServer = null; + if (server != null) server.stop(); } } - diff --git a/src/main/java/me/fredthedoggy/restpapi/Restpapi.java b/src/main/java/me/fredthedoggy/restpapi/Restpapi.java index 6752dc2..dceca6f 100644 --- a/src/main/java/me/fredthedoggy/restpapi/Restpapi.java +++ b/src/main/java/me/fredthedoggy/restpapi/Restpapi.java @@ -1,26 +1,20 @@ package me.fredthedoggy.restpapi; -import org.bukkit.configuration.file.FileConfiguration; import org.bukkit.plugin.java.JavaPlugin; public final class Restpapi extends JavaPlugin { - private RestPapiLoader loader; - FileConfiguration config = getConfig(); - SparkWrapper webServer; @Override public void onEnable() { - this.loader = new RestPapiLoader(this); - this.loader.enable(); + loader = new RestPapiLoader(this); + loader.enable(); } @Override public void onDisable() { - this.loader.disable(); + if (loader != null) loader.disable(); } - public RestPapiLoader getLoader() { - return loader; - } + public RestPapiLoader getLoader() { return loader; } } diff --git a/src/main/java/me/fredthedoggy/restpapi/SparkWrapper.java b/src/main/java/me/fredthedoggy/restpapi/SparkWrapper.java index 2b7dfb3..60d675c 100644 --- a/src/main/java/me/fredthedoggy/restpapi/SparkWrapper.java +++ b/src/main/java/me/fredthedoggy/restpapi/SparkWrapper.java @@ -1,130 +1,180 @@ package me.fredthedoggy.restpapi; +import com.google.gson.Gson; import me.clip.placeholderapi.PlaceholderAPI; import org.bukkit.Bukkit; +import org.bukkit.OfflinePlayer; +import org.bukkit.scheduler.BukkitTask; +import spark.Request; +import spark.Response; import spark.Service; -import java.util.List; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import static spark.Service.ignite; -class SparkWrapper { - Service http; - - void create(int port, List tokens) { - http = ignite().port(port); - - // New endpoint for server-wide placeholders - http.get("/server/:placeholder", (request, response) -> { - response.type("application/json"); - - String placeholderResult = PlaceholderAPI.setPlaceholders(null, "%" + request.params(":placeholder") + "%"); +final class SparkWrapper { + private static final Gson JSON = new Gson(); + private final Restpapi plugin; + private final RestConfig config; + private final Semaphore concurrent; + private final RequestLimiter limiter; + private final AtomicBoolean accepting = new AtomicBoolean(true); + private Service http; + + SparkWrapper(Restpapi plugin, RestConfig config) { + this.plugin = plugin; + this.config = config; + concurrent = new Semaphore(config.maxConcurrent()); + limiter = new RequestLimiter(config.rateLimit(), config.rateWindowSeconds()); + } - if (placeholderResult.equals("%" + request.params(":placeholder") + "%")) { - response.status(406); - return "{\"status\":\"406\",\"message\":\"Invalid Placeholder\"}"; - } else { - response.status(200); - return "{\"status\":\"200\",\"message\":\"" + placeholderResult + "\"}"; + void start() { + Service service = ignite(); + http = service; + AtomicReference startFailure = new AtomicReference<>(); + boolean routesRegistered = false; + try { + service.initExceptionHandler(startFailure::set); + service.untrustForwardHeaders(); + service.ipAddress(config.bind()); + service.port(config.port()); + service.threadPool(32, 4, 30000); + service.get("/server/:placeholder", (request, response) -> handle(request, response, false)); + routesRegistered = true; + service.get("/:uuid/:placeholder", (request, response) -> handle(request, response, true)); + service.notFound((request, response) -> json(response, 404, "Invalid URI")); + service.internalServerError((request, response) -> json(response, 500, "Internal Server Error")); + service.awaitInitialization(); + if (startFailure.get() != null) { + throw new IllegalStateException("HTTP listener could not bind", startFailure.get()); } - }); - - http.get("/:uuid/:placeholder", (request, response) -> { - - Bukkit.getLogger().log(Level.SEVERE, " !!! Using insecure version TOKEN DISABLED! !!!"); - Bukkit.getLogger().log(Level.SEVERE, "Token received: " + request.headers("token")); - /* - // todo: add back later - // Avoid someone spamming PlaceholderAPI requests. + } catch (RuntimeException exception) { + if (routesRegistered) stop(); + else { + accepting.set(false); + http = null; + } + throw exception; + } + } - if (request.headers("token") == null) { - response.type("application/json"); - response.status(401); - return "{\"status\":\"401\",\"message\":\"Unauthorized\"}"; - } else if (tokens.stream().noneMatch(request.headers("token")::contains)) { - response.type("application/json"); - response.status(401); - return "{\"status\":\"401\",\"message\":\"Unauthorized\"}"; - } else { - */ - response.type("application/json"); - UUID specifiedUUID; - try { - specifiedUUID = UUID.fromString(request.params(":uuid")); + private String handle(Request request, Response response, boolean playerRoute) { + response.type("application/json"); + if (!accepting.get() || !plugin.isEnabled()) return json(response, 503, "Service Unavailable"); + if (!authorized(request.headers("Token"))) return json(response, 401, "Unauthorized"); + // Use the socket peer; forwarded headers can be forged unless a trusted proxy strips them. + if (!config.allowedIps().isEmpty() && !config.allowedIps().contains(request.ip())) { + return json(response, 403, "Forbidden"); + } + if (!limiter.allow(request.ip())) return json(response, 429, "Too Many Requests"); + if (!concurrent.tryAcquire()) return json(response, 503, "Service Busy"); + try { + String name = request.params(":placeholder"); + if (name == null || name.isEmpty() || name.length() > 256 || name.indexOf('%') >= 0) { + return json(response, 400, "Invalid Placeholder"); } - catch(Exception e) { - response.type("application/json"); - response.status(400); - return "{\"status\":\"400\",\"message\":\"Invalid UUID\"}"; + UUID uuid = null; + if (playerRoute) { + try { + uuid = UUID.fromString(request.params(":uuid")); + } catch (IllegalArgumentException exception) { + return json(response, 400, "Invalid UUID"); + } } - if (Bukkit.getOfflinePlayer(specifiedUUID).hasPlayedBefore()) { - - response.type("application/json"); - response.status(200); - - String placeholderResult = PlaceholderAPI.setPlaceholders( - Bukkit.getOfflinePlayer(UUID.fromString(request.params(":uuid"))), - "%" + request.params(":placeholder") + "%" - ); - - String placeholder = "{\"status\":\"200\",\"message\":\"" + placeholderResult + "\"}"; - - if (placeholderResult.equals("%" + request.params(":placeholder") + "%")) { - - response.type("application/json"); - response.status(406); - return "{\"status\":\"406\",\"message\":\"Invalid Placeholder\"}"; - - } else { - - return placeholder; - + final UUID playerId = uuid; + final String expression = "%" + name + "%"; + CompletableFuture future = new CompletableFuture<>(); + BukkitTask task = Bukkit.getScheduler().runTask(plugin, () -> { + if (!accepting.get() || future.isDone()) return; + try { + OfflinePlayer player = playerId == null ? null : Bukkit.getOfflinePlayer(playerId); + if (player != null && !player.hasPlayedBefore() && !player.isOnline()) { + future.complete(new Lookup(400, "Player Has Not Played Before")); + return; + } + String result = PlaceholderAPI.setPlaceholders(player, expression); + future.complete(resolveResult(expression, result)); + } catch (Exception exception) { + plugin.getLogger().log(Level.WARNING, "Placeholder lookup failed", exception); + future.complete(new Lookup(500, "Internal Server Error")); } - } else { - - response.type("application/json"); - response.status(400); - return "{\"status\":\"400\",\"message\":\"Player Has Not Played Before\"}"; - + }); + try { + Lookup result = future.get(config.timeoutMillis(), TimeUnit.MILLISECONDS); + return json(response, Integer.parseInt(result.status), result.message); + } catch (TimeoutException exception) { + future.cancel(false); + task.cancel(); + return json(response, 504, "Lookup Timed Out"); + } catch (InterruptedException exception) { + Thread.currentThread().interrupt(); + task.cancel(); + return json(response, 503, "Service Unavailable"); + } catch (ExecutionException exception) { + plugin.getLogger().log(Level.WARNING, "Placeholder lookup failed", exception); + return json(response, 500, "Internal Server Error"); } - //} - - }); - http.get("/*", (request, response) -> { - - response.status(404); - response.type("application/json"); - return "{\"status\":\"404\",\"message\":\"Invalid URI\"}"; - - }); - http.get("/*/*/*", (request, response) -> { - - response.status(404); - response.type("application/json"); - return "{\"status\":\"404\",\"message\":\"Invalid URI\"}"; - - }); - http.get("/*/*/*/*", (request, response) -> { + } catch (RuntimeException exception) { + plugin.getLogger().log(Level.WARNING, "Could not schedule placeholder lookup", exception); + return json(response, 503, "Service Unavailable"); + } finally { + concurrent.release(); + } + } - response.status(404); - response.type("application/json"); - return "{\"status\":\"404\",\"message\":\"Invalid URI\"}"; + boolean authorized(String provided) { + if (provided == null || provided.isEmpty()) return false; + byte[] candidate = provided.getBytes(StandardCharsets.UTF_8); + boolean matched = false; + for (String token : config.tokens()) { + matched |= MessageDigest.isEqual(candidate, token.getBytes(StandardCharsets.UTF_8)); + } + return matched; + } - }); - http.notFound((request, response) -> { + static int placeholderStatus(String expression, String result) { + return expression.equals(result) ? 406 : 200; + } - response.status(404); - response.type("application/json"); - return "{\"status\":\"404\",\"message\":\"Invalid URI\"}"; + private static Lookup resolveResult(String expression, String result) { + int status = placeholderStatus(expression, result); + return new Lookup(status, status == 406 ? "Invalid Placeholder" : result); + } - }); + static String json(Response response, int status, String message) { + response.status(status); + response.type("application/json"); + return JSON.toJson(new Lookup(status, message)); } - void destroy() { - http.stop(); - Bukkit.getLogger().log(Level.WARNING, "[RestPAPI] Disabled Webserver"); + void stop() { + accepting.set(false); + Service service = http; + http = null; + if (service != null) { + service.stop(); + service.awaitStop(); + } } -} \ No newline at end of file + private static final class Lookup { + private final String status; + private final String message; + + Lookup(int status, String message) { + this.status = Integer.toString(status); + this.message = message; + } + } +} diff --git a/src/test/java/me/fredthedoggy/restpapi/RestSecurityTest.java b/src/test/java/me/fredthedoggy/restpapi/RestSecurityTest.java new file mode 100644 index 0000000..17e219f --- /dev/null +++ b/src/test/java/me/fredthedoggy/restpapi/RestSecurityTest.java @@ -0,0 +1,80 @@ +package me.fredthedoggy.restpapi; + +import com.google.gson.JsonParser; +import org.bukkit.configuration.file.YamlConfiguration; +import org.junit.jupiter.api.Test; +import spark.Response; + +import java.util.Arrays; +import java.util.Collections; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +class RestSecurityTest { + private YamlConfiguration config() { + YamlConfiguration yaml = new YamlConfiguration(); + yaml.set("tokens", Collections.singletonList("12345678-1234-1234-1234-123456789abc")); + return yaml; + } + + @Test + void tokensMustMatchExactly() { + SparkWrapper server = new SparkWrapper(mock(Restpapi.class), new RestConfig(config())); + assertTrue(server.authorized("12345678-1234-1234-1234-123456789abc")); + assertFalse(server.authorized("prefix12345678-1234-1234-1234-123456789abc")); + assertFalse(server.authorized("12345678-1234-1234-1234-123456789abc suffix")); + assertFalse(server.authorized(null)); + assertFalse(server.authorized("")); + } + + @Test + void invalidOrMissingTokensFailClosed() { + YamlConfiguration yaml = config(); + yaml.set("tokens", Arrays.asList("valid-valid-valid-valid", "")); + assertThrows(IllegalArgumentException.class, () -> new RestConfig(yaml)); + yaml.set("tokens", Collections.emptyList()); + assertThrows(IllegalArgumentException.class, () -> new RestConfig(yaml)); + yaml.set("tokens", Collections.singletonList("short")); + assertThrows(IllegalArgumentException.class, () -> new RestConfig(yaml)); + } + + @Test + void serializesPlaceholderTextAndKeepsLegacyStatus() { + Response response = mock(Response.class); + String message = "Name: \"Thiago\" \\ test\nline"; + String payload = SparkWrapper.json(response, 200, message); + assertEquals(message, JsonParser.parseString(payload).getAsJsonObject().get("message").getAsString()); + assertEquals("200", JsonParser.parseString(payload).getAsJsonObject().get("status").getAsString()); + verify(response).type("application/json"); + verify(response).status(200); + assertEquals("", JsonParser.parseString(SparkWrapper.json(response, 200, "")) + .getAsJsonObject().get("message").getAsString()); + } + + @Test + void limitsRequestsPerPeer() { + RequestLimiter limiter = new RequestLimiter(2, 60); + assertTrue(limiter.allow("192.0.2.1")); + assertTrue(limiter.allow("192.0.2.1")); + assertFalse(limiter.allow("192.0.2.1")); + assertTrue(limiter.allow("192.0.2.2")); + } + + @Test + void distinguishesUnknownPlaceholderFromEmptyValue() { + assertEquals(406, SparkWrapper.placeholderStatus("%unknown%", "%unknown%")); + assertEquals(200, SparkWrapper.placeholderStatus("%known%", "")); + assertEquals(200, SparkWrapper.placeholderStatus("%known%", "value")); + } + + @Test + void rejectsOutOfRangeNetworkSettings() { + YamlConfiguration yaml = config(); + yaml.set("port", 65536); + assertThrows(IllegalArgumentException.class, () -> new RestConfig(yaml)); + yaml.set("port", 8080); + yaml.set("max-concurrent", 1000); + assertThrows(IllegalArgumentException.class, () -> new RestConfig(yaml)); + } +} From cd12ec8830d72b21a435dbc2f0a86a529c9461fe Mon Sep 17 00:00:00 2001 From: ThiagoROX <51332006+SrBedrock@users.noreply.github.com> Date: Mon, 28 Sep 2026 15:34:58 +0700 Subject: [PATCH 2/7] Use runner JDK for Gradle builds --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 9d2780e..e1a61f9 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1 +1 @@ -org.gradle.java.home=C:/Program Files/Java/jdk-17 +# Use the JDK selected by JAVA_HOME (or the CI setup-java action). From 952ae176c01f7c1ffc7e7c0db9620a8d643adef9 Mon Sep 17 00:00:00 2001 From: ThiagoROX <51332006+SrBedrock@users.noreply.github.com> Date: Mon, 28 Sep 2026 15:36:53 +0700 Subject: [PATCH 3/7] Provide Bukkit and PAPI dependencies to regression tests --- build.gradle | 2 ++ 1 file changed, 2 insertions(+) diff --git a/build.gradle b/build.gradle index 71c6731..8300b59 100644 --- a/build.gradle +++ b/build.gradle @@ -55,6 +55,8 @@ dependencies { implementation 'com.google.code.gson:gson:2.11.0' testImplementation 'org.junit.jupiter:junit-jupiter:5.11.4' testImplementation 'org.mockito:mockito-core:5.15.2' + testImplementation 'org.spigotmc:spigot-api:1.20.4-R0.1-SNAPSHOT' + testImplementation 'me.clip:placeholderapi:2.11.5' } test { From 0725ebb7f24072baeb9861d7d22d15c127a1a175 Mon Sep 17 00:00:00 2001 From: ThiagoROX <51332006+SrBedrock@users.noreply.github.com> Date: Mon, 28 Sep 2026 15:41:50 +0700 Subject: [PATCH 4/7] Build with Java 25, Paper 1.21.11 and Gradle 9.8.0 --- .github/workflows/build.yml | 2 +- README.md | 2 +- build.gradle | 24 ++++++++++++------------ gradle/wrapper/gradle-wrapper.properties | 2 +- src/main/resources/plugin.yml | 4 ++-- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7e1c664..3166c8e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -13,6 +13,6 @@ jobs: - uses: actions/setup-java@v4 with: distribution: temurin - java-version: '17' + java-version: '25' - uses: gradle/actions/setup-gradle@v4 - run: bash gradlew test shadowJar --no-daemon diff --git a/README.md b/README.md index 51b0e39..bcbb836 100644 --- a/README.md +++ b/README.md @@ -52,4 +52,4 @@ Then query `https://api.example.com/survival/UUID/player_name` with the `Token` ## Build -`bash gradlew test shadowJar`. The shaded plugin JAR is written under `build/libs`. +Use JDK 25 and `bash gradlew test shadowJar` (Gradle 9.8.0). The plugin targets Paper API 1.21.11; the shaded JAR is written under `build/libs`. diff --git a/build.gradle b/build.gradle index 8300b59..4be1c1a 100644 --- a/build.gradle +++ b/build.gradle @@ -1,21 +1,24 @@ import org.apache.tools.ant.filters.ReplaceTokens plugins { - id 'com.github.johnrengelman.shadow' version '7.1.2' // Reverted shadow plugin version + id 'com.gradleup.shadow' version '9.6.1' id 'java' } -sourceCompatibility = targetCompatibility = JavaVersion.VERSION_1_8 -compileJava.options.encoding 'UTF-8' - group 'me.fredthedoggy' version '1.0.5' java { + toolchain.languageVersion = JavaLanguageVersion.of(25) withJavadocJar() withSourcesJar() } +tasks.withType(JavaCompile).configureEach { + options.encoding = 'UTF-8' + options.release = 25 +} + // Disable the default 'jar' task to ensure only the shadowJar is produced tasks.jar { enabled = false @@ -33,8 +36,6 @@ shadowJar { // Explicitly include the main source set output for relocation from sourceSets.main.output - // Minimize the JAR to remove unused classes and potentially fix relocation issues - minimize() } // Make sure 'build' depends on 'shadowJar' @@ -42,21 +43,20 @@ build.dependsOn shadowJar repositories { mavenCentral() - maven { url = 'https://oss.sonatype.org/content/groups/public/' } - maven { url = 'https://hub.spigotmc.org/nexus/content/repositories/snapshots/' } + maven { url = 'https://repo.papermc.io/repository/maven-public/' } maven { url = 'https://repo.extendedclip.com/content/repositories/placeholderapi/' } } dependencies { - compileOnly 'org.spigotmc:spigot-api:1.20.4-R0.1-SNAPSHOT' - compileOnly 'me.clip:placeholderapi:2.11.5' + compileOnly 'io.papermc.paper:paper-api:1.21.11-R0.1-SNAPSHOT' + compileOnly 'me.clip:placeholderapi:2.12.3' implementation 'org.bstats:bstats-bukkit:3.0.2' implementation 'com.sparkjava:spark-core:2.9.4' implementation 'com.google.code.gson:gson:2.11.0' testImplementation 'org.junit.jupiter:junit-jupiter:5.11.4' testImplementation 'org.mockito:mockito-core:5.15.2' - testImplementation 'org.spigotmc:spigot-api:1.20.4-R0.1-SNAPSHOT' - testImplementation 'me.clip:placeholderapi:2.11.5' + testImplementation 'io.papermc.paper:paper-api:1.21.11-R0.1-SNAPSHOT' + testImplementation 'me.clip:placeholderapi:2.12.3' } test { diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index a595206..79f3414 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,5 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-8.5-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-9.8.0-bin.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists diff --git a/src/main/resources/plugin.yml b/src/main/resources/plugin.yml index ecbba46..910f33a 100644 --- a/src/main/resources/plugin.yml +++ b/src/main/resources/plugin.yml @@ -1,7 +1,7 @@ name: RestPAPI version: 9.9 main: me.fredthedoggy.restpapi.Restpapi -api-version: 1.13 +api-version: '1.21.11' prefix: RestPAPI load: STARTUP authors: [ Fredthedoggy ] @@ -14,4 +14,4 @@ commands: rpapi: description: Rest Papi Main Command usage: /rpapi - permission: restpapi.admin \ No newline at end of file + permission: restpapi.admin From 9fab6f60906a9d46b3c677b6ee63d772f23c4737 Mon Sep 17 00:00:00 2001 From: ThiagoROX <51332006+SrBedrock@users.noreply.github.com> Date: Mon, 28 Sep 2026 15:44:23 +0700 Subject: [PATCH 5/7] Add JUnit Platform launcher for Gradle 9 tests --- build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/build.gradle b/build.gradle index 4be1c1a..f12fd51 100644 --- a/build.gradle +++ b/build.gradle @@ -54,6 +54,7 @@ dependencies { implementation 'com.sparkjava:spark-core:2.9.4' implementation 'com.google.code.gson:gson:2.11.0' testImplementation 'org.junit.jupiter:junit-jupiter:5.11.4' + testRuntimeOnly 'org.junit.platform:junit-platform-launcher' testImplementation 'org.mockito:mockito-core:5.15.2' testImplementation 'io.papermc.paper:paper-api:1.21.11-R0.1-SNAPSHOT' testImplementation 'me.clip:placeholderapi:2.12.3' From 22ae2c24b2fe06b5a4b1d9fb0126debd5457898c Mon Sep 17 00:00:00 2001 From: ThiagoROX <51332006+SrBedrock@users.noreply.github.com> Date: Mon, 28 Sep 2026 15:46:11 +0700 Subject: [PATCH 6/7] Update Mockito for Java 25 tests --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index f12fd51..a686ab5 100644 --- a/build.gradle +++ b/build.gradle @@ -55,7 +55,7 @@ dependencies { implementation 'com.google.code.gson:gson:2.11.0' testImplementation 'org.junit.jupiter:junit-jupiter:5.11.4' testRuntimeOnly 'org.junit.platform:junit-platform-launcher' - testImplementation 'org.mockito:mockito-core:5.15.2' + testImplementation 'org.mockito:mockito-core:5.24.0' testImplementation 'io.papermc.paper:paper-api:1.21.11-R0.1-SNAPSHOT' testImplementation 'me.clip:placeholderapi:2.12.3' } From 4517a7c9a6ba4388a52c5711de7d707402462d21 Mon Sep 17 00:00:00 2001 From: ThiagoROX <51332006+SrBedrock@users.noreply.github.com> Date: Mon, 28 Sep 2026 16:07:40 +0700 Subject: [PATCH 7/7] Address PR review: async reload and nonblocking HTTP shutdown --- .github/workflows/build.yml | 2 +- README.md | 2 +- .../restpapi/RestPapiCommand.java | 24 ++++++- .../fredthedoggy/restpapi/RestPapiLoader.java | 62 ++++++++++++++----- .../fredthedoggy/restpapi/SparkWrapper.java | 33 +++++++--- .../restpapi/RestSecurityTest.java | 15 +++++ 6 files changed, 111 insertions(+), 27 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3166c8e..8ec7807 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -3,7 +3,7 @@ name: Build on: pull_request: push: - branches: [master] + branches: [master, main] jobs: build: diff --git a/README.md b/README.md index bcbb836..42128a6 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ Tokens must contain at least 16 characters, cannot be blank or padded with space `bind` selects the interface **inside the container** (default `0.0.0.0`). `allowed-ips` is an exact match list of socket peer IPs; an empty list permits any peer with a valid token. Behind Nginx it will normally see the proxy address, not the original client. It deliberately ignores `X-Forwarded-For`. The rate limit is per socket peer and uses a fixed window; no more than 4096 distinct peers are tracked. The concurrency limit returns HTTP 503 instead of queuing unbounded lookups. Placeholder evaluation runs on the Minecraft main thread; clients receive HTTP 504 if it takes longer than `timeout-ms`. A lookup already running on the main thread cannot be interrupted. -The command `/restpapi reload` reads the file again, validates it before stopping the old listener, and attempts to restore the old listener if binding the new one fails. A failed rollback disables the plugin. A successful reload updates both the port and token set. +The command `/restpapi reload` reads the file again, validates it before stopping the old listener, and attempts to restore the old listener if binding the new one fails. A failed rollback disables the plugin. Listener shutdown and startup take place off the Minecraft main thread; the command reports the result after they finish. In-flight lookups receive 503 during shutdown. A successful reload updates both the port and token set. ## Requests diff --git a/src/main/java/me/fredthedoggy/restpapi/RestPapiCommand.java b/src/main/java/me/fredthedoggy/restpapi/RestPapiCommand.java index a5f37ab..294b5ef 100644 --- a/src/main/java/me/fredthedoggy/restpapi/RestPapiCommand.java +++ b/src/main/java/me/fredthedoggy/restpapi/RestPapiCommand.java @@ -1,9 +1,11 @@ package me.fredthedoggy.restpapi; import org.bukkit.ChatColor; +import org.bukkit.Bukkit; import org.bukkit.command.Command; import org.bukkit.command.CommandExecutor; import org.bukkit.command.CommandSender; +import java.util.concurrent.CompletableFuture; public final class RestPapiCommand implements CommandExecutor { private final Restpapi plugin; @@ -13,12 +15,28 @@ public final class RestPapiCommand implements CommandExecutor { @Override public boolean onCommand(CommandSender sender, Command command, String label, String[] args) { if (args.length == 1 && args[0].equalsIgnoreCase("reload")) { - boolean success = plugin.getLoader().reload(); - sender.sendMessage((success ? ChatColor.GREEN : ChatColor.RED) - + (success ? "RestPAPI configuration reloaded." : "RestPAPI reload failed; check console.")); + CompletableFuture result = plugin.getLoader().reload(); + if (result.isDone()) { + sendResult(sender, result.getNow(false)); + } else { + sender.sendMessage(ChatColor.YELLOW + "RestPAPI reload in progress."); + result.whenComplete((success, error) -> { + if (!plugin.isEnabled()) return; + try { + Bukkit.getScheduler().runTask(plugin, () -> sendResult(sender, error == null && success)); + } catch (RuntimeException exception) { + plugin.getLogger().warning("Could not deliver REST reload result: " + exception.getMessage()); + } + }); + } } else { sender.sendMessage(ChatColor.GREEN + "Usage: /" + label + " reload"); } return true; } + + private void sendResult(CommandSender sender, boolean success) { + sender.sendMessage((success ? ChatColor.GREEN : ChatColor.RED) + + (success ? "RestPAPI configuration reloaded." : "RestPAPI reload failed; check console.")); + } } diff --git a/src/main/java/me/fredthedoggy/restpapi/RestPapiLoader.java b/src/main/java/me/fredthedoggy/restpapi/RestPapiLoader.java index 33ca1cf..e19a8fd 100644 --- a/src/main/java/me/fredthedoggy/restpapi/RestPapiLoader.java +++ b/src/main/java/me/fredthedoggy/restpapi/RestPapiLoader.java @@ -7,12 +7,15 @@ import java.util.Arrays; import java.util.Objects; import java.util.UUID; +import java.util.concurrent.CompletableFuture; import java.util.logging.Level; public final class RestPapiLoader { private final Restpapi plugin; private SparkWrapper webServer; private RestConfig runningConfig; + private boolean reloading; + private boolean disabled; RestPapiLoader(Restpapi plugin) { this.plugin = plugin; } @@ -44,44 +47,75 @@ void enable() { } } - synchronized boolean reload() { + synchronized CompletableFuture reload() { + if (disabled || reloading) return CompletableFuture.completedFuture(false); plugin.reloadConfig(); final RestConfig next; try { next = new RestConfig(plugin.getConfig()); } catch (RuntimeException exception) { plugin.getLogger().log(Level.WARNING, "Invalid REST config; previous service remains active", exception); - return false; + return CompletableFuture.completedFuture(false); } + reloading = true; SparkWrapper previous = webServer; - if (previous != null) previous.stop(); - webServer = null; - SparkWrapper replacement = new SparkWrapper(plugin, next); + RestConfig previousConfig = runningConfig; + return CompletableFuture.supplyAsync(() -> replace(previous, previousConfig, next)); + } + + private boolean replace(SparkWrapper previous, RestConfig previousConfig, RestConfig next) { try { + if (previous != null) { + previous.stop(); + previous.awaitStop(); // Never wait for Spark workers on the Bukkit tick thread. + } + synchronized (this) { + if (disabled) return false; + } + SparkWrapper replacement = new SparkWrapper(plugin, next); replacement.start(); - webServer = replacement; - runningConfig = next; + synchronized (this) { + if (disabled) { + replacement.stop(); + return false; + } + webServer = replacement; + runningConfig = next; + } plugin.getLogger().info("REST listening on " + next.bind() + ":" + next.port()); return true; } catch (RuntimeException exception) { plugin.getLogger().log(Level.SEVERE, "REST reload failed; restoring previous service", exception); - if (runningConfig != null) { + if (previousConfig != null && !isDisabled()) { try { - SparkWrapper restored = new SparkWrapper(plugin, runningConfig); + SparkWrapper restored = new SparkWrapper(plugin, previousConfig); restored.start(); - webServer = restored; + synchronized (this) { + if (disabled) restored.stop(); + else webServer = restored; + } } catch (RuntimeException rollbackException) { plugin.getLogger().log(Level.SEVERE, "REST rollback failed; disabling plugin", rollbackException); - Bukkit.getPluginManager().disablePlugin(plugin); + Bukkit.getScheduler().runTask(plugin, () -> Bukkit.getPluginManager().disablePlugin(plugin)); } } return false; + } finally { + synchronized (this) { + reloading = false; + } } } - synchronized void disable() { - SparkWrapper server = webServer; - webServer = null; + private synchronized boolean isDisabled() { return disabled; } + + void disable() { + SparkWrapper server; + synchronized (this) { + disabled = true; + server = webServer; + webServer = null; + } if (server != null) server.stop(); } } diff --git a/src/main/java/me/fredthedoggy/restpapi/SparkWrapper.java b/src/main/java/me/fredthedoggy/restpapi/SparkWrapper.java index 60d675c..aec252a 100644 --- a/src/main/java/me/fredthedoggy/restpapi/SparkWrapper.java +++ b/src/main/java/me/fredthedoggy/restpapi/SparkWrapper.java @@ -12,7 +12,9 @@ import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.util.UUID; +import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; @@ -30,6 +32,8 @@ final class SparkWrapper { private final Semaphore concurrent; private final RequestLimiter limiter; private final AtomicBoolean accepting = new AtomicBoolean(true); + private final AtomicBoolean stopped = new AtomicBoolean(false); + private final Set> pending = ConcurrentHashMap.newKeySet(); private Service http; SparkWrapper(Restpapi plugin, RestConfig config) { @@ -60,7 +64,10 @@ void start() { throw new IllegalStateException("HTTP listener could not bind", startFailure.get()); } } catch (RuntimeException exception) { - if (routesRegistered) stop(); + if (routesRegistered) { + stop(); + awaitStop(); + } else { accepting.set(false); http = null; @@ -79,6 +86,7 @@ private String handle(Request request, Response response, boolean playerRoute) { } if (!limiter.allow(request.ip())) return json(response, 429, "Too Many Requests"); if (!concurrent.tryAcquire()) return json(response, 503, "Service Busy"); + CompletableFuture future = new CompletableFuture<>(); try { String name = request.params(":placeholder"); if (name == null || name.isEmpty() || name.length() > 256 || name.indexOf('%') >= 0) { @@ -94,7 +102,7 @@ private String handle(Request request, Response response, boolean playerRoute) { } final UUID playerId = uuid; final String expression = "%" + name + "%"; - CompletableFuture future = new CompletableFuture<>(); + track(future); BukkitTask task = Bukkit.getScheduler().runTask(plugin, () -> { if (!accepting.get() || future.isDone()) return; try { @@ -129,6 +137,7 @@ private String handle(Request request, Response response, boolean playerRoute) { plugin.getLogger().log(Level.WARNING, "Could not schedule placeholder lookup", exception); return json(response, 503, "Service Unavailable"); } finally { + pending.remove(future); concurrent.release(); } } @@ -152,6 +161,11 @@ private static Lookup resolveResult(String expression, String result) { return new Lookup(status, status == 406 ? "Invalid Placeholder" : result); } + void track(CompletableFuture future) { + pending.add(future); + if (!accepting.get()) future.complete(new Lookup(503, "Service Unavailable")); + } + static String json(Response response, int status, String message) { response.status(status); response.type("application/json"); @@ -159,16 +173,19 @@ static String json(Response response, int status, String message) { } void stop() { + if (!stopped.compareAndSet(false, true)) return; accepting.set(false); - Service service = http; - http = null; - if (service != null) { - service.stop(); - service.awaitStop(); + for (CompletableFuture future : pending) { + future.complete(new Lookup(503, "Service Unavailable")); } + if (http != null) http.stop(); + } + + void awaitStop() { + if (http != null) http.awaitStop(); } - private static final class Lookup { + static final class Lookup { private final String status; private final String message; diff --git a/src/test/java/me/fredthedoggy/restpapi/RestSecurityTest.java b/src/test/java/me/fredthedoggy/restpapi/RestSecurityTest.java index 17e219f..ce86656 100644 --- a/src/test/java/me/fredthedoggy/restpapi/RestSecurityTest.java +++ b/src/test/java/me/fredthedoggy/restpapi/RestSecurityTest.java @@ -7,6 +7,8 @@ import java.util.Arrays; import java.util.Collections; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; @@ -77,4 +79,17 @@ void rejectsOutOfRangeNetworkSettings() { yaml.set("max-concurrent", 1000); assertThrows(IllegalArgumentException.class, () -> new RestConfig(yaml)); } + + @Test + void shutdownUnblocksAnAwaitingHttpRequestWithoutRunningItsBukkitTask() throws Exception { + SparkWrapper server = new SparkWrapper(mock(Restpapi.class), new RestConfig(config())); + CompletableFuture lookup = new CompletableFuture<>(); + server.track(lookup); + server.stop(); + assertNotNull(lookup.get(100, TimeUnit.MILLISECONDS)); + + CompletableFuture lateLookup = new CompletableFuture<>(); + server.track(lateLookup); + assertNotNull(lateLookup.get(100, TimeUnit.MILLISECONDS)); + } }