From ef861819e336b0466e4f5138d597223c3de8c5e4 Mon Sep 17 00:00:00 2001 From: Adrian <68704415+4drian3d@users.noreply.github.com> Date: Sun, 11 Feb 2024 12:00:07 -0500 Subject: [PATCH] Do not apply a resource pack that has already been applied (#1236) * Do not apply a resource pack that has already been applied * Throw IllegalStateException in case of applying a resource pack already applied from the API * Updated some dependencies * Added support for ServerResourcePackSendEvent resource pack modification --- .../api/proxy/player/ResourcePackInfo.java | 3 +- gradle/libs.versions.toml | 20 ++++++------ .../backend/BackendPlaySessionHandler.java | 29 ++++++++++++----- .../backend/ConfigSessionHandler.java | 31 +++++++++++++------ .../connection/client/ConnectedPlayer.java | 1 + .../LegacyResourcePackHandler.java | 7 +++++ .../ModernResourcePackHandler.java | 14 +++++++++ .../resourcepack/ResourcePackHandler.java | 18 ++++++++++- settings.gradle.kts | 2 +- 9 files changed, 93 insertions(+), 32 deletions(-) diff --git a/api/src/main/java/com/velocitypowered/api/proxy/player/ResourcePackInfo.java b/api/src/main/java/com/velocitypowered/api/proxy/player/ResourcePackInfo.java index 8f4b0d2ef..82937685a 100644 --- a/api/src/main/java/com/velocitypowered/api/proxy/player/ResourcePackInfo.java +++ b/api/src/main/java/com/velocitypowered/api/proxy/player/ResourcePackInfo.java @@ -54,8 +54,7 @@ public interface ResourcePackInfo extends ResourcePackRequestLike { * * @return the hash if present or null otherwise */ - @Nullable - byte[] getHash(); + byte @Nullable [] getHash(); /** * Gets the {@link Origin} of this resource-pack. diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index d9ec6bc12..4cda1ea76 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -2,26 +2,26 @@ configurate3 = "3.7.3" configurate4 = "4.1.2" flare = "2.0.1" -log4j = "2.20.0" +log4j = "2.22.1" netty = "4.1.106.Final" [plugins] indra-publishing = "net.kyori.indra.publishing:2.0.6" -shadow = "com.github.johnrengelman.shadow:8.1.0" +shadow = "com.github.johnrengelman.shadow:8.1.1" spotless = "com.diffplug.spotless:6.12.0" [libraries] adventure-bom = "net.kyori:adventure-bom:4.15.0" -adventure-facet = "net.kyori:adventure-platform-facet:4.3.0" -asm = "org.ow2.asm:asm:9.5" +adventure-facet = "net.kyori:adventure-platform-facet:4.3.2" +asm = "org.ow2.asm:asm:9.6" auto-service = "com.google.auto.service:auto-service:1.0.1" auto-service-annotations = "com.google.auto.service:auto-service-annotations:1.0.1" brigadier = "com.velocitypowered:velocity-brigadier:1.0.0-SNAPSHOT" -bstats = "org.bstats:bstats-base:3.0.1" +bstats = "org.bstats:bstats-base:3.0.2" caffeine = "com.github.ben-manes.caffeine:caffeine:3.1.5" -checker-qual = "org.checkerframework:checker-qual:3.28.0" +checker-qual = "org.checkerframework:checker-qual:3.42.0" checkstyle = "com.puppycrawl.tools:checkstyle:10.9.3" -completablefutures = "com.spotify:completable-futures:0.3.5" +completablefutures = "com.spotify:completable-futures:0.3.6" configurate3-hocon = { module = "org.spongepowered:configurate-hocon", version.ref = "configurate3" } configurate3-yaml = { module = "org.spongepowered:configurate-yaml", version.ref = "configurate3" } configurate3-gson = { module = "org.spongepowered:configurate-gson", version.ref = "configurate3" } @@ -34,7 +34,7 @@ flare-core = { module = "space.vectrix.flare:flare", version.ref = "flare" } flare-fastutil = { module = "space.vectrix.flare:flare-fastutil", version.ref = "flare" } jline = "org.jline:jline-terminal-jansi:3.23.0" jopt = "net.sf.jopt-simple:jopt-simple:5.0.4" -junit = "org.junit.jupiter:junit-jupiter:5.9.0" +junit = "org.junit.jupiter:junit-jupiter:5.10.2" jspecify = "org.jspecify:jspecify:0.3.0" kyori-ansi = "net.kyori:ansi:1.0.3" guava = "com.google.guava:guava:25.1-jre" @@ -46,7 +46,7 @@ log4j-core = { module = "org.apache.logging.log4j:log4j-core", version.ref = "lo log4j-slf4j-impl = { module = "org.apache.logging.log4j:log4j-slf4j2-impl", version.ref = "log4j" } log4j-iostreams = { module = "org.apache.logging.log4j:log4j-iostreams", version.ref = "log4j" } log4j-jul = { module = "org.apache.logging.log4j:log4j-jul", version.ref = "log4j" } -mockito = "org.mockito:mockito-core:5.2.0" +mockito = "org.mockito:mockito-core:5.10.0" netty-codec = { module = "io.netty:netty-codec", version.ref = "netty" } netty-codec-haproxy = { module = "io.netty:netty-codec-haproxy", version.ref = "netty" } netty-codec-http = { module = "io.netty:netty-codec-http", version.ref = "netty" } @@ -54,7 +54,7 @@ netty-handler = { module = "io.netty:netty-handler", version.ref = "netty" } netty-transport-native-epoll = { module = "io.netty:netty-transport-native-epoll", version.ref = "netty" } netty-transport-native-kqueue = { module = "io.netty:netty-transport-native-kqueue", version.ref = "netty" } nightconfig = "com.electronwill.night-config:toml:3.6.7" -slf4j = "org.slf4j:slf4j-api:2.0.7" +slf4j = "org.slf4j:slf4j-api:2.0.12" snakeyaml = "org.yaml:snakeyaml:1.33" spotbugs-annotations = "com.github.spotbugs:spotbugs-annotations:4.7.3" terminalconsoleappender = "net.minecrell:terminalconsoleappender:1.3.0" diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java index 8724cbad5..30a0340f1 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java @@ -174,36 +174,49 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler { } @Override - public boolean handle(ResourcePackRequestPacket packet) { - ResourcePackInfo.Builder builder = new VelocityResourcePackInfo.BuilderImpl( + public boolean handle(final ResourcePackRequestPacket packet) { + final ResourcePackInfo.Builder builder = new VelocityResourcePackInfo.BuilderImpl( Preconditions.checkNotNull(packet.getUrl())) .setId(packet.getId()) .setPrompt(packet.getPrompt() == null ? null : packet.getPrompt().getComponent()) .setShouldForce(packet.isRequired()) .setOrigin(ResourcePackInfo.Origin.DOWNSTREAM_SERVER); - String hash = packet.getHash(); + final String hash = packet.getHash(); if (hash != null && !hash.isEmpty()) { if (PLAUSIBLE_SHA1_HASH.matcher(hash).matches()) { builder.setHash(ByteBufUtil.decodeHexDump(hash)); } } - ServerResourcePackSendEvent event = new ServerResourcePackSendEvent( - builder.build(), this.serverConn); - + final ResourcePackInfo resourcePackInfo = builder.build(); + final ServerResourcePackSendEvent event = new ServerResourcePackSendEvent( + resourcePackInfo, this.serverConn); server.getEventManager().fire(event).thenAcceptAsync(serverResourcePackSendEvent -> { if (playerConnection.isClosed()) { return; } if (serverResourcePackSendEvent.getResult().isAllowed()) { final ResourcePackInfo toSend = serverResourcePackSendEvent.getProvidedResourcePack(); + boolean modifiedPack = false; if (toSend != serverResourcePackSendEvent.getReceivedResourcePack()) { ((VelocityResourcePackInfo) toSend) .setOriginalOrigin(ResourcePackInfo.Origin.DOWNSTREAM_SERVER); + modifiedPack = true; + } + if (serverConn.getPlayer().resourcePackHandler().hasPackAppliedByHash(toSend.getHash())) { + // Do not apply a resource pack that has already been applied + if (serverConn.getConnection() != null) { + serverConn.getConnection().write(new ResourcePackResponsePacket( + packet.getId(), packet.getHash(), PlayerResourcePackStatusEvent.Status.ACCEPTED)); + } + if (modifiedPack) { + logger.warn("A plugin has tried to modify a ResourcePack provided by the backend server " + + "with a ResourcePack already applied, the applying of the resource pack will be skipped."); + } + } else { + serverConn.getPlayer().resourcePackHandler().queueResourcePack(toSend); } - - serverConn.getPlayer().resourcePackHandler().queueResourcePack(toSend); } else if (serverConn.getConnection() != null) { serverConn.getConnection().write(new ResourcePackResponsePacket( packet.getId(), diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/ConfigSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/ConfigSessionHandler.java index 421063ade..1617fb138 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/ConfigSessionHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/ConfigSessionHandler.java @@ -45,7 +45,6 @@ import com.velocitypowered.proxy.protocol.packet.config.TagsUpdatePacket; import com.velocitypowered.proxy.protocol.util.PluginMessageUtil; import java.io.IOException; import java.util.concurrent.CompletableFuture; -import java.util.regex.Pattern; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -54,8 +53,6 @@ import org.apache.logging.log4j.Logger; * 1.20.2+ switching. Yes, some of this is exceptionally stupid. */ public class ConfigSessionHandler implements MinecraftSessionHandler { - - private static final Pattern PLAUSIBLE_SHA1_HASH = Pattern.compile("^[a-z0-9]{40}$"); private static final Logger logger = LogManager.getLogger(ConfigSessionHandler.class); private final VelocityServer server; private final VelocityServerConnection serverConn; @@ -118,25 +115,39 @@ public class ConfigSessionHandler implements MinecraftSessionHandler { } @Override - public boolean handle(ResourcePackRequestPacket packet) { + public boolean handle(final ResourcePackRequestPacket packet) { final MinecraftConnection playerConnection = serverConn.getPlayer().getConnection(); - ServerResourcePackSendEvent event = - new ServerResourcePackSendEvent(packet.toServerPromptedPack(), this.serverConn); + final ResourcePackInfo resourcePackInfo = packet.toServerPromptedPack(); + final ServerResourcePackSendEvent event = + new ServerResourcePackSendEvent(resourcePackInfo, this.serverConn); server.getEventManager().fire(event).thenAcceptAsync(serverResourcePackSendEvent -> { if (playerConnection.isClosed()) { return; } if (serverResourcePackSendEvent.getResult().isAllowed()) { - ResourcePackInfo toSend = serverResourcePackSendEvent.getProvidedResourcePack(); + final ResourcePackInfo toSend = serverResourcePackSendEvent.getProvidedResourcePack(); + boolean modifiedPack = false; if (toSend != serverResourcePackSendEvent.getReceivedResourcePack()) { ((VelocityResourcePackInfo) toSend).setOriginalOrigin( ResourcePackInfo.Origin.DOWNSTREAM_SERVER); + modifiedPack = true; + } + if (serverConn.getPlayer().resourcePackHandler().hasPackAppliedByHash(toSend.getHash())) { + // Do not apply a resource pack that has already been applied + if (serverConn.getConnection() != null) { + serverConn.getConnection().write(new ResourcePackResponsePacket( + packet.getId(), packet.getHash(), PlayerResourcePackStatusEvent.Status.ACCEPTED)); + } + if (modifiedPack) { + logger.warn("A plugin has tried to modify a ResourcePack provided by the backend server " + + "with a ResourcePack already applied, the applying of the resource pack will be skipped."); + } + } else { + resourcePackToApply = null; + serverConn.getPlayer().resourcePackHandler().queueResourcePack(toSend); } - - resourcePackToApply = null; - serverConn.getPlayer().resourcePackHandler().queueResourcePack(toSend); } else if (serverConn.getConnection() != null) { serverConn.getConnection().write(new ResourcePackResponsePacket( packet.getId(), packet.getHash(), PlayerResourcePackStatusEvent.Status.DECLINED)); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java index e7d70183d..3a4cf7145 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java @@ -1027,6 +1027,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player, @Override public void sendResourcePackOffer(ResourcePackInfo packInfo) { + this.resourcePackHandler.checkAlreadyAppliedPack(packInfo.getHash()); if (this.getProtocolVersion().noLessThan(ProtocolVersion.MINECRAFT_1_8)) { Preconditions.checkNotNull(packInfo, "packInfo"); this.resourcePackHandler.queueResourcePack(packInfo); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/LegacyResourcePackHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/LegacyResourcePackHandler.java index b3797e751..2ad9c8411 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/LegacyResourcePackHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/LegacyResourcePackHandler.java @@ -23,6 +23,7 @@ import com.velocitypowered.api.proxy.player.ResourcePackInfo; import com.velocitypowered.proxy.VelocityServer; import com.velocitypowered.proxy.connection.client.ConnectedPlayer; import java.util.ArrayDeque; +import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.Queue; @@ -169,6 +170,12 @@ public sealed class LegacyResourcePackHandler extends ResourcePackHandler return handleResponseResult(queued, bundle); } + @Override + public boolean hasPackAppliedByHash(final byte[] hash) { + return this.appliedResourcePack != null + && Arrays.equals(this.appliedResourcePack.getHash(), hash); + } + protected boolean shouldDisconnectForForcePack(final PlayerResourcePackStatusEvent event) { return event.getStatus() == PlayerResourcePackStatusEvent.Status.DECLINED && event.getPackInfo() != null && event.getPackInfo().getShouldForce(); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ModernResourcePackHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ModernResourcePackHandler.java index f41895125..640c7ed55 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ModernResourcePackHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ModernResourcePackHandler.java @@ -23,6 +23,7 @@ import com.velocitypowered.api.event.player.PlayerResourcePackStatusEvent; import com.velocitypowered.api.proxy.player.ResourcePackInfo; import com.velocitypowered.proxy.VelocityServer; import com.velocitypowered.proxy.connection.client.ConnectedPlayer; +import java.util.Arrays; import java.util.Collection; import java.util.LinkedList; import java.util.List; @@ -167,4 +168,17 @@ public final class ModernResourcePackHandler extends ResourcePackHandler { return handleResponseResult(queued, bundle); } + + @Override + public boolean hasPackAppliedByHash(final byte[] hash) { + if (hash == null) { + return false; + } + for (final Map.Entry appliedPack : appliedResourcePacks.entrySet()) { + if (Arrays.equals(appliedPack.getValue().getHash(), hash)) { + return true; + } + } + return false; + } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ResourcePackHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ResourcePackHandler.java index 42a525b00..68640fb89 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ResourcePackHandler.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/player/resourcepack/ResourcePackHandler.java @@ -92,7 +92,9 @@ public abstract sealed class ResourcePackHandler */ public void queueResourcePack(final @NotNull ResourcePackRequest request) { for (final net.kyori.adventure.resource.ResourcePackInfo pack : request.packs()) { - queueResourcePack(VelocityResourcePackInfo.fromAdventureRequest(request, pack)); + final ResourcePackInfo resourcePackInfo = VelocityResourcePackInfo.fromAdventureRequest(request, pack); + this.checkAlreadyAppliedPack(resourcePackInfo.getHash()); + queueResourcePack(resourcePackInfo); } } @@ -156,4 +158,18 @@ public abstract sealed class ResourcePackHandler } return handled; } + + /** + * Check if a pack has already been applied. + * + * @param hash the resource pack hash + */ + public abstract boolean hasPackAppliedByHash(final byte[] hash); + + @SuppressWarnings("checkstyle:MissingJavadocMethod") + public void checkAlreadyAppliedPack(final byte[] hash) { + if (this.hasPackAppliedByHash(hash)) { + throw new IllegalStateException("Cannot apply a resource pack already applied"); + } + } } diff --git a/settings.gradle.kts b/settings.gradle.kts index 9222673e2..594ed9ff1 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -17,7 +17,7 @@ pluginManagement { } plugins { - id("org.gradle.toolchains.foojay-resolver-convention") version "0.4.0" + id("org.gradle.toolchains.foojay-resolver-convention") version "0.8.0" } rootProject.name = "velocity"