From 29f91b94fa516a85de6f6d2d47e5dc05956e72a6 Mon Sep 17 00:00:00 2001 From: Thomas Meaney Date: Sat, 1 Aug 2026 14:48:52 +0200 Subject: [PATCH 1/2] feat: serve world downloads as pre-signed S3 links Adds world.download.storage, choosing between the built-in HTTP server (local, the default) and pre-signed links from the bucket the backups already use (s3). The S3 path needs no open port and imposes no size limit: archives upload in parts, streamed off disk, so neither the heap nor the five-gigabyte ceiling on a single PUT bounds a world. Where the archive goes is now a DownloadDelivery, since the mode was otherwise branched on in four places. WorldDownloadService keeps the export orchestration; the deliveries own their own lifecycle, budget and cleanup. Progress carries a phase, so the bar tracks the upload instead of sitting full while it runs. Deliveries reserve budget rather than sample it, links start their life when handed out rather than when the command was typed, exports are bounded by the staging disk's free space, and the purge runs off the main thread. Exports get their own thread so they cannot starve backups. behind-proxy keys pinning and rate limiting on the last X-Forwarded-For hop; off by default, since trusting the header on a directly reachable port would let anyone claim any address. Also stops /worlds setItem wiping a configured icon texture when the held head's profile carries none. --- .../subcommand/worlds/DownloadSubCommand.java | 36 ++- .../subcommand/worlds/SetItemSubCommand.java | 7 +- .../buildsystem/config/ConfigService.java | 19 ++ .../buildsystem/config/PluginConfig.java | 44 ++- .../world/backup/BackupServiceImpl.java | 4 +- .../world/backup/storage/s3/AwsV4Signer.java | 100 +++++- .../world/backup/storage/s3/Listing.java | 49 +-- .../backup/storage/s3/PercentEncoding.java | 19 ++ .../world/backup/storage/s3/S3Client.java | 214 +++++++++++-- .../world/backup/storage/s3/S3Xml.java | 114 +++++++ .../world/download/DownloadDelivery.java | 93 ++++++ .../world/download/DownloadProgress.java | 53 ++++ .../world/download/LocalDownloadDelivery.java | 300 ++++++++++++++++++ .../world/download/S3DownloadDelivery.java | 193 +++++++++++ .../world/download/WorldDownloadService.java | 290 +++++++---------- .../src/main/resources/config.yml | 23 +- .../src/main/resources/messages.yml | 1 + .../backup/storage/s3/AwsV4SignerTest.java | 29 ++ .../storage/s3/S3ClientPartSizeTest.java | 74 +++++ 19 files changed, 1391 insertions(+), 271 deletions(-) create mode 100644 buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/S3Xml.java create mode 100644 buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/DownloadDelivery.java create mode 100644 buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/DownloadProgress.java create mode 100644 buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/LocalDownloadDelivery.java create mode 100644 buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/S3DownloadDelivery.java create mode 100644 buildsystem-core/src/test/java/de/eintosti/buildsystem/world/backup/storage/s3/S3ClientPartSizeTest.java diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/DownloadSubCommand.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/DownloadSubCommand.java index 5e9c10db..095225f4 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/DownloadSubCommand.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/DownloadSubCommand.java @@ -27,6 +27,7 @@ import de.eintosti.buildsystem.util.TaskScheduler; import de.eintosti.buildsystem.util.WorldFlush; import de.eintosti.buildsystem.world.WorldServiceImpl; +import de.eintosti.buildsystem.world.download.DownloadProgress; import de.eintosti.buildsystem.world.download.ExportProgressBar; import de.eintosti.buildsystem.world.download.WorldDownloadService; import de.eintosti.buildsystem.world.download.WorldExporter; @@ -38,6 +39,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Pattern; @@ -109,14 +111,18 @@ public void execute(Player player, String worldName, String[] args) { messages.sendMessage(player, "worlds_download_preparing", worldPlaceholder); buildWorld.getWorld().ifPresent(WorldFlush::saveAndFlush); - AtomicLong packedBytes = new AtomicLong(); + AtomicLong doneBytes = new AtomicLong(); AtomicLong totalBytes = new AtomicLong(); - BukkitTask animation = startProgressAnimation(player, buildWorld, packedBytes, totalBytes); + AtomicReference phase = new AtomicReference<>(DownloadProgress.Phase.PACKING); + BukkitTask animation = startProgressAnimation(player, buildWorld, phase, doneBytes, totalBytes); downloadService - .prepare(buildWorld, (packed, total) -> { - packedBytes.set(packed); + .prepare(buildWorld, (currentPhase, done, total) -> { + // Set the phase last: it is what the animation reads to pick its message, so flipping it before + // the counters would draw the new phase with the old one's numbers. + doneBytes.set(done); totalBytes.set(total); + phase.set(currentPhase); }) .whenCompleteAsync( (url, throwable) -> { @@ -135,11 +141,16 @@ public void execute(Player player, String worldName, String[] args) { } /** - * Drives the action bar while the export runs. The frame counter advances every tick of this task, so the bar - * keeps moving even while a single large region file is being packed. + * Drives the action bar while the download is prepared. The frame counter advances every tick of this task, so the + * bar keeps moving even while a single large region file is being packed, and the message follows the phase so a + * full bar is never left standing over work that is still running. */ private BukkitTask startProgressAnimation( - Player player, BuildWorld buildWorld, AtomicLong packedBytes, AtomicLong totalBytes) { + Player player, + BuildWorld buildWorld, + AtomicReference phase, + AtomicLong doneBytes, + AtomicLong totalBytes) { AtomicInteger frame = new AtomicInteger(); return scheduler.runTimer( () -> { @@ -147,13 +158,13 @@ private BukkitTask startProgressAnimation( return; } long total = totalBytes.get(); - double fraction = total <= 0 ? 0 : (double) packedBytes.get() / total; + double fraction = total <= 0 ? 0 : (double) doneBytes.get() / total; int currentFrame = frame.getAndIncrement(); sendActionBar( player, messages.getString( - "worlds_download_progress", + messageKey(phase.get()), player, Placeholders.of() .add("%world%", buildWorld.getName()) @@ -166,6 +177,13 @@ private BukkitTask startProgressAnimation( ANIMATION_PERIOD_TICKS); } + private static String messageKey(DownloadProgress.Phase phase) { + return switch (phase) { + case PACKING -> "worlds_download_progress"; + case PUBLISHING -> "worlds_download_uploading"; + }; + } + private static void sendActionBar(Player player, String message) { player.spigot().sendMessage(ChatMessageType.ACTION_BAR, TextComponent.fromLegacyText(message)); } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SetItemSubCommand.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SetItemSubCommand.java index e45534ed..be883715 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SetItemSubCommand.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SetItemSubCommand.java @@ -56,7 +56,12 @@ public void execute(Player player, String worldName, String[] args) { buildWorld.setIcon(itemStack.getType()); if (itemStack.getItemMeta() instanceof SkullMeta) { - buildWorld.setIconSkullTexture(skullTexture(itemStack)); + // Only overwrite a stored texture with one we actually read. A head whose profile needs a Mojang lookup + // reads as null here, and taking that as "no texture" would wipe whatever was configured before. + String texture = skullTexture(itemStack); + if (texture != null) { + buildWorld.setIconSkullTexture(texture); + } } messages.sendMessage(player, "worlds_setitem_set", Placeholders.of("%world%", buildWorld.getName())); } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/ConfigService.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/ConfigService.java index b3b3517b..f342c71f 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/ConfigService.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/ConfigService.java @@ -201,8 +201,10 @@ private static PluginConfig.World parseWorld(FileConfiguration config, Logger lo int downloadPort = config.getInt("world.download.port", 8080); PluginConfig.World.Download download = new PluginConfig.World.Download( config.getBoolean("world.download.enabled", false), + downloadStorage(config, logger), downloadPort, Objects.requireNonNullElse(config.getString("world.download.url"), "http://localhost:" + downloadPort), + config.getBoolean("world.download.behind-proxy", false), Math.max(1, config.getInt("world.download.expiration-minutes", 30)), Math.max(1, config.getInt("world.download.max-size-mb", 2048)), Math.max(1, config.getInt("world.download.max-storage-mb", 8192)), @@ -227,6 +229,23 @@ private static PluginConfig.World parseWorld(FileConfiguration config, Logger lo download); } + /** + * {@return where prepared world archives are served from} An unrecognised value falls back to the built-in server + * rather than disabling downloads, so a typo does not silently take the feature away. + */ + private static PluginConfig.World.Download.Storage downloadStorage(FileConfiguration config, Logger logger) { + String type = Objects.requireNonNullElse(config.getString("world.download.storage"), "local") + .toLowerCase(Locale.ROOT); + return switch (type) { + case "local" -> PluginConfig.World.Download.Storage.LOCAL; + case "s3" -> PluginConfig.World.Download.Storage.S3; + default -> { + logger.warning("Unknown download storage type '" + type + "', defaulting to local storage."); + yield PluginConfig.World.Download.Storage.LOCAL; + } + }; + } + private static PluginConfig.World.VoidBlock parseVoidBlock(FileConfiguration config, Logger logger) { boolean enabled = config.getBoolean("world.void-block.enabled", true); String raw = Objects.requireNonNullElse(config.getString("world.void-block.material"), "GOLD_BLOCK"); diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/PluginConfig.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/PluginConfig.java index 0cdffc7f..5afe56a8 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/PluginConfig.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/PluginConfig.java @@ -163,6 +163,26 @@ public record S3( @Nullable String path) implements StorageSettings { + /** + * {@return the access key, preferring {@code AWS_ACCESS_KEY_ID}} Lets operators keep the secret out of + * config.yml. + */ + public @Nullable String resolvedAccessKey() { + return envOrConfig("AWS_ACCESS_KEY_ID", accessKey); + } + + /** + * {@return the secret key, preferring {@code AWS_SECRET_ACCESS_KEY}} + */ + public @Nullable String resolvedSecretKey() { + return envOrConfig("AWS_SECRET_ACCESS_KEY", secretKey); + } + + private static @Nullable String envOrConfig(String envKey, @Nullable String configValue) { + String env = System.getenv(envKey); + return env == null || env.isBlank() ? configValue : env; + } + /** * Overridden so the access key and secret key never appear in a log or pasted support output. */ @@ -187,18 +207,40 @@ public record AutoBackup(boolean enabled, boolean onlyActiveWorlds, int interval * hands out world archives, so it stays an explicit decision by the operator. * * @param enabled Whether the download server runs at all + * @param storage Where a prepared archive is served from * @param port The port the server listens on * @param url The base URL players are sent, for servers reached through a proxy or a domain + * @param behindProxy Whether to take the client's address from {@code X-Forwarded-For} rather than the socket * @param expirationMinutes How long a download link stays valid before the archive is deleted */ public record Download( boolean enabled, + Storage storage, int port, String url, + boolean behindProxy, int expirationMinutes, int maxSizeMb, int maxStorageMb, - int maxConcurrentDownloads) {} + int maxConcurrentDownloads) { + + /** + * Where a prepared archive lives while a player fetches it. + */ + public enum Storage { + + /** + * Served by the plugin's own HTTP server, from the port above. + */ + LOCAL, + + /** + * Uploaded to the bucket the backups already use and handed out as a pre-signed link, so no port has + * to be opened. Requires {@code world.backup.storage.type: s3}. + */ + S3 + } + } } public record Folder(boolean overridePermissions, boolean overrideProjects) {} diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupServiceImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupServiceImpl.java index 1f8283fa..9886c01d 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupServiceImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupServiceImpl.java @@ -144,8 +144,8 @@ yield new SftpBackupStorage( s.path()); } case PluginConfig.World.Backup.S3 s3 -> { - String accessKey = envOrConfig("AWS_ACCESS_KEY_ID", s3.accessKey()); - String secretKey = envOrConfig("AWS_SECRET_ACCESS_KEY", s3.secretKey()); + String accessKey = s3.resolvedAccessKey(); + String secretKey = s3.resolvedSecretKey(); requireNonBlank(accessKey, "backup.s3.access-key (or AWS_ACCESS_KEY_ID)"); requireNonBlank(secretKey, "backup.s3.secret-key (or AWS_SECRET_ACCESS_KEY)"); requireNonBlank(s3.region(), "backup.s3.region"); diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/AwsV4Signer.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/AwsV4Signer.java index 0d4f9702..fec7275b 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/AwsV4Signer.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/AwsV4Signer.java @@ -20,6 +20,7 @@ import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +import java.time.Duration; import java.time.Instant; import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; @@ -45,6 +46,18 @@ final class AwsV4Signer { private static final String ALGORITHM = "AWS4-HMAC-SHA256"; private static final String SERVICE = "s3"; + + /** + * Stands in for the payload hash when the body is too large to hash before sending. S3 accepts it in place of a + * real digest, at the cost of the signature no longer covering the body. + */ + static final String UNSIGNED_PAYLOAD = "UNSIGNED-PAYLOAD"; + + /** + * The longest life S3 grants a pre-signed URL. + */ + private static final Duration MAX_PRESIGNED_EXPIRY = Duration.ofDays(7); + private static final DateTimeFormatter AMZ_DATE = DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss'Z'").withZone(ZoneOffset.UTC); private static final DateTimeFormatter SCOPE_DATE = @@ -81,9 +94,31 @@ Map sign( Map headers, byte[] payload, Instant timestamp) { + return sign(method, host, canonicalUri, canonicalQuery, headers, hex(sha256(payload)), timestamp); + } + + /** + * Signs a request whose body has already been hashed, for one too large to hold in memory. + * + * @param method The HTTP method + * @param host The request host, which must match the {@code Host} header actually sent + * @param canonicalUri The already-encoded path, beginning with {@code /} + * @param canonicalQuery The already-encoded and sorted query string, or empty + * @param headers Additional headers to sign + * @param payloadHash The hex-encoded SHA-256 of the body, or {@link #UNSIGNED_PAYLOAD} + * @param timestamp The signing time + * @return The headers to send, including the signature + */ + Map sign( + String method, + String host, + String canonicalUri, + String canonicalQuery, + Map headers, + String payloadHash, + Instant timestamp) { String amzDate = AMZ_DATE.format(timestamp); String scopeDate = SCOPE_DATE.format(timestamp); - String payloadHash = hex(sha256(payload)); SortedMap canonicalHeaders = new TreeMap<>(); headers.forEach((name, value) -> { @@ -112,17 +147,8 @@ Map sign( + '\n' + payloadHash; - String scope = scopeDate + "/" + region + "/" + SERVICE + "/aws4_request"; - String stringToSign = ALGORITHM - + '\n' - + amzDate - + '\n' - + scope - + '\n' - + hex(sha256(canonicalRequest.getBytes(StandardCharsets.UTF_8))); - - byte[] signingKey = signingKey(scopeDate); - String signature = hex(hmac(signingKey, stringToSign)); + String scope = scope(scopeDate); + String signature = hex(hmac(signingKey(scopeDate), stringToSign(amzDate, scope, canonicalRequest))); Map signed = new LinkedHashMap<>(headers); signed.put("x-amz-date", amzDate); @@ -134,6 +160,56 @@ Map sign( return signed; } + /** + * Signs a {@code GET} as a pre-signed URL: everything that would be a signature header moves into the query, so + * the URL alone authorises the request and any HTTP client can follow it without credentials. + * + * @param host The request host + * @param canonicalUri The already-encoded path, beginning with {@code /} + * @param expiry How long the URL stays valid, capped at the seven days S3 allows + * @param timestamp The signing time + * @return The encoded query string to append to the URL + */ + String presignGet(String host, String canonicalUri, Duration expiry, Instant timestamp) { + String amzDate = AMZ_DATE.format(timestamp); + String scopeDate = SCOPE_DATE.format(timestamp); + String scope = scope(scopeDate); + + Map parameters = new TreeMap<>(); + parameters.put("X-Amz-Algorithm", ALGORITHM); + parameters.put("X-Amz-Credential", accessKey + "/" + scope); + parameters.put("X-Amz-Date", amzDate); + parameters.put("X-Amz-Expires", Long.toString(expirySeconds(expiry))); + parameters.put("X-Amz-SignedHeaders", "host"); + String canonicalQuery = PercentEncoding.query(parameters); + + // Only the host is signed, and the body is a response rather than a request, so there is nothing to hash. + String canonicalRequest = + "GET\n" + canonicalUri + '\n' + canonicalQuery + "\nhost:" + host + "\n\nhost\n" + UNSIGNED_PAYLOAD; + + String signature = hex(hmac(signingKey(scopeDate), stringToSign(amzDate, scope, canonicalRequest))); + return canonicalQuery + "&X-Amz-Signature=" + signature; + } + + private static long expirySeconds(Duration expiry) { + Duration capped = expiry.compareTo(MAX_PRESIGNED_EXPIRY) > 0 ? MAX_PRESIGNED_EXPIRY : expiry; + return Math.max(1L, capped.toSeconds()); + } + + private String scope(String scopeDate) { + return scopeDate + "/" + region + "/" + SERVICE + "/aws4_request"; + } + + private static String stringToSign(String amzDate, String scope, String canonicalRequest) { + return ALGORITHM + + '\n' + + amzDate + + '\n' + + scope + + '\n' + + hex(sha256(canonicalRequest.getBytes(StandardCharsets.UTF_8))); + } + /** * Rejects a header name or value containing a line break. The canonical request is newline-delimited, so an * injected break would let a caller forge the signed content. diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/Listing.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/Listing.java index e9222ed6..850d338f 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/Listing.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/Listing.java @@ -18,21 +18,16 @@ package de.eintosti.buildsystem.world.backup.storage.s3; import de.eintosti.buildsystem.world.backup.storage.s3.S3Client.S3Object; -import java.io.ByteArrayInputStream; import java.io.IOException; import java.time.Instant; import java.time.format.DateTimeParseException; import java.util.ArrayList; import java.util.List; -import javax.xml.XMLConstants; -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.ParserConfigurationException; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.NodeList; -import org.xml.sax.SAXException; /** * One page of a {@code ListObjectsV2} response. @@ -51,58 +46,26 @@ record Listing(List objects, @Nullable String nextContinuationToken) { * @throws IOException If the document is not a listing this client understands */ static Listing parse(byte[] xml) throws IOException { - Document document = parseSafely(xml); + Document document = S3Xml.parse(xml, "listing"); + Element root = document.getDocumentElement(); List objects = new ArrayList<>(); NodeList contents = document.getElementsByTagName("Contents"); for (int i = 0; i < contents.getLength(); i++) { Element entry = (Element) contents.item(i); - objects.add(new S3Object(required(entry, "Key"), lastModified(entry))); + objects.add(new S3Object(S3Xml.required(entry, "Key"), lastModified(entry))); } - boolean truncated = Boolean.parseBoolean(optional(document, "IsTruncated")); - return new Listing(objects, truncated ? optional(document, "NextContinuationToken") : null); + boolean truncated = Boolean.parseBoolean(S3Xml.optional(root, "IsTruncated")); + return new Listing(objects, truncated ? S3Xml.optional(root, "NextContinuationToken") : null); } private static Instant lastModified(Element entry) throws IOException { - String value = required(entry, "LastModified"); + String value = S3Xml.required(entry, "LastModified"); try { return Instant.parse(value); } catch (DateTimeParseException e) { throw new IOException("S3 returned an unreadable LastModified: " + value, e); } } - - /** - * {@return the document parsed with entity resolution disabled} The response is remote input, so a hostile or - * compromised endpoint must not be able to make the parser read local files or fetch URLs. - */ - private static Document parseSafely(byte[] xml) throws IOException { - try { - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - factory.setFeature("http://xml.org/sax/features/external-general-entities", false); - factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); - factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); - factory.setXIncludeAware(false); - factory.setExpandEntityReferences(false); - return factory.newDocumentBuilder().parse(new ByteArrayInputStream(xml)); - } catch (ParserConfigurationException | SAXException e) { - throw new IOException("Could not parse the S3 listing", e); - } - } - - private static String required(Element parent, String tag) throws IOException { - NodeList nodes = parent.getElementsByTagName(tag); - if (nodes.getLength() == 0 || nodes.item(0).getTextContent() == null) { - throw new IOException("S3 listing entry is missing <" + tag + ">"); - } - return nodes.item(0).getTextContent().trim(); - } - - private static @Nullable String optional(Document document, String tag) { - NodeList nodes = document.getElementsByTagName(tag); - return nodes.getLength() == 0 ? null : nodes.item(0).getTextContent().trim(); - } } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/PercentEncoding.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/PercentEncoding.java index 4da6f3e6..8cb435c4 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/PercentEncoding.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/PercentEncoding.java @@ -18,6 +18,8 @@ package de.eintosti.buildsystem.world.backup.storage.s3; import java.nio.charset.StandardCharsets; +import java.util.Map; +import java.util.TreeMap; import org.jspecify.annotations.NullMarked; /** @@ -68,6 +70,23 @@ static String encodePath(String key) { return encoded.toString(); } + /** + * {@return the parameters as a query string, sorted and encoded the way SigV4 canonicalises them} The same string + * is both signed and sent, so a request can never disagree with its own signature. + * + * @param parameters The query parameters + */ + static String query(Map parameters) { + StringBuilder query = new StringBuilder(); + new TreeMap<>(parameters).forEach((name, value) -> { + if (!query.isEmpty()) { + query.append('&'); + } + query.append(encode(name)).append('=').append(encode(value)); + }); + return query.toString(); + } + private static boolean isUnreserved(char c) { return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/S3Client.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/S3Client.java index 44a61e13..1d012d54 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/S3Client.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/S3Client.java @@ -17,7 +17,10 @@ */ package de.eintosti.buildsystem.world.backup.storage.s3; +import com.google.common.io.ByteStreams; import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpRequest; @@ -33,8 +36,10 @@ import java.util.List; import java.util.Map; import java.util.TreeMap; +import java.util.function.LongConsumer; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; +import org.w3c.dom.Element; /** * A minimal S3 client covering the four operations the backup storage needs: list, put, get and delete. Written @@ -50,6 +55,23 @@ public final class S3Client implements AutoCloseable { private static final Duration CONNECT_TIMEOUT = Duration.ofSeconds(30); private static final Duration REQUEST_TIMEOUT = Duration.ofMinutes(5); + /** + * Uploading a world archive is bounded by the server's uplink rather than by S3, so it gets its own budget: a + * gigabyte over a slow connection legitimately takes longer than the timeout the small requests use. + */ + private static final Duration UPLOAD_TIMEOUT = Duration.ofHours(2); + + /** + * The part size used until a file is large enough to need bigger ones. Comfortably above the five megabytes S3 + * requires of every part but the last, and small enough that a failed part is cheap to lose. + */ + private static final long MIN_PART_SIZE = 16L * 1024 * 1024; + + /** + * The most parts S3 accepts in one upload. + */ + private static final long MAX_PARTS = 10_000L; + private final HttpClient http; private final AwsV4Signer signer; private final BucketEndpoint endpoint; @@ -124,6 +146,126 @@ public void put(String key, byte[] content) throws IOException { body(request("PUT", key, Map.of(), Payload.of(content), BodyHandlers.ofByteArray()), "upload " + key); } + /** + * Uploads a file as a multipart upload, streaming each part off disk so neither the heap nor the five-gigabyte + * ceiling on a single {@code PUT} bounds how large the file may be. + * + *

A failed upload is aborted rather than abandoned. An incomplete multipart upload does not show up in a + * listing but is still billed for. + * + * @param key The object key + * @param file The file to upload + * @param uploaded Notified with the running total of bytes accepted by S3, for reporting progress + * @throws IOException If the request fails or S3 returns an error + */ + public void putFile(String key, Path file, LongConsumer uploaded) throws IOException { + long size = Files.size(file); + long partSize = partSize(size); + String uploadId = createMultipartUpload(key); + + try { + List etags = new ArrayList<>(); + long offset = 0L; + while (offset < size || etags.isEmpty()) { + long length = Math.min(partSize, size - offset); + etags.add(uploadPart(key, uploadId, etags.size() + 1, file, offset, length)); + offset += length; + uploaded.accept(offset); + } + completeMultipartUpload(key, uploadId, etags); + } catch (IOException | RuntimeException e) { + abortMultipartUpload(key, uploadId, e); + throw e; + } + } + + /** + * {@return the part size to split a file of {@code size} into} Grown when the file would otherwise need more than + * the ten thousand parts S3 allows, so the part count is bounded rather than the file size. + */ + static long partSize(long size) { + long needed = (size + MAX_PARTS - 1) / MAX_PARTS; + return Math.max(MIN_PART_SIZE, needed); + } + + private String createMultipartUpload(String key) throws IOException { + byte[] xml = body( + request("POST", key, Map.of("uploads", ""), Payload.empty(), BodyHandlers.ofByteArray()), + "start a multipart upload of " + key); + return S3Xml.required(S3Xml.parse(xml, "multipart upload").getDocumentElement(), "UploadId"); + } + + /** + * {@return the part's ETag, which the completion request must quote back} + */ + private String uploadPart(String key, String uploadId, int partNumber, Path file, long offset, long length) + throws IOException { + Map query = Map.of("partNumber", Integer.toString(partNumber), "uploadId", uploadId); + HttpResponse response = request( + "PUT", + key, + query, + Payload.ofFileRange(file, offset, length), + BodyHandlers.ofByteArray(), + UPLOAD_TIMEOUT); + body(response, "upload part " + partNumber + " of " + key); + + return response.headers() + .firstValue("ETag") + .orElseThrow(() -> new IOException("S3 did not return an ETag for part " + partNumber + " of " + key)); + } + + private void completeMultipartUpload(String key, String uploadId, List etags) throws IOException { + StringBuilder xml = new StringBuilder(""); + for (int i = 0; i < etags.size(); i++) { + xml.append("") + .append(i + 1) + .append("") + .append(S3Xml.escape(etags.get(i))) + .append(""); + } + xml.append(""); + + byte[] response = body( + request( + "POST", + key, + Map.of("uploadId", uploadId), + Payload.of(xml.toString().getBytes(StandardCharsets.UTF_8)), + BodyHandlers.ofByteArray()), + "complete the multipart upload of " + key); + + // S3 answers 200 and only then reports a failure inside the document, so the status alone proves nothing. + Element root = S3Xml.parse(response, "multipart completion").getDocumentElement(); + if ("Error".equals(root.getTagName())) { + throw new IOException("S3 failed to complete the multipart upload of " + key + " - " + + new String(response, StandardCharsets.UTF_8).trim()); + } + } + + /** + * Discards a half-finished upload, keeping the original failure as the one that surfaces. + */ + private void abortMultipartUpload(String key, String uploadId, Throwable cause) { + try { + request("DELETE", key, Map.of("uploadId", uploadId), Payload.empty(), BodyHandlers.ofByteArray()); + } catch (IOException | RuntimeException e) { + cause.addSuppressed(new IOException("Failed to abort the multipart upload of " + key, e)); + } + } + + /** + * {@return a URL that grants anyone holding it a single object for a limited time} Nothing is sent to S3 to make + * one: the URL carries its own signature, so it can be handed out and followed by a plain browser. + * + * @param key The object key + * @param expiry How long the URL stays valid, capped at the seven days S3 allows + */ + public String presignedGetUrl(String key, Duration expiry) { + String query = signer.presignGet(endpoint.host(), endpoint.pathOf(key), expiry, Instant.now()); + return endpoint.urlOf(key, query); + } + /** * Downloads an object to a file, streaming it rather than buffering: a world backup can be far larger than the * heap the server has spare. @@ -161,18 +303,23 @@ public void delete(String key) throws IOException { private HttpResponse request( String method, String key, Map query, Payload payload, BodyHandler handler) throws IOException { - String canonicalQuery = canonicalQuery(query); + return request(method, key, query, payload, handler, REQUEST_TIMEOUT); + } + + private HttpResponse request( + String method, + String key, + Map query, + Payload payload, + BodyHandler handler, + Duration timeout) + throws IOException { + String canonicalQuery = PercentEncoding.query(query); Map headers = signer.sign( - method, - endpoint.host(), - endpoint.pathOf(key), - canonicalQuery, - Map.of(), - payload.bytes(), - Instant.now()); + method, endpoint.host(), endpoint.pathOf(key), canonicalQuery, Map.of(), payload.hash(), Instant.now()); HttpRequest.Builder request = HttpRequest.newBuilder(URI.create(endpoint.urlOf(key, canonicalQuery))) - .timeout(REQUEST_TIMEOUT) + .timeout(timeout) .method(method, payload.publisher()); headers.forEach(request::header); @@ -212,42 +359,47 @@ private static String failureMessage(String action, int status, byte[] errorDocu return "S3 request failed to " + action + " (HTTP " + status + ")" + detail; } - /** - * {@return the query string in the sorted, encoded form SigV4 requires} - */ - private static String canonicalQuery(Map query) { - StringBuilder canonical = new StringBuilder(); - new TreeMap<>(query).forEach((name, value) -> { - if (!canonical.isEmpty()) { - canonical.append('&'); - } - canonical.append(PercentEncoding.encode(name)).append('=').append(PercentEncoding.encode(value)); - }); - return canonical.toString(); - } - @Override public void close() { http.close(); } /** - * A request body, pairing the bytes that get signed with the publisher that sends them so the two cannot drift. + * A request body, pairing the hash that gets signed with the publisher that sends it so the two cannot drift. */ - private record Payload(byte[] bytes) { + private record Payload(String hash, HttpRequest.BodyPublisher publisher) { static Payload empty() { - return new Payload(new byte[0]); + return new Payload(AwsV4Signer.hex(AwsV4Signer.sha256(new byte[0])), HttpRequest.BodyPublishers.noBody()); } static Payload of(byte[] bytes) { - return new Payload(bytes); + return new Payload( + AwsV4Signer.hex(AwsV4Signer.sha256(bytes)), HttpRequest.BodyPublishers.ofByteArray(bytes)); } - HttpRequest.BodyPublisher publisher() { - return bytes.length == 0 - ? HttpRequest.BodyPublishers.noBody() - : HttpRequest.BodyPublishers.ofByteArray(bytes); + /** + * {@return a payload streamed from a slice of a file} S3 rejects a chunked body, so the length is declared up + * front and the bytes are read on demand, keeping the part out of memory. + * + *

Sent unsigned: hashing the body first would mean reading the archive a second time. The signature covers + * the request but not its contents, which TLS protects in transit. + * + * @param file The file to read from + * @param offset Where the slice starts + * @param length How many bytes the slice holds + */ + static Payload ofFileRange(Path file, long offset, long length) { + HttpRequest.BodyPublisher slice = HttpRequest.BodyPublishers.ofInputStream(() -> { + try { + InputStream in = Files.newInputStream(file); + in.skipNBytes(offset); + return ByteStreams.limit(in, length); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); + return new Payload(AwsV4Signer.UNSIGNED_PAYLOAD, HttpRequest.BodyPublishers.fromPublisher(slice, length)); } } } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/S3Xml.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/S3Xml.java new file mode 100644 index 00000000..2e289141 --- /dev/null +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/S3Xml.java @@ -0,0 +1,114 @@ +/* + * Copyright (c) 2018-2026, Thomas Meaney + * Copyright (c) contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package de.eintosti.buildsystem.world.backup.storage.s3; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.NodeList; +import org.xml.sax.SAXException; + +/** + * Reads the XML documents S3 answers with. In one place so every response goes through the same hardened parser + * configuration. + */ +@NullMarked +final class S3Xml { + + private S3Xml() {} + + /** + * {@return the document parsed with entity resolution disabled} A response is remote input, so a hostile or + * compromised endpoint must not be able to make the parser read local files or fetch URLs. + * + * @param xml The response document + * @param what What was being read, for the error message + * @throws IOException If the bytes are not parseable XML + */ + static Document parse(byte[] xml, String what) throws IOException { + try { + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + factory.setXIncludeAware(false); + factory.setExpandEntityReferences(false); + return factory.newDocumentBuilder().parse(new ByteArrayInputStream(xml)); + } catch (ParserConfigurationException | SAXException e) { + throw new IOException("Could not parse the S3 " + what, e); + } + } + + /** + * {@return the text of the first {@code tag} below {@code parent}} + * + * @param parent The element to search + * @param tag The tag name + * @throws IOException If the element is absent + */ + static String required(Element parent, String tag) throws IOException { + String value = optional(parent, tag); + if (value == null) { + throw new IOException("S3 response is missing <" + tag + ">"); + } + return value; + } + + /** + * {@return the text of the first {@code tag} below {@code parent}, or {@code null} if there is none} + * + * @param parent The element to search + * @param tag The tag name + */ + static @Nullable String optional(Element parent, String tag) { + NodeList nodes = parent.getElementsByTagName(tag); + if (nodes.getLength() == 0 || nodes.item(0).getTextContent() == null) { + return null; + } + return nodes.item(0).getTextContent().trim(); + } + + /** + * Escapes text for inclusion in a request document. + * + * @param value The text to escape + * @return The escaped text + */ + static String escape(String value) { + StringBuilder escaped = new StringBuilder(value.length()); + for (char c : value.toCharArray()) { + switch (c) { + case '&' -> escaped.append("&"); + case '<' -> escaped.append("<"); + case '>' -> escaped.append(">"); + case '"' -> escaped.append("""); + case '\'' -> escaped.append("'"); + default -> escaped.append(c); + } + } + return escaped.toString(); + } +} diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/DownloadDelivery.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/DownloadDelivery.java new file mode 100644 index 00000000..172a81c2 --- /dev/null +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/DownloadDelivery.java @@ -0,0 +1,93 @@ +/* + * Copyright (c) 2018-2026, Thomas Meaney + * Copyright (c) contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package de.eintosti.buildsystem.world.download; + +import java.io.IOException; +import java.nio.file.Path; +import java.time.Duration; +import org.jspecify.annotations.NullMarked; + +/** + * Turns a packed world into a link and owns whatever that link needs until it expires. {@link WorldDownloadService} + * does the exporting; where the archive then goes is a delivery's business. + */ +@NullMarked +interface DownloadDelivery extends AutoCloseable { + + /** + * Makes an archive downloadable. + * + *

Called off the main thread, and takes ownership of {@code archive}: the delivery either keeps the file until + * the link expires or deletes it once the bytes are somewhere else. + * + * @param archive The packed world + * @param fileName The name the player should save it as + * @param lifetime How long the link stays valid, counted from the moment it is handed out rather than from when + * the export was asked for, so packing and uploading do not eat into it + * @param progress Notified with the running total of bytes published, where publishing takes measurable time + * @return The URL to hand the player + * @throws IOException If the archive cannot be published + */ + String publish(Path archive, String fileName, Duration lifetime, ProgressListener progress) throws IOException; + + /** + * Claims room for one archive against whatever budget this delivery keeps. + * + *

Reserving rather than merely reading the free space is what keeps two exports started at the same moment from + * both being told the whole remainder is theirs. Every granted reservation must be handed back to + * {@link #release(long)}. + * + * @return The largest archive the caller may now produce, {@link Long#MAX_VALUE} when the delivery bounds nothing, + * or zero or less when it is momentarily full + */ + long reserve(); + + /** + * Hands back a reservation, whether the archive was published or the export failed. + * + * @param reserved What {@link #reserve()} granted + */ + void release(long reserved); + + /** + * {@return whether publishing takes long enough to be worth showing the player} False when it only has to + * register a local file. + */ + boolean reportsPublishProgress(); + + /** + * Drops everything whose link has expired. + */ + void purgeExpired(); + + @Override + void close(); + + /** + * Notified as bytes are published. + */ + @FunctionalInterface + interface ProgressListener { + + /** + * @param published How many bytes have been published so far + * @param total How many there are in total + */ + void update(long published, long total); + } +} diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/DownloadProgress.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/DownloadProgress.java new file mode 100644 index 00000000..cf9fac79 --- /dev/null +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/DownloadProgress.java @@ -0,0 +1,53 @@ +/* + * Copyright (c) 2018-2026, Thomas Meaney + * Copyright (c) contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package de.eintosti.buildsystem.world.download; + +import org.jspecify.annotations.NullMarked; + +/** + * Notified as a world is made downloadable. Carries the phase so the player sees which step is running, rather than a + * bar that fills and then sits there while a later step works. + */ +@NullMarked +@FunctionalInterface +public interface DownloadProgress { + + /** + * @param phase What is happening now + * @param done How far the phase has got, in bytes + * @param total How many bytes the phase covers, or zero while that is not yet known + */ + void update(Phase phase, long done, long total); + + /** + * The steps a download goes through, in order. + */ + enum Phase { + + /** + * The world is being packed into an archive. + */ + PACKING, + + /** + * The archive is being put where the player can fetch it. Instant when it is served locally, as long as the + * uplink needs when it is uploaded. + */ + PUBLISHING + } +} diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/LocalDownloadDelivery.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/LocalDownloadDelivery.java new file mode 100644 index 00000000..34f2682d --- /dev/null +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/LocalDownloadDelivery.java @@ -0,0 +1,300 @@ +/* + * Copyright (c) 2018-2026, Thomas Meaney + * Copyright (c) contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package de.eintosti.buildsystem.world.download; + +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpServer; +import de.eintosti.buildsystem.config.ConfigService; +import de.eintosti.buildsystem.config.PluginConfig; +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetSocketAddress; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; +import java.util.List; +import java.util.Locale; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Semaphore; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.logging.Level; +import java.util.logging.Logger; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +/** + * Serves archives from the game server itself, over an HTTP server on the configured port. + * + *

An archive is only ever reachable through the unguessable, expiring token it was registered under: the token is + * the whole URL path, so no request can name a file, and nothing outside the plugin's {@code downloads} directory is + * served. A link is pinned to the first client that uses it, requests are rate limited per address, and concurrent + * transfers are capped so downloads cannot crowd each other out. + */ +@NullMarked +final class LocalDownloadDelivery implements DownloadDelivery { + + private static final String CONTEXT_PATH = "/download/"; + private static final int MAX_REQUESTS_PER_WINDOW = 30; + private static final long RATE_LIMIT_WINDOW_MILLIS = Duration.ofMinutes(1).toMillis(); + + private final ConfigService configService; + private final Logger logger; + + private final DownloadRegistry registry = new DownloadRegistry(); + private final RequestRateLimiter rateLimiter = + new RequestRateLimiter(MAX_REQUESTS_PER_WINDOW, RATE_LIMIT_WINDOW_MILLIS); + + private final HttpServer server; + private final ExecutorService httpExecutor; + private final Semaphore transferSlots; + + /** + * Budget claimed by exports that are still packing, and so have no file for the registry to count yet. + */ + private long reserved; + + private LocalDownloadDelivery( + ConfigService configService, + Logger logger, + HttpServer server, + ExecutorService httpExecutor, + int concurrentDownloads) { + this.configService = configService; + this.logger = logger; + this.server = server; + this.httpExecutor = httpExecutor; + this.transferSlots = new Semaphore(concurrentDownloads); + } + + /** + * {@return a delivery serving on the configured port, or {@code null} if the port could not be bound} The reason + * is logged, so a failure to start reads as a configuration problem rather than a missing feature. + * + * @param configService The live configuration + * @param logger The plugin logger + */ + static @Nullable LocalDownloadDelivery open(ConfigService configService, Logger logger) { + PluginConfig.World.Download config = configService.current().world().download(); + int concurrentDownloads = Math.max(1, config.maxConcurrentDownloads()); + ExecutorService executor = Executors.newFixedThreadPool(concurrentDownloads + 1, threadFactory()); + + LocalDownloadDelivery delivery; + try { + HttpServer server = HttpServer.create(new InetSocketAddress(config.port()), 0); + delivery = new LocalDownloadDelivery(configService, logger, server, executor, concurrentDownloads); + server.createContext(CONTEXT_PATH, delivery::handle); + server.setExecutor(executor); + server.start(); + } catch (IOException e) { + executor.shutdownNow(); + logger.log(Level.SEVERE, "Failed to start the world download server on port " + config.port(), e); + return null; + } + + logger.info("World downloads are available on port " + config.port()); + warnAboutPlaintext(config, logger); + return delivery; + } + + @Override + public String publish(Path archive, String fileName, Duration lifetime, ProgressListener progress) { + return url(registry.register(archive, fileName, System.currentTimeMillis() + lifetime.toMillis())); + } + + /** + * {@return what is left of the storage budget, capped by the largest single export allowed} Both limits exist + * because every live archive sits on the game server's own disk until its link expires. + * + *

Synchronized, and counting reservations that have not produced a file yet, so exports running at the same + * time divide the budget instead of each being promised all of it. + */ + @Override + public synchronized long reserve() { + PluginConfig.World.Download config = config(); + long remaining = megabytes(config.maxStorageMb()) - registry.totalBytes() - reserved; + long granted = Math.min(megabytes(config.maxSizeMb()), remaining); + if (granted > 0) { + reserved += granted; + } + return granted; + } + + @Override + public synchronized void release(long amount) { + if (amount > 0) { + reserved = Math.max(0L, reserved - amount); + } + } + + /** + * {@return false} Registering a token is instant; the transfer itself happens later, when the player clicks. + */ + @Override + public boolean reportsPublishProgress() { + return false; + } + + @Override + public void purgeExpired() { + registry.purgeExpired(this::delete); + rateLimiter.purgeStale(); + } + + @Override + public void close() { + server.stop(0); + httpExecutor.shutdownNow(); + registry.clear(); + rateLimiter.clear(); + } + + /** + * Warns when links leave the server unencrypted. A token in a plaintext URL is readable by anything on the path, + * so the only safe plain-HTTP setup is one that terminates TLS in front of this server. + */ + private static void warnAboutPlaintext(PluginConfig.World.Download config, Logger logger) { + if (config.url().toLowerCase(Locale.ROOT).startsWith("https://")) { + return; + } + logger.warning("World downloads are served over plain HTTP." + + " Anyone able to observe the traffic can reuse a download link." + + " Put the port behind a TLS proxy and point world.download.url at it."); + } + + private void handle(HttpExchange exchange) throws IOException { + String clientAddress = clientAddress(exchange); + try (exchange) { + if (!rateLimiter.allow(clientAddress)) { + respondEmpty(exchange, 429); + return; + } + + if (!"GET".equalsIgnoreCase(exchange.getRequestMethod())) { + respondEmpty(exchange, 405); + return; + } + + DownloadRegistry.Download download = registry.claim(token(exchange), clientAddress); + if (download == null || !Files.isRegularFile(download.file())) { + respondEmpty(exchange, 404); + return; + } + + transfer(exchange, download); + } catch (IOException e) { + logger.log(Level.FINE, "World download aborted", e); + } + } + + private void transfer(HttpExchange exchange, DownloadRegistry.Download download) throws IOException { + if (!transferSlots.tryAcquire()) { + exchange.getResponseHeaders().set("Retry-After", "30"); + respondEmpty(exchange, 503); + return; + } + + try { + exchange.getResponseHeaders().set("Content-Type", "application/zip"); + exchange.getResponseHeaders() + .set("Content-Disposition", "attachment; filename=\"" + download.fileName() + "\""); + exchange.getResponseHeaders().set("X-Content-Type-Options", "nosniff"); + exchange.getResponseHeaders().set("Cache-Control", "no-store"); + exchange.sendResponseHeaders(200, Files.size(download.file())); + + try (OutputStream out = exchange.getResponseBody()) { + Files.copy(download.file(), out); + } + } finally { + transferSlots.release(); + } + } + + /** + * {@return the requested token, or the empty string for any request shaped differently} The token is the entire + * path below the context, so a request can neither name a file nor escape the download folder. + */ + private static String token(HttpExchange exchange) { + String path = exchange.getRequestURI().getPath(); + return path.length() > CONTEXT_PATH.length() ? path.substring(CONTEXT_PATH.length()) : ""; + } + + /** + * {@return the address a request is attributed to, for pinning and rate limiting} + * + *

Behind a reverse proxy every request arrives from the proxy, which would pin all links to one identity and + * pool every player into one rate limit. {@code behind-proxy} switches to the last {@code X-Forwarded-For} entry: + * the last is the one the proxy itself appended, so unlike the earlier entries a client cannot forge it. Off by + * default, because trusting the header when the port is reachable directly would let anyone claim any address. + */ + private String clientAddress(HttpExchange exchange) { + if (!config().behindProxy()) { + return exchange.getRemoteAddress().getAddress().getHostAddress(); + } + + List forwarded = exchange.getRequestHeaders().get("X-Forwarded-For"); + if (forwarded != null) { + for (int i = forwarded.size() - 1; i >= 0; i--) { + String[] hops = forwarded.get(i).split(","); + for (int hop = hops.length - 1; hop >= 0; hop--) { + String address = hops[hop].trim(); + if (!address.isEmpty()) { + return address; + } + } + } + } + return exchange.getRemoteAddress().getAddress().getHostAddress(); + } + + private String url(String token) { + String baseUrl = config().url(); + String trimmed = baseUrl.endsWith("/") ? baseUrl.substring(0, baseUrl.length() - 1) : baseUrl; + return trimmed + CONTEXT_PATH + token; + } + + private void delete(Path file) { + try { + Files.deleteIfExists(file); + } catch (IOException e) { + logger.log(Level.WARNING, "Failed to delete expired world download " + file, e); + } + } + + private PluginConfig.World.Download config() { + return configService.current().world().download(); + } + + private static long megabytes(int megabytes) { + return megabytes * 1024L * 1024L; + } + + private static void respondEmpty(HttpExchange exchange, int status) throws IOException { + exchange.sendResponseHeaders(status, -1); + } + + private static ThreadFactory threadFactory() { + AtomicInteger counter = new AtomicInteger(); + return runnable -> { + Thread thread = new Thread(runnable, "BuildSystem-Download-" + counter.incrementAndGet()); + thread.setDaemon(true); + return thread; + }; + } +} diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/S3DownloadDelivery.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/S3DownloadDelivery.java new file mode 100644 index 00000000..391a14ed --- /dev/null +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/S3DownloadDelivery.java @@ -0,0 +1,193 @@ +/* + * Copyright (c) 2018-2026, Thomas Meaney + * Copyright (c) contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package de.eintosti.buildsystem.world.download; + +import de.eintosti.buildsystem.config.ConfigService; +import de.eintosti.buildsystem.config.PluginConfig; +import de.eintosti.buildsystem.world.backup.storage.s3.S3Client; +import java.io.IOException; +import java.net.URI; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; +import java.util.Map; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executor; +import java.util.logging.Level; +import java.util.logging.Logger; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +/** + * Uploads archives to the bucket the backups already use and hands out pre-signed links. + * + *

No port has to be opened, and the uplink carries the world once rather than once per player. The trade is that a + * pre-signed URL is a plain bearer token: unlike a local link it cannot be pinned to one client or rate limited, so + * whoever it is forwarded to can use it until it expires. + * + *

Credentials, bucket and endpoint come from {@code world.backup.storage.s3}, so the two features cannot drift + * apart. + */ +@NullMarked +final class S3DownloadDelivery implements DownloadDelivery { + + /** + * Where uploads live in the bucket, kept apart from the backups so a lifecycle rule can treat them differently. + */ + private static final String KEY_PREFIX = "downloads/"; + + private final S3Client s3; + private final Logger logger; + + /** + * The uploaded objects and when their links die. An object outlives the request that made it, so something has to + * remember it is there; nothing else would ever delete it. + */ + private final Map uploads = new ConcurrentHashMap<>(); + + private S3DownloadDelivery(S3Client s3, Logger logger) { + this.s3 = s3; + this.logger = logger; + } + + /** + * {@return a delivery uploading to the bucket the backups use, or {@code null} if that is not configured} The + * reason is logged: downloads silently doing nothing would be worse than downloads that are visibly off. + * + * @param configService The live configuration + * @param logger The plugin logger + * @param background Where the leftovers of a previous run are cleared, which must not hold up startup + */ + static @Nullable S3DownloadDelivery open(ConfigService configService, Logger logger, Executor background) { + if (!(configService.current().world().backup().storage() instanceof PluginConfig.World.Backup.S3 settings)) { + logger.severe("world.download.storage is 's3' but backups are not stored on S3." + + " Set world.backup.storage.type to 's3', or world.download.storage back to 'local'." + + " World downloads are disabled."); + return null; + } + + String accessKey = settings.resolvedAccessKey(); + String secretKey = settings.resolvedSecretKey(); + if (isBlank(accessKey) || isBlank(secretKey) || isBlank(settings.region()) || isBlank(settings.bucket())) { + logger.severe("World downloads are disabled:" + + " the S3 backup storage is missing its credentials, region or bucket."); + return null; + } + + String url = settings.url(); + S3Client s3 = new S3Client( + accessKey, secretKey, settings.region(), settings.bucket(), isBlank(url) ? null : URI.create(url)); + logger.info("World downloads are served as pre-signed links from bucket '" + settings.bucket() + "'"); + S3DownloadDelivery delivery = new S3DownloadDelivery(s3, logger); + background.execute(delivery::clearLeftovers); + return delivery; + } + + /** + * Deletes every upload left in the bucket by a previous run. Their links did not survive the restart, so the + * objects are unreachable and would otherwise be billed forever. + * + *

Assumes the prefix belongs to this server alone, the same assumption the local delivery makes about its + * downloads directory. + */ + private void clearLeftovers() { + try { + s3.list(KEY_PREFIX).forEach(object -> delete(object.key())); + } catch (IOException e) { + logger.log(Level.WARNING, "Failed to clear leftover world downloads from the bucket", e); + } + } + + @Override + public String publish(Path archive, String fileName, Duration lifetime, ProgressListener progress) + throws IOException { + long total = Files.size(archive); + // The signed URL is the secret, not the key. A directory per upload only keeps two exports of the same world + // from colliding. + String key = KEY_PREFIX + UUID.randomUUID() + "/" + fileName; + + try { + s3.putFile(key, archive, uploaded -> progress.update(uploaded, total)); + } finally { + // Uploaded or aborted, the local copy has no further use. + Files.deleteIfExists(archive); + } + + // Signed only once the bytes are up, so a long upload does not come out of the link's life. + uploads.put(key, System.currentTimeMillis() + lifetime.toMillis()); + return s3.presignedGetUrl(key, lifetime); + } + + /** + * {@return {@link Long#MAX_VALUE}} The bucket has no budget to divide, so nothing is reserved here. The archive is + * still staged on disk first, which {@link WorldDownloadService} bounds by the free space it finds there. + */ + @Override + public long reserve() { + return Long.MAX_VALUE; + } + + @Override + public void release(long reserved) { + // Nothing was reserved. + } + + /** + * {@return true} An upload runs at the speed of the server's uplink, which is slow enough that a player left + * watching a full bar would read it as a hang. + */ + @Override + public boolean reportsPublishProgress() { + return true; + } + + @Override + public void purgeExpired() { + long now = System.currentTimeMillis(); + uploads.entrySet().removeIf(upload -> { + if (now <= upload.getValue()) { + return false; + } + delete(upload.getKey()); + return true; + }); + } + + /** + * Closes the client without deleting what is still live, since that would hold up shutdown for a round trip per + * object. The next start clears them instead. + */ + @Override + public void close() { + uploads.clear(); + s3.close(); + } + + private void delete(String key) { + try { + s3.delete(key); + } catch (IOException e) { + logger.log(Level.WARNING, "Failed to delete the expired world download " + key, e); + } + } + + private static boolean isBlank(@Nullable String value) { + return value == null || value.isBlank(); + } +} diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/WorldDownloadService.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/WorldDownloadService.java index b9a6d707..e2f9ad1f 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/WorldDownloadService.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/WorldDownloadService.java @@ -17,8 +17,6 @@ */ package de.eintosti.buildsystem.world.download; -import com.sun.net.httpserver.HttpExchange; -import com.sun.net.httpserver.HttpServer; import de.eintosti.buildsystem.api.world.BuildWorld; import de.eintosti.buildsystem.config.ConfigService; import de.eintosti.buildsystem.config.PluginConfig; @@ -26,20 +24,15 @@ import de.eintosti.buildsystem.util.TaskScheduler; import java.io.File; import java.io.IOException; -import java.io.OutputStream; import java.io.UncheckedIOException; -import java.net.InetSocketAddress; import java.nio.file.Files; import java.nio.file.Path; import java.time.Duration; import java.util.List; -import java.util.Locale; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.concurrent.Semaphore; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Level; import java.util.logging.Logger; import org.bukkit.Bukkit; @@ -49,35 +42,43 @@ import org.jspecify.annotations.Nullable; /** - * Serves world exports over HTTP so players can download a world as a single-player save. + * Packs a world into a single-player save and hands the player a link to it. * - *

Off by default. When enabled, an archive is only ever reachable through the unguessable, expiring token it was - * registered under: the token is the whole URL path, so no request can name a file, and nothing outside the plugin's - * {@code downloads} directory is served. A link is pinned to the first client that uses it, requests are rate limited - * per address, and concurrent transfers are capped so downloads cannot crowd each other out. Archives are deleted - * together with their token — on expiry, on reload and on shutdown. + *

Off by default, since either way of serving the archive exposes it outside the game. The packing is the same + * whichever way that is; where the archive then goes is the {@link DownloadDelivery}'s business — + * {@link LocalDownloadDelivery the built-in HTTP server} or {@link S3DownloadDelivery a pre-signed S3 link}. Archives + * are cleared on reload and on shutdown, and each is dropped when its link expires. */ @NullMarked public final class WorldDownloadService { - private static final String CONTEXT_PATH = "/download/"; - private static final int MAX_REQUESTS_PER_WINDOW = 30; - private static final long RATE_LIMIT_WINDOW_MILLIS = Duration.ofMinutes(1).toMillis(); private static final long PURGE_INTERVAL_TICKS = Duration.ofMinutes(1).toSeconds() * 20L; + /** + * Free space kept clear of the staging archive. The game server writes its own worlds and player data to the same + * disk, so an export must not be allowed to consume the last byte of it. + */ + private static final long RESERVED_DISK_BYTES = 1024L * 1024 * 1024; + private final ConfigService configService; private final TaskScheduler scheduler; private final Logger logger; private final File downloadFolder; - private final DownloadRegistry registry = new DownloadRegistry(); - private final RequestRateLimiter rateLimiter = - new RequestRateLimiter(MAX_REQUESTS_PER_WINDOW, RATE_LIMIT_WINDOW_MILLIS); + /** + * Guards against a slow purge overlapping the next tick of the timer. + */ + private final AtomicBoolean purging = new AtomicBoolean(); - private @Nullable HttpServer server; - private @Nullable ExecutorService httpExecutor; - private @Nullable Semaphore transferSlots; + private @Nullable DownloadDelivery delivery; private @Nullable BukkitTask purgeTask; + private @Nullable ExecutorService exportExecutor; + + /** + * Bumped by every {@link #stop()}. An export carries the value it started under, so one that outlives a reload is + * discarded instead of publishing to a delivery that is already closed. + */ + private volatile int epoch; public WorldDownloadService(ConfigService configService, TaskScheduler scheduler, Logger logger, File dataFolder) { this.configService = configService; @@ -87,8 +88,8 @@ public WorldDownloadService(ConfigService configService, TaskScheduler scheduler } /** - * Starts the download server if it is enabled in the config. Any archive left behind by a previous run is deleted: - * its token did not survive the restart, so the file is unreachable. + * Starts world downloads if they are enabled in the config. Any archive left behind by a previous run is deleted: + * its link did not survive the restart, so the file is unreachable. */ public void start() { PluginConfig.World.Download config = config(); @@ -102,52 +103,51 @@ public void start() { return; } - int concurrentDownloads = Math.max(1, config.maxConcurrentDownloads()); - ExecutorService executor = Executors.newFixedThreadPool(concurrentDownloads + 1, threadFactory()); - try { - HttpServer httpServer = HttpServer.create(new InetSocketAddress(config.port()), 0); - httpServer.createContext(CONTEXT_PATH, this::handle); - httpServer.setExecutor(executor); - httpServer.start(); - this.server = httpServer; - this.httpExecutor = executor; - this.transferSlots = new Semaphore(concurrentDownloads); - } catch (IOException e) { - executor.shutdownNow(); - logger.log(Level.SEVERE, "Failed to start the world download server on port " + config.port(), e); + this.delivery = switch (config.storage()) { + case LOCAL -> LocalDownloadDelivery.open(configService, logger); + case S3 -> S3DownloadDelivery.open(configService, logger, scheduler.background()); + }; + if (delivery == null) { return; } + // Exports get their own thread rather than the shared background pool: packing a world holds a thread for + // minutes, and on that pool it would starve backups and world saves. One at a time also keeps two exports + // from thrashing the same disk. + this.exportExecutor = Executors.newSingleThreadExecutor(runnable -> { + Thread thread = new Thread(runnable, "BuildSystem-Export"); + thread.setDaemon(true); + return thread; + }); this.purgeTask = scheduler.runTimer(this::purge, PURGE_INTERVAL_TICKS, PURGE_INTERVAL_TICKS); - logger.info("World downloads are available on port " + config.port()); - warnAboutPlaintext(config); } /** - * Stops the download server and deletes every archive it was serving. + * Stops world downloads and drops every archive being served. */ public void stop() { + epoch++; if (purgeTask != null) { purgeTask.cancel(); purgeTask = null; } - if (server != null) { - server.stop(0); - server = null; + if (exportExecutor != null) { + // shutdown(), not shutdownNow(): a queued export that got dropped would leave its future hanging, and the + // player who asked for it stuck behind their own "already preparing" guard forever. Queued exports run + // instead, see the bumped epoch immediately, and fail. + exportExecutor.shutdown(); + exportExecutor = null; } - if (httpExecutor != null) { - httpExecutor.shutdownNow(); - httpExecutor = null; + if (delivery != null) { + delivery.close(); + delivery = null; } - transferSlots = null; - registry.clear(); - rateLimiter.clear(); clearDownloadFolder(); } /** - * Applies a changed config by restarting the server, so toggling downloads off takes effect immediately rather - * than at the next restart. + * Applies a changed config by restarting, so toggling downloads off takes effect immediately rather than at the + * next restart. */ public void reload() { stop(); @@ -155,7 +155,7 @@ public void reload() { } public boolean isEnabled() { - return server != null; + return delivery != null; } public int getExpirationMinutes() { @@ -163,143 +163,120 @@ public int getExpirationMinutes() { } /** - * Exports {@code buildWorld} on a background thread and registers it for download. + * Exports {@code buildWorld} on a background thread and makes it downloadable. * *

Must be called from the main thread: the world's folder and the main level's are resolved through the server * before the export moves off it. * * @param buildWorld The world to export - * @param progress Notified as the world is packed, for showing the player how far along it is + * @param progress Notified as the world is packed and published, for showing the player how far along it is * @return A future completed with the download URL, or completed exceptionally if the export fails */ - public CompletableFuture prepare(BuildWorld buildWorld, WorldExporter.ExportProgress progress) { - if (server == null) { + public CompletableFuture prepare(BuildWorld buildWorld, DownloadProgress progress) { + DownloadDelivery target = delivery; + ExecutorService executor = exportExecutor; + if (target == null || executor == null) { return CompletableFuture.failedFuture(new IllegalStateException("World downloads are disabled")); } - PluginConfig.World.Download config = config(); - long storageBudget = megabytes(config.maxStorageMb()); - if (registry.totalBytes() >= storageBudget) { + // The archive is staged on this disk whichever delivery takes it, so the free space bounds every export - + // including one the delivery itself would not limit. + long usableDisk = downloadFolder.getUsableSpace() - RESERVED_DISK_BYTES; + if (usableDisk <= 0) { + logger.warning("Refusing a world export: less than 1 GB is free on the world download disk."); + return CompletableFuture.failedFuture(new StorageFullException()); + } + + long reserved = target.reserve(); + if (reserved <= 0) { return CompletableFuture.failedFuture(new StorageFullException()); } + long capacity = Math.min(reserved, usableDisk); File worldFolder = FileUtils.worldFolder(buildWorld.getName()); List worlds = Bukkit.getWorlds(); if (worlds.isEmpty()) { + target.release(reserved); return CompletableFuture.failedFuture(new IllegalStateException("No main level is loaded")); } File defaultLevelFolder = worlds.getFirst().getWorldFolder(); String worldName = buildWorld.getName(); - long maxArchiveBytes = Math.min(megabytes(config.maxSizeMb()), storageBudget - registry.totalBytes()); - long expiresAt = System.currentTimeMillis() - + Duration.ofMinutes(config.expirationMinutes()).toMillis(); + String fileName = WorldExporter.fileName(worldName) + ".zip"; Path archive = new File(downloadFolder, worldName.hashCode() + "-" + System.nanoTime() + ".zip").toPath(); + int startedUnder = epoch; return CompletableFuture.supplyAsync( () -> { try { + // Queued behind another export when downloads were stopped: there is nothing to pack for. + if (startedUnder != epoch) { + throw new IOException("World downloads were restarted before the export began"); + } + WorldExporter.export( - worldName, worldFolder, defaultLevelFolder, archive, maxArchiveBytes, progress); + worldName, + worldFolder, + defaultLevelFolder, + archive, + capacity, + (packed, total) -> progress.update(DownloadProgress.Phase.PACKING, packed, total)); + + // Downloads may have been stopped or reloaded while this ran. The delivery captured above is + // closed by now and the staging folder has been wiped, so there is nothing left to publish to. + if (startedUnder != epoch) { + deleteQuietly(archive); + throw new IOException("World downloads were restarted while the export was running"); + } + + if (target.reportsPublishProgress()) { + progress.update(DownloadProgress.Phase.PUBLISHING, 0L, 0L); + } + // A lifetime, not a deadline: the delivery starts the clock when it hands the link out, so + // neither packing nor uploading eats into the window the player was promised. + return target.publish( + archive, + fileName, + Duration.ofMinutes(config().expirationMinutes()), + (published, total) -> + progress.update(DownloadProgress.Phase.PUBLISHING, published, total)); } catch (IOException e) { throw new UncheckedIOException(e); + } finally { + target.release(reserved); } - String token = registry.register(archive, WorldExporter.fileName(worldName) + ".zip", expiresAt); - return url(token); }, - scheduler.background()); + executor); } /** - * Warns when links leave the server unencrypted. A token in a plaintext URL is readable by anything on the path, - * so the only safe plain-HTTP setup is one that terminates TLS in front of this server. + * Hands the purge to a background thread. Dropping an S3 upload is an HTTPS round trip per object, which on the + * main thread would stall the tick for as long as the bucket takes to answer. */ - private void warnAboutPlaintext(PluginConfig.World.Download config) { - if (config.url().toLowerCase(Locale.ROOT).startsWith("https://")) { + private void purge() { + DownloadDelivery target = delivery; + if (target == null || !purging.compareAndSet(false, true)) { return; } - logger.warning("World downloads are served over plain HTTP. Anyone able to observe the traffic can reuse a " - + "download link. Put the port behind a TLS proxy and point world.download.url at it."); - } - - private void handle(HttpExchange exchange) throws IOException { - String clientAddress = clientAddress(exchange); - try (exchange) { - if (!rateLimiter.allow(clientAddress)) { - respondEmpty(exchange, 429); - return; - } - - if (!"GET".equalsIgnoreCase(exchange.getRequestMethod())) { - respondEmpty(exchange, 405); - return; + scheduler.background().execute(() -> { + try { + target.purgeExpired(); + } catch (RuntimeException e) { + logger.log(Level.WARNING, "Failed to purge expired world downloads", e); + } finally { + purging.set(false); } - - DownloadRegistry.Download download = registry.claim(token(exchange), clientAddress); - if (download == null || !Files.isRegularFile(download.file())) { - respondEmpty(exchange, 404); - return; - } - - transfer(exchange, download); - } catch (IOException e) { - // A client that disconnects mid-download is routine and must not spam the console. - logger.log(Level.FINE, "World download aborted", e); - } + }); } - private void transfer(HttpExchange exchange, DownloadRegistry.Download download) throws IOException { - Semaphore slots = transferSlots; - if (slots == null || !slots.tryAcquire()) { - exchange.getResponseHeaders().set("Retry-After", "30"); - respondEmpty(exchange, 503); - return; - } - + private void deleteQuietly(Path file) { try { - exchange.getResponseHeaders().set("Content-Type", "application/zip"); - exchange.getResponseHeaders() - .set("Content-Disposition", "attachment; filename=\"" + download.fileName() + "\""); - exchange.getResponseHeaders().set("X-Content-Type-Options", "nosniff"); - exchange.getResponseHeaders().set("Cache-Control", "no-store"); - exchange.sendResponseHeaders(200, Files.size(download.file())); - - try (OutputStream out = exchange.getResponseBody()) { - Files.copy(download.file(), out); - } - } finally { - slots.release(); + Files.deleteIfExists(file); + } catch (IOException e) { + logger.log(Level.WARNING, "Failed to delete the abandoned world export " + file, e); } } - /** - * {@return the requested token, or the empty string for any request shaped differently} The token is the entire - * path below the context, so a request can neither name a file nor escape the download folder. - */ - private String token(HttpExchange exchange) { - String path = exchange.getRequestURI().getPath(); - return path.length() > CONTEXT_PATH.length() ? path.substring(CONTEXT_PATH.length()) : ""; - } - - private static String clientAddress(HttpExchange exchange) { - return exchange.getRemoteAddress().getAddress().getHostAddress(); - } - - private void purge() { - registry.purgeExpired(this::delete); - rateLimiter.purgeStale(); - } - - private String url(String token) { - String baseUrl = config().url(); - String trimmed = baseUrl.endsWith("/") ? baseUrl.substring(0, baseUrl.length() - 1) : baseUrl; - return trimmed + CONTEXT_PATH + token; - } - - private static long megabytes(int megabytes) { - return megabytes * 1024L * 1024L; - } - private PluginConfig.World.Download config() { return configService.current().world().download(); } @@ -315,27 +292,6 @@ private void clearDownloadFolder() { } } - private void delete(Path file) { - try { - Files.deleteIfExists(file); - } catch (IOException e) { - logger.log(Level.WARNING, "Failed to delete expired world download " + file, e); - } - } - - private static void respondEmpty(HttpExchange exchange, int status) throws IOException { - exchange.sendResponseHeaders(status, -1); - } - - private static ThreadFactory threadFactory() { - AtomicInteger counter = new AtomicInteger(); - return runnable -> { - Thread thread = new Thread(runnable, "BuildSystem-Download-" + counter.incrementAndGet()); - thread.setDaemon(true); - return thread; - }; - } - /** * Thrown when the live archives already fill the configured storage budget. */ diff --git a/buildsystem-core/src/main/resources/config.yml b/buildsystem-core/src/main/resources/config.yml index b5ae91e4..893fd19b 100644 --- a/buildsystem-core/src/main/resources/config.yml +++ b/buildsystem-core/src/main/resources/config.yml @@ -111,16 +111,29 @@ world: path: backups/worlds/ # Lets players download a world as a single-player save via /worlds download. - # Enabling this opens a port on the server that serves world archives, each - # reachable only through an unguessable link that expires with the archive. + # Enabling this hands out world archives, each reachable only through an + # unguessable link that expires with the archive. download: enabled: false + # Where a prepared archive is served from. + # "local" uses the port below. + # "s3" uploads it to the bucket the backups use and hands out an expiring + # pre-signed link instead, so no port has to be opened. It needs + # world.backup.storage.type to be "s3", and puts no limit on world size. + storage: local + # The port the built-in server listens on, and the address players are sent + # to. Change the url if the port is reached through a proxy or a domain, + # e.g. "https://downloads.example.com". Both are ignored when storage is s3. port: 8080 - # The address players are sent to. Change it if the port is reached through - # a proxy or a domain, e.g. "https://downloads.example.com" url: "http://localhost:8080" + # Enable only when the port is reachable solely through a reverse proxy. + # Links are then pinned and rate limited per player rather than per proxy, + # using X-Forwarded-For. Leave it false if the port is reachable directly: + # anyone could otherwise send that header and claim any address. + behind-proxy: false expiration-minutes: 30 - # Largest single export, and the budget shared by all live downloads. + # Largest single export, and the budget shared by all live downloads. Only + # apply to local storage, where every archive sits on this server's disk. max-size-mb: 2048 max-storage-mb: 8192 max-concurrent-downloads: 3 diff --git a/buildsystem-core/src/main/resources/messages.yml b/buildsystem-core/src/main/resources/messages.yml index 3f26ccb3..6a2a287a 100644 --- a/buildsystem-core/src/main/resources/messages.yml +++ b/buildsystem-core/src/main/resources/messages.yml @@ -209,6 +209,7 @@ worlds_download_disabled: "%prefix% &cWorld downloads are disabled on this serve worlds_download_in_progress: "%prefix% &cYou are already preparing a download." worlds_download_preparing: "%prefix% &7Preparing &b%world% &7for download..." worlds_download_progress: "&8[%bar%&8] &f%percent%% &7Packing &b%world% &7%spinner%" +worlds_download_uploading: "&8[%bar%&8] &f%percent%% &7Uploading &b%world% &7%spinner%" worlds_download_failed: "%prefix% &cUnable to prepare %world% for download. See the console for details." worlds_download_too_large: "%prefix% &c%world% is too large to download." worlds_download_storage_full: "%prefix% &cToo many downloads are pending. Try again once one expires." diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/world/backup/storage/s3/AwsV4SignerTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/world/backup/storage/s3/AwsV4SignerTest.java index 55ac5564..8e441173 100644 --- a/buildsystem-core/src/test/java/de/eintosti/buildsystem/world/backup/storage/s3/AwsV4SignerTest.java +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/world/backup/storage/s3/AwsV4SignerTest.java @@ -20,6 +20,7 @@ import static org.junit.jupiter.api.Assertions.*; import java.nio.charset.StandardCharsets; +import java.time.Duration; import java.time.Instant; import java.time.ZoneOffset; import java.time.ZonedDateTime; @@ -59,6 +60,34 @@ void getObjectExampleMatchesPublishedSignature() { signed.get("Authorization")); } + @Test + @DisplayName("The documented pre-signed URL example produces the documented signature") + void presignedUrlExampleMatchesPublishedSignature() { + AwsV4Signer signer = new AwsV4Signer(ACCESS_KEY, SECRET_KEY, "us-east-1"); + + String query = signer.presignGet( + "examplebucket.s3.amazonaws.com", "/test.txt", Duration.ofSeconds(86400), SIGNING_TIME); + + assertEquals( + "X-Amz-Algorithm=AWS4-HMAC-SHA256" + + "&X-Amz-Credential=AKIAIOSFODNN7EXAMPLE%2F20130524%2Fus-east-1%2Fs3%2Faws4_request" + + "&X-Amz-Date=20130524T000000Z" + + "&X-Amz-Expires=86400" + + "&X-Amz-SignedHeaders=host" + + "&X-Amz-Signature=aeeed9bbccd4d02ee5c0109b86d86835f995330da4c265957d157751f604d404", + query); + } + + @Test + @DisplayName("A pre-signed URL cannot outlive the seven days S3 allows") + void presignedExpiryIsCapped() { + AwsV4Signer signer = new AwsV4Signer(ACCESS_KEY, SECRET_KEY, "us-east-1"); + + String query = signer.presignGet("b.s3.amazonaws.com", "/k.zip", Duration.ofDays(30), SIGNING_TIME); + + assertTrue(query.contains("X-Amz-Expires=604800"), query); + } + @Test @DisplayName("An empty payload hashes to the documented constant") void emptyPayloadHash() { diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/world/backup/storage/s3/S3ClientPartSizeTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/world/backup/storage/s3/S3ClientPartSizeTest.java new file mode 100644 index 00000000..018d4390 --- /dev/null +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/world/backup/storage/s3/S3ClientPartSizeTest.java @@ -0,0 +1,74 @@ +/* + * Copyright (c) 2018-2026, Thomas Meaney + * Copyright (c) contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package de.eintosti.buildsystem.world.backup.storage.s3; + +import static org.junit.jupiter.api.Assertions.*; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +/** + * Pins the multipart split, which decides whether a large world can be uploaded at all: too small a part and the + * upload needs more than the ten thousand parts S3 allows, too small a last part and S3 rejects it. + */ +class S3ClientPartSizeTest { + + private static final long MIB = 1024L * 1024L; + private static final long MIN_PART_SIZE = 16 * MIB; + private static final long MAX_PARTS = 10_000L; + + @Test + @DisplayName("A world that fits in the default part count keeps the base part size") + void smallWorldsUseTheBasePartSize() { + assertEquals(MIN_PART_SIZE, S3Client.partSize(0L)); + assertEquals(MIN_PART_SIZE, S3Client.partSize(MIB)); + assertEquals(MIN_PART_SIZE, S3Client.partSize(2048 * MIB)); + assertEquals(MIN_PART_SIZE, S3Client.partSize(MIN_PART_SIZE * MAX_PARTS)); + } + + @Test + @DisplayName("Past 160 GB the part size grows instead of the part count") + void hugeWorldsGrowThePartSize() { + long justOver = MIN_PART_SIZE * MAX_PARTS + 1; + assertTrue(S3Client.partSize(justOver) > MIN_PART_SIZE, "part size must grow once 10000 parts is not enough"); + assertTrue(partCount(justOver) <= MAX_PARTS, "must never need more than 10000 parts"); + } + + @Test + @DisplayName("No world size needs more parts than S3 accepts") + void partCountNeverExceedsTheLimit() { + long[] sizes = { + 1L, MIB, 1024 * MIB, 100L * 1024 * MIB, 1024L * 1024 * MIB, Long.MAX_VALUE / 2, + }; + for (long size : sizes) { + assertTrue(partCount(size) <= MAX_PARTS, "too many parts for a size of " + size); + } + } + + @Test + @DisplayName("Every part but the last clears the five-megabyte minimum S3 enforces") + void partsClearTheServiceMinimum() { + assertTrue(S3Client.partSize(1L) >= 5 * MIB); + assertTrue(S3Client.partSize(Long.MAX_VALUE / 2) >= 5 * MIB); + } + + private static long partCount(long size) { + long partSize = S3Client.partSize(size); + return (size + partSize - 1) / partSize; + } +} From 6bc304f3f3c8fb023751498e5c320685f11bc9dd Mon Sep 17 00:00:00 2001 From: Thomas Meaney Date: Sat, 1 Aug 2026 15:00:13 +0200 Subject: [PATCH 2/2] refactor: move the external storage credentials to the root The S3 and SFTP credentials sat under world.backup.storage only because backups needed them first. World downloads read the same bucket, and a second feature reading a key path named after the first is a lie that only gets more expensive to keep. They move to a root storage section, and each feature names the backend it wants: world.backup.storage and world.download.storage are now plain type names. Where files land within a backend belongs to the feature, not the credentials, so backups keep their prefix in world.backup.path and downloads keep theirs. Downloads no longer require backups to be on S3, since neither feature owns the bucket any more. A backend is only selected once its credentials are present, so a half-configured one falls back to local with the missing key named, rather than being handed out and failing at connection time. Existing configs migrate on first start. --- .../buildsystem/config/ConfigService.java | 43 ++-- .../buildsystem/config/PluginConfig.java | 197 ++++++++++-------- .../config/StorageSettingsFactory.java | 154 ++++++++------ .../migration/ConfigMigrationManager.java | 3 +- .../config/migration/MigrationV4ToV5.java | 79 +++++++ .../world/backup/BackupServiceImpl.java | 59 +++--- .../world/download/S3DownloadDelivery.java | 17 +- .../world/download/WorldDownloadService.java | 5 + .../src/main/resources/config.yml | 55 ++--- .../config/ConfigDefaultsDriftTest.java | 11 +- .../buildsystem/config/PluginConfigTest.java | 112 ++++++---- .../config/migration/MigrationV4ToV5Test.java | 96 +++++++++ 12 files changed, 550 insertions(+), 281 deletions(-) create mode 100644 buildsystem-core/src/main/java/de/eintosti/buildsystem/config/migration/MigrationV4ToV5.java create mode 100644 buildsystem-core/src/test/java/de/eintosti/buildsystem/config/migration/MigrationV4ToV5Test.java diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/ConfigService.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/ConfigService.java index f342c71f..f54b0aa1 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/ConfigService.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/ConfigService.java @@ -101,7 +101,12 @@ public void saveConfig() { * @return The parsed {@link PluginConfig} */ static PluginConfig parse(FileConfiguration config, Logger logger, XMaterial worldEditWand) { - return new PluginConfig(parseSettings(config, worldEditWand), parseWorld(config, logger), parseFolder(config)); + PluginConfig.Storage storage = StorageSettingsFactory.fromConfig(config); + return new PluginConfig( + parseSettings(config, worldEditWand), + storage, + parseWorld(config, storage, logger), + parseFolder(config)); } private static PluginConfig.Settings parseSettings(FileConfiguration config, XMaterial worldEditWand) { @@ -139,7 +144,8 @@ private static PluginConfig.Settings parseSettings(FileConfiguration config, XMa navigator); } - private static PluginConfig.World parseWorld(FileConfiguration config, Logger logger) { + private static PluginConfig.World parseWorld( + FileConfiguration config, PluginConfig.Storage storage, Logger logger) { PluginConfig.World.VoidBlock voidBlock = parseVoidBlock(config, logger); PluginConfig.World.Limits limits = new PluginConfig.World.Limits( @@ -195,13 +201,25 @@ private static PluginConfig.World parseWorld(FileConfiguration config, Logger lo PluginConfig.World.Backup backup = new PluginConfig.World.Backup( Math.min(config.getInt("world.backup.max-backups-per-world", 5), 18), - StorageSettingsFactory.fromConfig(config, logger), + StorageSettingsFactory.typeAt( + config, + "world.backup.storage", + storage, + EnumSet.allOf(PluginConfig.Storage.Type.class), + logger), + Objects.requireNonNullElse(config.getString("world.backup.path"), "backups/worlds/"), autoBackup); int downloadPort = config.getInt("world.download.port", 8080); PluginConfig.World.Download download = new PluginConfig.World.Download( config.getBoolean("world.download.enabled", false), - downloadStorage(config, logger), + // Downloads hand out a link, which SFTP has no way to produce. + StorageSettingsFactory.typeAt( + config, + "world.download.storage", + storage, + EnumSet.of(PluginConfig.Storage.Type.LOCAL, PluginConfig.Storage.Type.S3), + logger), downloadPort, Objects.requireNonNullElse(config.getString("world.download.url"), "http://localhost:" + downloadPort), config.getBoolean("world.download.behind-proxy", false), @@ -229,23 +247,6 @@ private static PluginConfig.World parseWorld(FileConfiguration config, Logger lo download); } - /** - * {@return where prepared world archives are served from} An unrecognised value falls back to the built-in server - * rather than disabling downloads, so a typo does not silently take the feature away. - */ - private static PluginConfig.World.Download.Storage downloadStorage(FileConfiguration config, Logger logger) { - String type = Objects.requireNonNullElse(config.getString("world.download.storage"), "local") - .toLowerCase(Locale.ROOT); - return switch (type) { - case "local" -> PluginConfig.World.Download.Storage.LOCAL; - case "s3" -> PluginConfig.World.Download.Storage.S3; - default -> { - logger.warning("Unknown download storage type '" + type + "', defaulting to local storage."); - yield PluginConfig.World.Download.Storage.LOCAL; - } - }; - } - private static PluginConfig.World.VoidBlock parseVoidBlock(FileConfiguration config, Logger logger) { boolean enabled = config.getBoolean("world.void-block.enabled", true); String raw = Objects.requireNonNullElse(config.getString("world.void-block.material"), "GOLD_BLOCK"); diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/PluginConfig.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/PluginConfig.java index 5afe56a8..271e5273 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/PluginConfig.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/PluginConfig.java @@ -30,7 +30,105 @@ import org.jspecify.annotations.Nullable; @NullMarked -public record PluginConfig(Settings settings, World world, Folder folder) { +public record PluginConfig(Settings settings, Storage storage, World world, Folder folder) { + + /** + * Credentials for the external services the plugin can talk to, defined once at the root rather than inside the + * feature that happened to need them first. Backups and world downloads both select a backend by name and read it + * from here, so a bucket configured for one is the same bucket for the other. + * + * @param s3 The S3 or S3-compatible service + * @param sftp The SFTP server + */ + public record Storage(S3 s3, Sftp sftp) { + + /** + * Which backend a feature stores its files on. + */ + public enum Type { + + /** + * The server's own disk, under the plugin folder. + */ + LOCAL, + + /** + * The bucket configured in {@code storage.s3}. + */ + S3, + + /** + * The server configured in {@code storage.sftp}. + */ + SFTP + } + + public record Sftp( + @Nullable String host, + int port, + @Nullable String username, + @Nullable String password) { + + /** + * {@return the password, preferring {@code BUILDSYSTEM_SFTP_PASSWORD}} Lets operators keep the secret out + * of config.yml. + */ + public @Nullable String resolvedPassword() { + return envOrConfig("BUILDSYSTEM_SFTP_PASSWORD", password); + } + + /** + * Overridden so the password never appears in a log or pasted support output. + */ + @Override + public String toString() { + return "Sftp[host=%s, port=%d, username=%s, password=%s]" + .formatted(host, port, username, password == null ? null : "***"); + } + } + + public record S3( + @Nullable String url, + @Nullable String accessKey, + @Nullable String secretKey, + @Nullable String region, + @Nullable String bucket) { + + /** + * {@return the access key, preferring {@code AWS_ACCESS_KEY_ID}} Lets operators keep the secret out of + * config.yml. + */ + public @Nullable String resolvedAccessKey() { + return envOrConfig("AWS_ACCESS_KEY_ID", accessKey); + } + + /** + * {@return the secret key, preferring {@code AWS_SECRET_ACCESS_KEY}} + */ + public @Nullable String resolvedSecretKey() { + return envOrConfig("AWS_SECRET_ACCESS_KEY", secretKey); + } + + /** + * Overridden so the access key and secret key never appear in a log or pasted support output. + */ + @Override + public String toString() { + return "S3[url=%s, accessKey=%s, secretKey=%s, region=%s, bucket=%s]" + .formatted( + url, + accessKey == null ? null : "***", + secretKey == null ? null : "***", + region, + bucket); + } + } + + private static @Nullable String envOrConfig(String envKey, @Nullable String configValue) { + String env = System.getenv(envKey); + return env == null || env.isBlank() ? configValue : env; + } + } public record Settings( boolean updateChecker, @@ -130,74 +228,13 @@ public record Unload(boolean enabled, String timeUntilUnload, Set blackl } } - public record Backup(int maxBackupsPerWorld, StorageSettings storage, AutoBackup autoBackup) { - - public sealed interface StorageSettings permits Local, Sftp, S3 {} - - public record Local() implements StorageSettings {} - - public record Sftp( - @Nullable String host, - int port, - @Nullable String username, - @Nullable String password, - @Nullable String path) - implements StorageSettings { - - /** - * Overridden so the password never appears in a log or pasted support output. - */ - @Override - public String toString() { - return "Sftp[host=%s, port=%d, username=%s, password=%s, path=%s]" - .formatted(host, port, username, password == null ? null : "***", path); - } - } - - public record S3( - @Nullable String url, - @Nullable String accessKey, - @Nullable String secretKey, - @Nullable String region, - @Nullable String bucket, - @Nullable String path) - implements StorageSettings { - - /** - * {@return the access key, preferring {@code AWS_ACCESS_KEY_ID}} Lets operators keep the secret out of - * config.yml. - */ - public @Nullable String resolvedAccessKey() { - return envOrConfig("AWS_ACCESS_KEY_ID", accessKey); - } - - /** - * {@return the secret key, preferring {@code AWS_SECRET_ACCESS_KEY}} - */ - public @Nullable String resolvedSecretKey() { - return envOrConfig("AWS_SECRET_ACCESS_KEY", secretKey); - } - - private static @Nullable String envOrConfig(String envKey, @Nullable String configValue) { - String env = System.getenv(envKey); - return env == null || env.isBlank() ? configValue : env; - } - - /** - * Overridden so the access key and secret key never appear in a log or pasted support output. - */ - @Override - public String toString() { - return "S3[url=%s, accessKey=%s, secretKey=%s, region=%s, bucket=%s, path=%s]" - .formatted( - url, - accessKey == null ? null : "***", - secretKey == null ? null : "***", - region, - bucket, - path); - } - } + /** + * @param maxBackupsPerWorld How many backups are kept per world + * @param storage Which backend backups are written to, configured under the root {@code storage} section + * @param path Where backups live within that backend + * @param autoBackup The scheduled backup settings + */ + public record Backup(int maxBackupsPerWorld, Storage.Type storage, String path, AutoBackup autoBackup) { public record AutoBackup(boolean enabled, boolean onlyActiveWorlds, int interval) {} } @@ -215,32 +252,14 @@ public record AutoBackup(boolean enabled, boolean onlyActiveWorlds, int interval */ public record Download( boolean enabled, - Storage storage, + Storage.Type storage, int port, String url, boolean behindProxy, int expirationMinutes, int maxSizeMb, int maxStorageMb, - int maxConcurrentDownloads) { - - /** - * Where a prepared archive lives while a player fetches it. - */ - public enum Storage { - - /** - * Served by the plugin's own HTTP server, from the port above. - */ - LOCAL, - - /** - * Uploaded to the bucket the backups already use and handed out as a pre-signed link, so no port has - * to be opened. Requires {@code world.backup.storage.type: s3}. - */ - S3 - } - } + int maxConcurrentDownloads) {} } public record Folder(boolean overridePermissions, boolean overrideProjects) {} diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/StorageSettingsFactory.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/StorageSettingsFactory.java index 9b5ed6c5..54ab3598 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/StorageSettingsFactory.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/StorageSettingsFactory.java @@ -17,97 +17,123 @@ */ package de.eintosti.buildsystem.config; -import de.eintosti.buildsystem.config.PluginConfig.World.Backup.Local; -import de.eintosti.buildsystem.config.PluginConfig.World.Backup.S3; -import de.eintosti.buildsystem.config.PluginConfig.World.Backup.Sftp; -import de.eintosti.buildsystem.config.PluginConfig.World.Backup.StorageSettings; +import de.eintosti.buildsystem.config.PluginConfig.Storage; import java.util.LinkedHashMap; import java.util.Locale; import java.util.Map; +import java.util.Objects; +import java.util.Set; import java.util.logging.Logger; import org.bukkit.configuration.file.FileConfiguration; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; /** - * Builds the backup {@link StorageSettings} from config, validating that a chosen remote backend actually has its - * required credentials before selecting it. A misconfigured remote logs exactly which key is missing and falls back to - * {@link Local local} storage, rather than constructing a remote with {@code null} fields that fails only later at - * connection time. + * Reads the root {@code storage} section and resolves which backend a feature may use. + * + *

A backend is only selected once its credentials are actually present: a misconfigured one logs exactly which key + * is missing and falls back to {@link Storage.Type#LOCAL local} storage, rather than being handed out and failing later + * at connection time. */ @NullMarked final class StorageSettingsFactory { - private static final String BASE = "world.backup.storage."; + private static final String BASE = "storage."; private StorageSettingsFactory() {} - static StorageSettings fromConfig(FileConfiguration config, Logger logger) { - String type = config.getString(BASE + "type", "local").toLowerCase(Locale.ROOT); - return switch (type) { - case "s3" -> s3(config, logger); - case "sftp" -> sftp(config, logger); - case "local" -> new Local(); - default -> { - logger.warning("Unknown backup storage type '" + type + "', defaulting to local storage."); - yield new Local(); - } - }; - } - - private static StorageSettings s3(FileConfiguration config, Logger logger) { - String prefix = BASE + "s3."; - // Only what must come from the config: url is optional (absent means AWS rather than an S3-compatible - // service), and the credentials may instead be supplied through AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY, - // which are resolved after this runs. - Map required = new LinkedHashMap<>(); - required.put(prefix + "region", config.getString(prefix + "region")); - required.put(prefix + "bucket", config.getString(prefix + "bucket")); - - if (fallbackOnMissing(required, "s3", logger)) { - return new Local(); - } - return new S3( - config.getString(prefix + "url"), - config.getString(prefix + "access-key"), - config.getString(prefix + "secret-key"), - config.getString(prefix + "region"), - config.getString(prefix + "bucket"), - config.getString(prefix + "path")); + /** + * {@return the credentials for every backend, whether or not a feature selected it} + */ + static Storage fromConfig(FileConfiguration config) { + String s3 = BASE + "s3."; + String sftp = BASE + "sftp."; + return new Storage( + new Storage.S3( + config.getString(s3 + "url"), + config.getString(s3 + "access-key"), + config.getString(s3 + "secret-key"), + config.getString(s3 + "region"), + config.getString(s3 + "bucket")), + new Storage.Sftp( + config.getString(sftp + "host"), + config.getInt(sftp + "port", 22), + config.getString(sftp + "username"), + config.getString(sftp + "password"))); } - private static StorageSettings sftp(FileConfiguration config, Logger logger) { - String prefix = BASE + "sftp."; - // The password may instead come from BUILDSYSTEM_SFTP_PASSWORD, which is resolved after this runs. - Map required = new LinkedHashMap<>(); - required.put(prefix + "host", config.getString(prefix + "host")); - required.put(prefix + "username", config.getString(prefix + "username")); + /** + * {@return the backend named at {@code path}, or {@link Storage.Type#LOCAL} when it is unknown, unsupported by the + * feature, or missing its credentials} + * + * @param config The raw configuration + * @param path The key naming the backend, e.g. {@code world.backup.storage} + * @param storage The parsed credentials, checked for whichever backend was named + * @param supported The backends this feature can actually use + * @param logger The plugin logger + */ + static Storage.Type typeAt( + FileConfiguration config, String path, Storage storage, Set supported, Logger logger) { + String name = + Objects.requireNonNullElse(config.getString(path), "local").toLowerCase(Locale.ROOT); + Storage.Type type = + switch (name) { + case "local" -> Storage.Type.LOCAL; + case "s3" -> Storage.Type.S3; + case "sftp" -> Storage.Type.SFTP; + default -> { + logger.warning( + "Unknown storage type '" + name + "' at " + path + "; falling back to local storage."); + yield Storage.Type.LOCAL; + } + }; - if (fallbackOnMissing(required, "sftp", logger)) { - return new Local(); + if (type != Storage.Type.LOCAL && !supported.contains(type)) { + logger.warning(path + " does not support '" + name + "' storage; falling back to local storage."); + return Storage.Type.LOCAL; } - - return new Sftp( - config.getString(prefix + "host"), - config.getInt(prefix + "port", 22), - config.getString(prefix + "username"), - config.getString(prefix + "password"), - config.getString(prefix + "path")); + return hasCredentials(type, storage, logger) ? type : Storage.Type.LOCAL; } /** - * {@return whether a required value is missing} Logs the first blank/absent key together with the backend name when - * so, signalling the caller to fall back to local storage. + * {@return whether the backend has everything it needs to connect} Logs the first missing key when it does not. */ - private static boolean fallbackOnMissing(Map required, String backend, Logger logger) { - for (Map.Entry entry : required.entrySet()) { - if (isBlank(entry.getValue())) { - logger.warning("Backup storage '" + backend + "' is missing required setting '" + entry.getKey() - + "'; falling back to local storage."); + private static boolean hasCredentials(Storage.Type type, Storage storage, Logger logger) { + Map required = new LinkedHashMap<>(); + switch (type) { + case LOCAL -> { return true; } + case S3 -> { + // url is optional: absent means AWS rather than an S3-compatible service. The credentials may be + // supplied through AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY instead of the config. + required.put(BASE + "s3.region", storage.s3().region()); + required.put(BASE + "s3.bucket", storage.s3().bucket()); + required.put( + BASE + "s3.access-key (or AWS_ACCESS_KEY_ID)", + storage.s3().resolvedAccessKey()); + required.put( + BASE + "s3.secret-key (or AWS_SECRET_ACCESS_KEY)", + storage.s3().resolvedSecretKey()); + } + case SFTP -> { + // The password may be supplied through BUILDSYSTEM_SFTP_PASSWORD instead of the config. + required.put(BASE + "sftp.host", storage.sftp().host()); + required.put(BASE + "sftp.username", storage.sftp().username()); + required.put( + BASE + "sftp.password (or BUILDSYSTEM_SFTP_PASSWORD)", + storage.sftp().resolvedPassword()); + } + } + + for (Map.Entry entry : required.entrySet()) { + if (isBlank(entry.getValue())) { + logger.warning("Storage '" + type.name().toLowerCase(Locale.ROOT) + "' is missing required setting '" + + entry.getKey() + "'; falling back to local storage."); + return false; + } } - return false; + return true; } private static boolean isBlank(@Nullable String value) { diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/migration/ConfigMigrationManager.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/migration/ConfigMigrationManager.java index f2399f6e..208cada0 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/migration/ConfigMigrationManager.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/migration/ConfigMigrationManager.java @@ -36,7 +36,7 @@ @NullMarked public class ConfigMigrationManager { - public static final int LATEST_VERSION = 4; + public static final int LATEST_VERSION = 5; private final BuildSystemPlugin plugin; private final Map migrations; @@ -53,6 +53,7 @@ public ConfigMigrationManager(BuildSystemPlugin plugin) { registerMigration(1, new MigrationV1ToV2()); registerMigration(2, new MigrationV2ToV3()); registerMigration(3, new MigrationV3ToV4()); + registerMigration(4, new MigrationV4ToV5()); } /** diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/migration/MigrationV4ToV5.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/migration/MigrationV4ToV5.java new file mode 100644 index 00000000..84b7970c --- /dev/null +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/migration/MigrationV4ToV5.java @@ -0,0 +1,79 @@ +/* + * Copyright (c) 2018-2026, Thomas Meaney + * Copyright (c) contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package de.eintosti.buildsystem.config.migration; + +import java.util.List; +import java.util.Locale; +import org.bukkit.configuration.file.FileConfiguration; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +/** + * Migrates config from v4 to v5: the S3 and SFTP credentials move out of {@code world.backup.storage} and up to a root + * {@code storage} section. + * + *

They were only ever nested under backups because backups needed them first. World downloads read the same bucket, + * and a second feature reading a key path named after the first is a lie that only gets more expensive. The backend a + * feature uses is now named by {@code world.backup.storage} directly, and the prefix it writes under moves with the + * feature rather than with the credentials. + */ +@NullMarked +public class MigrationV4ToV5 implements Migration { + + private static final String OLD_BASE = "world.backup.storage"; + + @Override + public void migrate(FileConfiguration config) { + String type = config.getString(OLD_BASE + ".type", "local").toLowerCase(Locale.ROOT); + + move(config, OLD_BASE + ".s3.url", "storage.s3.url"); + move(config, OLD_BASE + ".s3.access-key", "storage.s3.access-key"); + move(config, OLD_BASE + ".s3.secret-key", "storage.s3.secret-key"); + move(config, OLD_BASE + ".s3.region", "storage.s3.region"); + move(config, OLD_BASE + ".s3.bucket", "storage.s3.bucket"); + + move(config, OLD_BASE + ".sftp.host", "storage.sftp.host"); + move(config, OLD_BASE + ".sftp.port", "storage.sftp.port"); + move(config, OLD_BASE + ".sftp.username", "storage.sftp.username"); + move(config, OLD_BASE + ".sftp.password", "storage.sftp.password"); + + // Both backends carried their own path; only the one actually in use describes where backups really are. + String path = config.getString(OLD_BASE + "." + type + ".path"); + if (path != null && !path.isBlank()) { + config.set("world.backup.path", path); + } + + // Replacing the section with a scalar of the same name: the old subtree has to go first, or the type would be + // written as a child of the very section it replaces. + config.set(OLD_BASE, null); + config.set(OLD_BASE, type); + config.setComments( + OLD_BASE, List.of("Where backups are written: local, s3 or sftp.", "Credentials live under storage.")); + } + + /** + * Copies a value to its new key and drops the old one, leaving an absent value absent rather than writing a null. + */ + private static void move(FileConfiguration config, String from, String to) { + @Nullable Object value = config.get(from); + if (value != null) { + config.set(to, value); + } + config.set(from, null); + } +} diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupServiceImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupServiceImpl.java index 9886c01d..9f7d0c03 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupServiceImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupServiceImpl.java @@ -101,21 +101,20 @@ public BackupServiceImpl( * an operator can tell at a glance whether the configured backend was actually the one that loaded. */ private String describe(BackupStorage storage) { - PluginConfig.World.Backup.StorageSettings settings = - configService.current().world().backup().storage(); + PluginConfig current = configService.current(); + PluginConfig.Storage.Type configured = current.world().backup().storage(); switch (storage) { case LocalBackupStorage _ -> { - String reason = - settings instanceof PluginConfig.World.Backup.Local ? "" : " (configured backend failed)"; + String reason = configured == PluginConfig.Storage.Type.LOCAL ? "" : " (configured backend failed)"; return "locally in plugins/BuildSystem/backups" + reason; } - case S3BackupStorage _ - when settings instanceof PluginConfig.World.Backup.S3 s3 -> { + case S3BackupStorage _ -> { + PluginConfig.Storage.S3 s3 = current.storage().s3(); String service = s3.url() == null || s3.url().isBlank() ? "Amazon S3" : s3.url(); return "on " + service + " in bucket '" + s3.bucket() + "'"; } - case SftpBackupStorage _ - when settings instanceof PluginConfig.World.Backup.Sftp sftp -> { + case SftpBackupStorage _ -> { + PluginConfig.Storage.Sftp sftp = current.storage().sftp(); return "over SFTP on " + sftp.host() + ":" + sftp.port(); } default -> {} @@ -123,33 +122,37 @@ private String describe(BackupStorage storage) { return "using " + storage.getClass().getSimpleName(); } - private BackupStorage createStorage(PluginConfig.World.Backup.StorageSettings settings) { - return switch (settings) { - case PluginConfig.World.Backup.Local ignored -> localStorage(); - case PluginConfig.World.Backup.Sftp s -> { - String password = envOrConfig("BUILDSYSTEM_SFTP_PASSWORD", s.password()); - requireNonBlank(s.host(), "backup.sftp.host"); - requireNonBlank(s.username(), "backup.sftp.username"); - requireNonBlank(password, "backup.sftp.password (or BUILDSYSTEM_SFTP_PASSWORD)"); + private BackupStorage createStorage(PluginConfig.Storage.Type type) { + PluginConfig current = configService.current(); + String path = current.world().backup().path(); + return switch (type) { + case LOCAL -> localStorage(); + case SFTP -> { + PluginConfig.Storage.Sftp sftp = current.storage().sftp(); + String password = sftp.resolvedPassword(); + requireNonBlank(sftp.host(), "storage.sftp.host"); + requireNonBlank(sftp.username(), "storage.sftp.username"); + requireNonBlank(password, "storage.sftp.password (or BUILDSYSTEM_SFTP_PASSWORD)"); yield new SftpBackupStorage( plugin.getLogger(), executor, plugin.getDataFolder(), configService, this::getProfile, - s.host(), - s.port(), - s.username(), + sftp.host(), + sftp.port(), + sftp.username(), password, - s.path()); + path); } - case PluginConfig.World.Backup.S3 s3 -> { + case S3 -> { + PluginConfig.Storage.S3 s3 = current.storage().s3(); String accessKey = s3.resolvedAccessKey(); String secretKey = s3.resolvedSecretKey(); - requireNonBlank(accessKey, "backup.s3.access-key (or AWS_ACCESS_KEY_ID)"); - requireNonBlank(secretKey, "backup.s3.secret-key (or AWS_SECRET_ACCESS_KEY)"); - requireNonBlank(s3.region(), "backup.s3.region"); - requireNonBlank(s3.bucket(), "backup.s3.bucket"); + requireNonBlank(accessKey, "storage.s3.access-key (or AWS_ACCESS_KEY_ID)"); + requireNonBlank(secretKey, "storage.s3.secret-key (or AWS_SECRET_ACCESS_KEY)"); + requireNonBlank(s3.region(), "storage.s3.region"); + requireNonBlank(s3.bucket(), "storage.s3.bucket"); yield new S3BackupStorage( plugin.getLogger(), executor, @@ -161,14 +164,14 @@ yield new S3BackupStorage( secretKey, s3.region(), s3.bucket(), - s3.path()); + path); } }; } - private BackupStorage createStorageOrFallback(PluginConfig.World.Backup.StorageSettings settings) { + private BackupStorage createStorageOrFallback(PluginConfig.Storage.Type type) { try { - return createStorage(settings); + return createStorage(type); } catch (IllegalArgumentException e) { plugin.getLogger().severe("Backup storage disabled, falling back to local storage: " + e.getMessage()); return localStorage(); diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/S3DownloadDelivery.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/S3DownloadDelivery.java index 391a14ed..1d90ceb0 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/S3DownloadDelivery.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/S3DownloadDelivery.java @@ -35,14 +35,14 @@ import org.jspecify.annotations.Nullable; /** - * Uploads archives to the bucket the backups already use and hands out pre-signed links. + * Uploads archives to the configured bucket and hands out pre-signed links. * *

No port has to be opened, and the uplink carries the world once rather than once per player. The trade is that a * pre-signed URL is a plain bearer token: unlike a local link it cannot be pinned to one client or rate limited, so * whoever it is forwarded to can use it until it expires. * - *

Credentials, bucket and endpoint come from {@code world.backup.storage.s3}, so the two features cannot drift - * apart. + *

Credentials, bucket and endpoint come from the root {@code storage.s3} section, so downloads and backups share + * one definition without either owning it. Backups do not have to be on S3 for this to work. */ @NullMarked final class S3DownloadDelivery implements DownloadDelivery { @@ -75,18 +75,13 @@ private S3DownloadDelivery(S3Client s3, Logger logger) { * @param background Where the leftovers of a previous run are cleared, which must not hold up startup */ static @Nullable S3DownloadDelivery open(ConfigService configService, Logger logger, Executor background) { - if (!(configService.current().world().backup().storage() instanceof PluginConfig.World.Backup.S3 settings)) { - logger.severe("world.download.storage is 's3' but backups are not stored on S3." - + " Set world.backup.storage.type to 's3', or world.download.storage back to 'local'." - + " World downloads are disabled."); - return null; - } - + PluginConfig.Storage.S3 settings = configService.current().storage().s3(); String accessKey = settings.resolvedAccessKey(); String secretKey = settings.resolvedSecretKey(); if (isBlank(accessKey) || isBlank(secretKey) || isBlank(settings.region()) || isBlank(settings.bucket())) { logger.severe("World downloads are disabled:" - + " the S3 backup storage is missing its credentials, region or bucket."); + + " world.download.storage is 's3' but the storage.s3 section is missing its credentials," + + " region or bucket."); return null; } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/WorldDownloadService.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/WorldDownloadService.java index e2f9ad1f..31d8b529 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/WorldDownloadService.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/download/WorldDownloadService.java @@ -106,6 +106,11 @@ public void start() { this.delivery = switch (config.storage()) { case LOCAL -> LocalDownloadDelivery.open(configService, logger); case S3 -> S3DownloadDelivery.open(configService, logger, scheduler.background()); + case SFTP -> { + // Unreachable: the config parser downgrades a backend the feature cannot use to local. + logger.severe("World downloads cannot be served over SFTP; downloads are disabled."); + yield null; + } }; if (delivery == null) { return; diff --git a/buildsystem-core/src/main/resources/config.yml b/buildsystem-core/src/main/resources/config.yml index 893fd19b..6a1427cc 100644 --- a/buildsystem-core/src/main/resources/config.yml +++ b/buildsystem-core/src/main/resources/config.yml @@ -87,28 +87,11 @@ world: enabled: true interval: 900 only-active-worlds: true - storage: - # Options: local, s3, sftp - type: local - s3: - # Leave empty for Amazon S3. Set it to point at an S3-compatible service instead, - # e.g. MinIO, Backblaze B2 or Cloudflare R2: "https://s3.example.com" - url: null - # Can also be supplied as AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY environment - # variables, which take precedence and keep the secrets out of this file. - access-key: YOUR_ACCESS_KEY - secret-key: YOUR_SECRET_KEY - region: eu-central-1 - bucket: buildsystem-backups - path: backups/worlds/ - sftp: - host: YOUR_SFTP_HOST - port: 22 - username: YOUR_SFTP_USERNAME - # Can also be supplied as the BUILDSYSTEM_SFTP_PASSWORD environment variable, - # which takes precedence and keeps the secret out of this file. - password: YOUR_SFTP_PASSWORD - path: backups/worlds/ + # Where backups are written: local, s3 or sftp. Credentials live under the + # root "storage" section, so the same bucket serves every feature using it. + storage: local + # Where backups live within that backend. Ignored by local storage. + path: backups/worlds/ # Lets players download a world as a single-player save via /worlds download. # Enabling this hands out world archives, each reachable only through an @@ -117,9 +100,9 @@ world: enabled: false # Where a prepared archive is served from. # "local" uses the port below. - # "s3" uploads it to the bucket the backups use and hands out an expiring - # pre-signed link instead, so no port has to be opened. It needs - # world.backup.storage.type to be "s3", and puts no limit on world size. + # "s3" uploads it to the bucket under the root "storage" section and hands + # out an expiring pre-signed link instead, so no port has to be opened and + # no limit applies to world size. Backups need not be on S3 for this. storage: local # The port the built-in server listens on, and the address players are sent # to. Change the url if the port is reached through a proxy or a domain, @@ -138,6 +121,28 @@ world: max-storage-mb: 8192 max-concurrent-downloads: 3 +# Credentials for the external services BuildSystem can use. Defined once here +# and selected by name where a feature needs one, e.g. world.backup.storage or +# world.download.storage. A section only matters once something selects it. +storage: + s3: + # Leave empty for Amazon S3. Set it to point at an S3-compatible service instead, + # e.g. MinIO, Backblaze B2 or Cloudflare R2: "https://s3.example.com" + url: null + # Can also be supplied as AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY environment + # variables, which take precedence and keep the secrets out of this file. + access-key: YOUR_ACCESS_KEY + secret-key: YOUR_SECRET_KEY + region: eu-central-1 + bucket: buildsystem-backups + sftp: + host: YOUR_SFTP_HOST + port: 22 + username: YOUR_SFTP_USERNAME + # Can also be supplied as the BUILDSYSTEM_SFTP_PASSWORD environment variable, + # which takes precedence and keeps the secret out of this file. + password: YOUR_SFTP_PASSWORD + folder: override-permissions: true override-projects: false diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/ConfigDefaultsDriftTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/ConfigDefaultsDriftTest.java index dae760ac..f8f5860d 100644 --- a/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/ConfigDefaultsDriftTest.java +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/ConfigDefaultsDriftTest.java @@ -61,7 +61,16 @@ class ConfigDefaultsDriftTest { "world.deletionBlacklist", "world.unload.blacklistedWorlds", "settings.worldPermissionWhitelist", - "world.defaults.gameRules"); + "world.defaults.gameRules", + // Placeholders showing the shape of a credential, not values anything should fall back to. A server that + // never configures a backend must see null here rather than inherit "YOUR_ACCESS_KEY". + "storage.s3.accessKey", + "storage.s3.secretKey", + "storage.s3.region", + "storage.s3.bucket", + "storage.sftp.host", + "storage.sftp.username", + "storage.sftp.password"); @Test @DisplayName("Every parser default matches the value shipped in config.yml") diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/PluginConfigTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/PluginConfigTest.java index 1d5994c8..3b63afc0 100644 --- a/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/PluginConfigTest.java +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/PluginConfigTest.java @@ -71,8 +71,7 @@ void defaults_emptyConfig_producesDocumentedDefaults() { assertEquals("01:00:00", cfg.world().unload().timeUntilUnload()); // World - Backup assertEquals(5, cfg.world().backup().maxBackupsPerWorld()); - assertInstanceOf( - PluginConfig.World.Backup.Local.class, cfg.world().backup().storage()); + assertEquals(PluginConfig.Storage.Type.LOCAL, cfg.world().backup().storage()); assertTrue(cfg.world().backup().autoBackup().enabled()); assertEquals(900, cfg.world().backup().autoBackup().interval()); assertTrue(cfg.world().backup().autoBackup().onlyActiveWorlds()); @@ -276,27 +275,25 @@ void backupStorage_s3Type_returnsS3Record() { enabled: true interval: 900 only-active-worlds: true - storage: - type: s3 - s3: - url: "https://example.com" - access-key: "MYACCESSKEY" - secret-key: "MYSECRETKEY" - region: "eu-central-1" - bucket: "my-bucket" - path: "backups/" + storage: s3 + path: "backups/" + storage: + s3: + url: "https://example.com" + access-key: "MYACCESSKEY" + secret-key: "MYSECRETKEY" + region: "eu-central-1" + bucket: "my-bucket" """); - assertInstanceOf( - PluginConfig.World.Backup.S3.class, cfg.world().backup().storage()); - PluginConfig.World.Backup.S3 s3 = - (PluginConfig.World.Backup.S3) cfg.world().backup().storage(); + assertEquals(PluginConfig.Storage.Type.S3, cfg.world().backup().storage()); + assertEquals("backups/", cfg.world().backup().path()); + PluginConfig.Storage.S3 s3 = cfg.storage().s3(); assertEquals("https://example.com", s3.url()); assertEquals("MYACCESSKEY", s3.accessKey()); assertEquals("MYSECRETKEY", s3.secretKey()); assertEquals("eu-central-1", s3.region()); assertEquals("my-bucket", s3.bucket()); - assertEquals("backups/", s3.path()); } // ----------------------------------------------------------------------- @@ -315,25 +312,23 @@ void backupStorage_sftpType_returnsSftpRecord() { enabled: true interval: 900 only-active-worlds: true - storage: - type: sftp - sftp: - host: "sftp.example.com" - port: 22 - username: "user" - password: "pass" - path: "/backups/" + storage: sftp + path: "/backups/" + storage: + sftp: + host: "sftp.example.com" + port: 22 + username: "user" + password: "pass" """); - assertInstanceOf( - PluginConfig.World.Backup.Sftp.class, cfg.world().backup().storage()); - PluginConfig.World.Backup.Sftp sftp = - (PluginConfig.World.Backup.Sftp) cfg.world().backup().storage(); + assertEquals(PluginConfig.Storage.Type.SFTP, cfg.world().backup().storage()); + assertEquals("/backups/", cfg.world().backup().path()); + PluginConfig.Storage.Sftp sftp = cfg.storage().sftp(); assertEquals("sftp.example.com", sftp.host()); assertEquals(22, sftp.port()); assertEquals("user", sftp.username()); assertEquals("pass", sftp.password()); - assertEquals("/backups/", sftp.path()); } @Test @@ -343,16 +338,15 @@ void backupStorage_s3MissingRequiredKey_fallsBackToLocal() { PluginConfig cfg = parse(""" world: backup: - storage: - type: s3 - s3: - url: "https://example.com" - access-key: "MYACCESSKEY" - secret-key: "MYSECRETKEY" + storage: s3 + storage: + s3: + url: "https://example.com" + access-key: "MYACCESSKEY" + secret-key: "MYSECRETKEY" """); - assertInstanceOf( - PluginConfig.World.Backup.Local.class, cfg.world().backup().storage()); + assertEquals(PluginConfig.Storage.Type.LOCAL, cfg.world().backup().storage()); } // ----------------------------------------------------------------------- @@ -371,12 +365,48 @@ void backupStorage_unknownType_defaultsToLocal() { enabled: true interval: 900 only-active-worlds: true - storage: - type: unknown_type + storage: unknown_type + """); + + assertEquals(PluginConfig.Storage.Type.LOCAL, cfg.world().backup().storage()); + } + + @Test + void downloadStorage_sftp_fallsBackToLocal() { + // SFTP cannot hand out a link, so downloads must refuse it rather than start a delivery that cannot work. + PluginConfig cfg = parse(""" + world: + download: + storage: sftp + storage: + sftp: + host: "sftp.example.com" + username: "user" + password: "pass" + """); + + assertEquals(PluginConfig.Storage.Type.LOCAL, cfg.world().download().storage()); + } + + @Test + void downloadStorage_s3_doesNotRequireBackupsOnS3() { + // The credentials are shared, not owned by backups: downloads on S3 must work while backups stay local. + PluginConfig cfg = parse(""" + world: + backup: + storage: local + download: + storage: s3 + storage: + s3: + access-key: "MYACCESSKEY" + secret-key: "MYSECRETKEY" + region: "eu-central-1" + bucket: "my-bucket" """); - assertInstanceOf( - PluginConfig.World.Backup.Local.class, cfg.world().backup().storage()); + assertEquals(PluginConfig.Storage.Type.S3, cfg.world().download().storage()); + assertEquals(PluginConfig.Storage.Type.LOCAL, cfg.world().backup().storage()); } // ----------------------------------------------------------------------- diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/migration/MigrationV4ToV5Test.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/migration/MigrationV4ToV5Test.java new file mode 100644 index 00000000..8260ed5c --- /dev/null +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/config/migration/MigrationV4ToV5Test.java @@ -0,0 +1,96 @@ +/* + * Copyright (c) 2018-2026, Thomas Meaney + * Copyright (c) contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package de.eintosti.buildsystem.config.migration; + +import static org.junit.jupiter.api.Assertions.*; + +import org.bukkit.configuration.file.YamlConfiguration; +import org.junit.jupiter.api.Test; + +class MigrationV4ToV5Test { + + @Test + void migrate_s3Backend_movesCredentialsToRootAndKeepsItsPath() { + YamlConfiguration config = new YamlConfiguration(); + config.set("world.backup.storage.type", "s3"); + config.set("world.backup.storage.s3.url", "https://s3.example.com"); + config.set("world.backup.storage.s3.access-key", "KEY"); + config.set("world.backup.storage.s3.secret-key", "SECRET"); + config.set("world.backup.storage.s3.region", "eu-central-1"); + config.set("world.backup.storage.s3.bucket", "my-bucket"); + config.set("world.backup.storage.s3.path", "backups/worlds/"); + + new MigrationV4ToV5().migrate(config); + + assertEquals("https://s3.example.com", config.getString("storage.s3.url")); + assertEquals("KEY", config.getString("storage.s3.access-key")); + assertEquals("SECRET", config.getString("storage.s3.secret-key")); + assertEquals("eu-central-1", config.getString("storage.s3.region")); + assertEquals("my-bucket", config.getString("storage.s3.bucket")); + assertEquals("backups/worlds/", config.getString("world.backup.path")); + + // The section is gone and its name now holds the backend, not a subtree. + assertEquals("s3", config.getString("world.backup.storage")); + assertNull(config.get("world.backup.storage.s3")); + assertNull(config.get("world.backup.storage.type")); + } + + @Test + void migrate_sftpBackend_movesCredentialsAndTakesTheSftpPath() { + YamlConfiguration config = new YamlConfiguration(); + config.set("world.backup.storage.type", "sftp"); + config.set("world.backup.storage.sftp.host", "sftp.example.com"); + config.set("world.backup.storage.sftp.port", 2222); + config.set("world.backup.storage.sftp.username", "user"); + config.set("world.backup.storage.sftp.password", "pass"); + config.set("world.backup.storage.sftp.path", "/srv/backups/"); + // A leftover S3 path from an earlier experiment must not win over the backend actually in use. + config.set("world.backup.storage.s3.path", "wrong/"); + + new MigrationV4ToV5().migrate(config); + + assertEquals("sftp.example.com", config.getString("storage.sftp.host")); + assertEquals(2222, config.getInt("storage.sftp.port")); + assertEquals("user", config.getString("storage.sftp.username")); + assertEquals("pass", config.getString("storage.sftp.password")); + assertEquals("/srv/backups/", config.getString("world.backup.path")); + assertEquals("sftp", config.getString("world.backup.storage")); + } + + @Test + void migrate_localBackend_carriesNoPathOver() { + YamlConfiguration config = new YamlConfiguration(); + config.set("world.backup.storage.type", "local"); + config.set("world.backup.storage.s3.path", "unused/"); + + new MigrationV4ToV5().migrate(config); + + assertEquals("local", config.getString("world.backup.storage")); + assertNull(config.get("world.backup.path")); + } + + @Test + void migrate_missingSection_defaultsToLocalAndWritesNoCredentials() { + YamlConfiguration config = new YamlConfiguration(); + + new MigrationV4ToV5().migrate(config); + + assertEquals("local", config.getString("world.backup.storage")); + assertNull(config.get("storage")); + } +}