Skip to content

Commit 2c41819

Browse files
ButterDebuggerelectronicboyTheMiningTeamYTTheMiningTeamYTpowercasgamer
authored
Added config migration for new values and removed legacy value
* preliminary cleanup of plugin message channel handling * Fix spot * Appease checkstyle * Fix tests * Add more options for ping passthrough configuration. (Merge ping-passthrough-dev) * Added 'ALLBUTVERSION' option for ping passthrough. * Trying to get the GitHub build action to run * Added more configuration options for ping passthrough. * Updated default velocity.toml * Update proxy/src/main/java/com/velocitypowered/proxy/connection/util/ServerListPingHandler.java Co-authored-by: powercas_gamer <[email protected]> * Update proxy/src/main/java/com/velocitypowered/proxy/connection/util/ServerListPingHandler.java Co-authored-by: powercas_gamer <[email protected]> * Update proxy/src/main/java/com/velocitypowered/proxy/connection/util/ServerListPingHandler.java Co-authored-by: powercas_gamer <[email protected]> * Update proxy/src/main/java/com/velocitypowered/proxy/connection/util/ServerListPingHandler.java Co-authored-by: powercas_gamer <[email protected]> * Update proxy/src/main/java/com/velocitypowered/proxy/connection/util/ServerListPingHandler.java Co-authored-by: powercas_gamer <[email protected]> * Update proxy/src/main/java/com/velocitypowered/proxy/connection/util/ServerListPingHandler.java Co-authored-by: powercas_gamer <[email protected]> * Add support for the legacy ping passthrough. --------- Co-authored-by: TheMiningTeamYT <[email protected]> Co-authored-by: powercas_gamer <[email protected]> * Use an ImmutableList Builder * Appease checkstyle gods * Also validate length before caring to invest time into processing * Fix MinecraftChannelIdentifier parsing to align with vanilla (#1552) * Removed legacy ping passthrough & added config migration - Bumped config-version to 2.8 - Updated comments and grammar - Removed legacy ping passthrough in the config * Cleaned up code to match code style --------- Co-authored-by: Shane Freeder <[email protected]> Co-authored-by: Loganius <[email protected]> Co-authored-by: TheMiningTeamYT <[email protected]> Co-authored-by: powercas_gamer <[email protected]> Co-authored-by: booky <[email protected]>
1 parent 5eb3906 commit 2c41819

File tree

14 files changed

+278
-213
lines changed

14 files changed

+278
-213
lines changed

‎api/src/main/java/com/velocitypowered/api/proxy/messages/MinecraftChannelIdentifier.java‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
import com.google.common.base.Strings;
1313
import java.util.Objects;
14-
import java.util.regex.Pattern;
1514
import net.kyori.adventure.key.Key;
1615
import org.checkerframework.checker.nullness.qual.Nullable;
1716

@@ -21,8 +20,6 @@
2120
*/
2221
public final class MinecraftChannelIdentifier implements ChannelIdentifier {
2322

24-
private static final Pattern VALID_IDENTIFIER_REGEX = Pattern.compile("[a-z0-9/\\-_]*");
25-
2623
private final String namespace;
2724
private final String name;
2825

@@ -39,7 +36,7 @@ private MinecraftChannelIdentifier(String namespace, String name) {
3936
* @return a new channel identifier
4037
*/
4138
public static MinecraftChannelIdentifier forDefaultNamespace(String name) {
42-
return new MinecraftChannelIdentifier("minecraft", name);
39+
return new MinecraftChannelIdentifier(Key.MINECRAFT_NAMESPACE, name);
4340
}
4441

4542
/**
@@ -52,14 +49,10 @@ public static MinecraftChannelIdentifier forDefaultNamespace(String name) {
5249
public static MinecraftChannelIdentifier create(String namespace, String name) {
5350
checkArgument(!Strings.isNullOrEmpty(namespace), "namespace is null or empty");
5451
checkArgument(name != null, "namespace is null or empty");
55-
checkArgument(VALID_IDENTIFIER_REGEX.matcher(namespace).matches(),
56-
"namespace is not valid, must match: %s got %s",
57-
VALID_IDENTIFIER_REGEX.toString(),
58-
namespace);
59-
checkArgument(VALID_IDENTIFIER_REGEX.matcher(name).matches(),
60-
"name is not valid, must match: %s got %s",
61-
VALID_IDENTIFIER_REGEX.toString(),
62-
name);
52+
checkArgument(Key.parseableNamespace(namespace),
53+
"namespace is not valid, must match: [a-z0-9_.-] got %s", namespace);
54+
checkArgument(Key.parseableValue(name),
55+
"name is not valid, must match: [a-z0-9/._-] got %s", name);
6356
return new MinecraftChannelIdentifier(namespace, name);
6457
}
6558

@@ -72,10 +65,9 @@ public static MinecraftChannelIdentifier create(String namespace, String name) {
7265
public static MinecraftChannelIdentifier from(String identifier) {
7366
int colonPos = identifier.indexOf(':');
7467
if (colonPos == -1) {
75-
throw new IllegalArgumentException("Identifier does not contain a colon.");
76-
}
77-
if (colonPos + 1 == identifier.length()) {
78-
throw new IllegalArgumentException("Identifier is empty.");
68+
return create(Key.MINECRAFT_NAMESPACE, identifier);
69+
} else if (colonPos == 0) {
70+
return create(Key.MINECRAFT_NAMESPACE, identifier.substring(1));
7971
}
8072
String namespace = identifier.substring(0, colonPos);
8173
String name = identifier.substring(colonPos + 1);

‎api/src/test/java/com/velocitypowered/api/proxy/messages/MinecraftChannelIdentifierTest.java‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,25 @@ void createAllowsSlashes() {
4747
create("velocity", "test/test2");
4848
}
4949

50+
@Test
51+
void fromIdentifierDefaultNamespace() {
52+
assertEquals("minecraft", from("test").getNamespace());
53+
assertEquals("minecraft", from(":test").getNamespace());
54+
}
55+
56+
@Test
57+
void fromIdentifierAllowsEmptyName() {
58+
from("minecraft:");
59+
from(":");
60+
from("");
61+
}
62+
5063
@Test
5164
void fromIdentifierThrowsOnBadValues() {
5265
assertAll(
53-
() -> assertThrows(IllegalArgumentException.class, () -> from("")),
54-
() -> assertThrows(IllegalArgumentException.class, () -> from(":")),
55-
() -> assertThrows(IllegalArgumentException.class, () -> from(":a")),
56-
() -> assertThrows(IllegalArgumentException.class, () -> from("a:")),
5766
() -> assertThrows(IllegalArgumentException.class, () -> from("hello:$$$$$$")),
67+
() -> assertThrows(IllegalArgumentException.class, () -> from("he/llo:wor/ld")),
5868
() -> assertThrows(IllegalArgumentException.class, () -> from("hello::"))
5969
);
6070
}
61-
62-
63-
}
71+
}

‎proxy/src/main/java/com/velocitypowered/proxy/config/PingPassthroughMode.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
package com.velocitypowered.proxy.config;
1919

2020
/**
21-
* Object to contain all of the things that can be toggled for ping passthrough.
21+
* Object to contain all the things that can be toggled for ping passthrough.
2222
*/
2323
public class PingPassthroughMode {
2424
public boolean version;
@@ -33,11 +33,11 @@ public class PingPassthroughMode {
3333
* but checkstyle was yelling at me because I didn't include one.
3434
* Probably for the best.
3535
*
36-
* @param version whether the version should be passed through.
37-
* @param players whether the player count should be passed through.
38-
* @param description whether the description should be passed through.
39-
* @param favicon whether the favicon should be passed through.
40-
* @param modinfo whether the modinfo should be passed through.
36+
* @param version Whether the version should be passed through.
37+
* @param players Whether the player count should be passed through.
38+
* @param description Whether the description should be passed through.
39+
* @param favicon Whether the favicon should be passed through.
40+
* @param modinfo Whether the modinfo should be passed through.
4141
*/
4242
public PingPassthroughMode(boolean version, boolean players, boolean description, boolean favicon, boolean modinfo) {
4343
this.version = version;

‎proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import com.velocitypowered.proxy.config.migration.ForwardingMigration;
3131
import com.velocitypowered.proxy.config.migration.KeyAuthenticationMigration;
3232
import com.velocitypowered.proxy.config.migration.MotdMigration;
33+
import com.velocitypowered.proxy.config.migration.PingPassthroughMigration;
3334
import com.velocitypowered.proxy.config.migration.TransferIntegrationMigration;
3435
import com.velocitypowered.proxy.util.AddressUtil;
3536
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -79,8 +80,6 @@ public class VelocityConfiguration implements ProxyConfig {
7980
@Expose
8081
private PingPassthroughMode pingPassthrough = new PingPassthroughMode(false, false, false, false, false);
8182
@Expose
82-
private LegacyPingPassthroughMode legacyPingPassthrough = LegacyPingPassthroughMode.DISABLED;
83-
@Expose
8483
private boolean samplePlayersInPing = false;
8584
private final Servers servers;
8685
private final ForcedHosts forcedHosts;
@@ -109,9 +108,8 @@ private VelocityConfiguration(String bind, String motd, int showMaxPlayers, bool
109108
boolean preventClientProxyConnections, boolean announceForge,
110109
PlayerInfoForwarding playerInfoForwardingMode, byte[] forwardingSecret,
111110
boolean onlineModeKickExistingPlayers, PingPassthroughMode pingPassthrough,
112-
LegacyPingPassthroughMode legacyPingPassthrough, boolean samplePlayersInPing,
113-
boolean enablePlayerAddressLogging, Servers servers, ForcedHosts forcedHosts,
114-
Advanced advanced, Query query, Metrics metrics, boolean forceKeyAuthentication) {
111+
boolean samplePlayersInPing, boolean enablePlayerAddressLogging, Servers servers,
112+
ForcedHosts forcedHosts, Advanced advanced, Query query, Metrics metrics, boolean forceKeyAuthentication) {
115113
this.bind = bind;
116114
this.motd = motd;
117115
this.showMaxPlayers = showMaxPlayers;
@@ -122,7 +120,6 @@ private VelocityConfiguration(String bind, String motd, int showMaxPlayers, bool
122120
this.forwardingSecret = forwardingSecret;
123121
this.onlineModeKickExistingPlayers = onlineModeKickExistingPlayers;
124122
this.pingPassthrough = pingPassthrough;
125-
this.legacyPingPassthrough = legacyPingPassthrough;
126123
this.samplePlayersInPing = samplePlayersInPing;
127124
this.enablePlayerAddressLogging = enablePlayerAddressLogging;
128125
this.servers = servers;
@@ -408,10 +405,6 @@ public PingPassthroughMode getPingPassthrough() {
408405
return pingPassthrough;
409406
}
410407

411-
public LegacyPingPassthroughMode getLegacyPingPassthrough() {
412-
return legacyPingPassthrough;
413-
}
414-
415408
public boolean getSamplePlayersInPing() {
416409
return samplePlayersInPing;
417410
}
@@ -511,7 +504,8 @@ public static VelocityConfiguration read(Path path) throws IOException {
511504
new ForwardingMigration(),
512505
new KeyAuthenticationMigration(),
513506
new MotdMigration(),
514-
new TransferIntegrationMigration()
507+
new TransferIntegrationMigration(),
508+
new PingPassthroughMigration()
515509
};
516510

517511
for (final ConfigurationMigration migration : migrations) {
@@ -549,8 +543,6 @@ public static VelocityConfiguration read(Path path) throws IOException {
549543
final CommentedConfig metricsConfig = config.get("metrics");
550544
final PlayerInfoForwarding forwardingMode = config.getEnumOrElse(
551545
"player-info-forwarding-mode", PlayerInfoForwarding.NONE);
552-
final LegacyPingPassthroughMode legacyPingPassthrough = config.getEnumOrElse("ping-passthrough",
553-
LegacyPingPassthroughMode.DISABLED);
554546
final PingPassthroughMode pingPassthrough = new PingPassthroughMode(
555547
config.getOrElse("ping-passthrough-version", false),
556548
config.getOrElse("ping-passthrough-players", false),
@@ -589,7 +581,6 @@ public static VelocityConfiguration read(Path path) throws IOException {
589581
forwardingSecret,
590582
kickExisting,
591583
pingPassthrough,
592-
legacyPingPassthrough,
593584
samplePlayersInPing,
594585
enablePlayerAddressLogging,
595586
new Servers(serversConfig),

‎proxy/src/main/java/com/velocitypowered/proxy/config/migration/ConfigurationMigration.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ public sealed interface ConfigurationMigration
2828
permits ForwardingMigration,
2929
KeyAuthenticationMigration,
3030
MotdMigration,
31-
TransferIntegrationMigration {
31+
TransferIntegrationMigration,
32+
PingPassthroughMigration {
3233
boolean shouldMigrate(CommentedFileConfig config);
3334

3435
void migrate(CommentedFileConfig config, Logger logger) throws IOException;

‎proxy/src/main/java/com/velocitypowered/proxy/config/migration/PingPassthroughMigration.java‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
* Copyright (C) 2024 Velocity Contributors
3+
*
4+
* This program is free software: you can redistribute it and/or modify
5+
* it under the terms of the GNU General Public License as published by
6+
* the Free Software Foundation, either version 3 of the License, or
7+
* (at your option) any later version.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
* GNU General Public License for more details.
13+
*
14+
* You should have received a copy of the GNU General Public License
15+
* along with this program. If not, see <https://www.gnu.org/licenses/>.
16+
*/
17+
18+
package com.velocitypowered.proxy.config.migration;
19+
20+
import com.electronwill.nightconfig.core.file.CommentedFileConfig;
21+
import com.velocitypowered.proxy.config.LegacyPingPassthroughMode;
22+
import org.apache.logging.log4j.Logger;
23+
24+
/**
25+
* Migrate the old ping passthrough entry to separate config entries.
26+
*/
27+
public final class PingPassthroughMigration implements ConfigurationMigration {
28+
@Override
29+
public boolean shouldMigrate(final CommentedFileConfig config) {
30+
return configVersion(config) < 2.8;
31+
}
32+
33+
@Override
34+
public void migrate(final CommentedFileConfig config, final Logger logger) {
35+
// Get legacy ping passthrough value
36+
final LegacyPingPassthroughMode legacyMode = config.getEnumOrElse("ping-passthrough",
37+
LegacyPingPassthroughMode.DISABLED);
38+
39+
config.removeComment("ping-passthrough");
40+
config.remove("ping-passthrough");
41+
42+
// Create ping passthrough entry for the version
43+
config.set("ping-passthrough-version", legacyMode.equals(LegacyPingPassthroughMode.ALL));
44+
config.setComment(
45+
"ping-passthrough-version",
46+
" Should Velocity pass the version number from the backend server when responding to server list ping requests?"
47+
);
48+
49+
// Create ping passthrough entry for the players
50+
config.set("ping-passthrough-players", legacyMode.equals(LegacyPingPassthroughMode.ALL));
51+
config.setComment(
52+
"ping-passthrough-players",
53+
" Should Velocity pass the player count from the backend server when responding to server list ping requests?"
54+
);
55+
56+
// Create ping passthrough entry for the description
57+
config.set("ping-passthrough-description",
58+
legacyMode.equals(LegacyPingPassthroughMode.ALL)
59+
|| legacyMode.equals(LegacyPingPassthroughMode.DESCRIPTION)
60+
);
61+
config.setComment(
62+
"ping-passthrough-description",
63+
" Should Velocity pass the description from the backend server when responding to server list ping requests?"
64+
);
65+
66+
// Create ping passthrough entry for the favicon
67+
config.set("ping-passthrough-favicon", legacyMode.equals(LegacyPingPassthroughMode.ALL));
68+
config.setComment(
69+
"ping-passthrough-favicon",
70+
" Should Velocity pass the favicon (also known as the server icon) from the backend server when responding to server list ping requests?"
71+
);
72+
73+
// Create ping passthrough entry for the mods info
74+
config.set("ping-passthrough-modinfo",
75+
legacyMode.equals(LegacyPingPassthroughMode.ALL)
76+
|| legacyMode.equals(LegacyPingPassthroughMode.MODS)
77+
|| legacyMode.equals(LegacyPingPassthroughMode.DESCRIPTION)
78+
);
79+
config.setComment(
80+
"ping-passthrough-modinfo",
81+
" Should Velocity pass the mod list from the backend server when responding to server list ping requests?"
82+
);
83+
84+
// Update config version
85+
config.set("config-version", "2.8");
86+
}
87+
}

‎proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BungeeCordMessageResponder.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.velocitypowered.api.network.ProtocolVersion;
2121
import com.velocitypowered.api.proxy.Player;
2222
import com.velocitypowered.api.proxy.ServerConnection;
23+
import com.velocitypowered.api.proxy.messages.ChannelIdentifier;
2324
import com.velocitypowered.api.proxy.messages.LegacyChannelIdentifier;
2425
import com.velocitypowered.api.proxy.messages.MinecraftChannelIdentifier;
2526
import com.velocitypowered.api.proxy.server.RegisteredServer;
@@ -316,9 +317,9 @@ private void processGetPlayerServer(ByteBufDataInput in) {
316317
});
317318
}
318319

319-
static String getBungeeCordChannel(ProtocolVersion version) {
320-
return version.noLessThan(ProtocolVersion.MINECRAFT_1_13) ? MODERN_CHANNEL.getId()
321-
: LEGACY_CHANNEL.getId();
320+
static ChannelIdentifier getBungeeCordChannel(ProtocolVersion version) {
321+
return version.noLessThan(ProtocolVersion.MINECRAFT_1_13) ? MODERN_CHANNEL
322+
: LEGACY_CHANNEL;
322323
}
323324

324325
// Note: this method will always release the buffer!
@@ -329,8 +330,8 @@ private void sendResponseOnConnection(ByteBuf buf) {
329330
// Note: this method will always release the buffer!
330331
private static void sendServerResponse(ConnectedPlayer player, ByteBuf buf) {
331332
MinecraftConnection serverConnection = player.ensureAndGetCurrentServer().ensureConnected();
332-
String chan = getBungeeCordChannel(serverConnection.getProtocolVersion());
333-
PluginMessagePacket msg = new PluginMessagePacket(chan, buf);
333+
ChannelIdentifier chan = getBungeeCordChannel(serverConnection.getProtocolVersion());
334+
PluginMessagePacket msg = new PluginMessagePacket(chan.getId(), buf);
334335
serverConnection.write(msg);
335336
}
336337

‎proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@
3030
import com.velocitypowered.api.event.player.configuration.PlayerEnteredConfigurationEvent;
3131
import com.velocitypowered.api.network.ProtocolVersion;
3232
import com.velocitypowered.api.proxy.messages.ChannelIdentifier;
33-
import com.velocitypowered.api.proxy.messages.LegacyChannelIdentifier;
34-
import com.velocitypowered.api.proxy.messages.MinecraftChannelIdentifier;
3533
import com.velocitypowered.proxy.VelocityServer;
3634
import com.velocitypowered.proxy.connection.ConnectionTypes;
3735
import com.velocitypowered.proxy.connection.MinecraftConnection;
@@ -162,7 +160,7 @@ private boolean validateChat(String message) {
162160
@Override
163161
public void activated() {
164162
configSwitchFuture = new CompletableFuture<>();
165-
Collection<String> channels =
163+
Collection<ChannelIdentifier> channels =
166164
server.getChannelRegistrar().getChannelsForProtocol(player.getProtocolVersion());
167165
if (!channels.isEmpty()) {
168166
PluginMessagePacket register = constructChannelsPacket(player.getProtocolVersion(), channels);
@@ -310,22 +308,17 @@ public boolean handle(PluginMessagePacket packet) {
310308
logger.warn("A plugin message was received while the backend server was not "
311309
+ "ready. Channel: {}. Packet discarded.", packet.getChannel());
312310
} else if (PluginMessageUtil.isRegister(packet)) {
313-
List<String> channels = PluginMessageUtil.getChannels(packet);
311+
List<ChannelIdentifier> channels =
312+
PluginMessageUtil.getChannels(this.player.getClientsideChannels().size(), packet,
313+
this.player.getProtocolVersion());
314314
player.getClientsideChannels().addAll(channels);
315-
List<ChannelIdentifier> channelIdentifiers = new ArrayList<>();
316-
for (String channel : channels) {
317-
try {
318-
channelIdentifiers.add(MinecraftChannelIdentifier.from(channel));
319-
} catch (IllegalArgumentException e) {
320-
channelIdentifiers.add(new LegacyChannelIdentifier(channel));
321-
}
322-
}
323315
server.getEventManager()
324316
.fireAndForget(
325-
new PlayerChannelRegisterEvent(player, ImmutableList.copyOf(channelIdentifiers)));
317+
new PlayerChannelRegisterEvent(player, ImmutableList.copyOf(channels)));
326318
backendConn.write(packet.retain());
327319
} else if (PluginMessageUtil.isUnregister(packet)) {
328-
player.getClientsideChannels().removeAll(PluginMessageUtil.getChannels(packet));
320+
player.getClientsideChannels()
321+
.removeAll(PluginMessageUtil.getChannels(0, packet, this.player.getProtocolVersion()));
329322
backendConn.write(packet.retain());
330323
} else if (PluginMessageUtil.isMcBrand(packet)) {
331324
String brand = PluginMessageUtil.readBrandMessage(packet.content());
@@ -402,7 +395,8 @@ public boolean handle(FinishedUpdatePacket packet) {
402395
// Complete client switch
403396
player.getConnection().setActiveSessionHandler(StateRegistry.CONFIG);
404397
VelocityServerConnection serverConnection = player.getConnectedServer();
405-
server.getEventManager().fireAndForget(new PlayerEnteredConfigurationEvent(player, serverConnection));
398+
server.getEventManager()
399+
.fireAndForget(new PlayerEnteredConfigurationEvent(player, serverConnection));
406400
if (serverConnection != null) {
407401
MinecraftConnection smc = serverConnection.ensureConnected();
408402
CompletableFuture.runAsync(() -> {
@@ -589,7 +583,7 @@ public void handleBackendJoinGame(JoinGamePacket joinGame, VelocityServerConnect
589583

590584
// Tell the server about the proxy's plugin message channels.
591585
ProtocolVersion serverVersion = serverMc.getProtocolVersion();
592-
final Collection<String> channels = server.getChannelRegistrar()
586+
final Collection<ChannelIdentifier> channels = server.getChannelRegistrar()
593587
.getChannelsForProtocol(serverMc.getProtocolVersion());
594588
if (!channels.isEmpty()) {
595589
serverMc.delayedWrite(constructChannelsPacket(serverVersion, channels));

0 commit comments

Comments
 (0)