Skip to content

Add wildcard forced hosts (fix #1587) #1826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lllincoln wants to merge 10 commits into
base: dev/3.0.0
Choose a base branch
from
+148 −4
Open

Add wildcard forced hosts (fix #1587) #1826

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8f0d336
Add wildcard forced hosts support (#1587)
lllincoln Jun 22, 2026
cdbf7b5
Add additional test cases
lllincoln Jun 22, 2026
b777c79
Reformat test to pass checkstyle
lllincoln Jun 22, 2026
415d127
Add missing GPL header
lllincoln Jun 22, 2026
70efa55
Use fictious domains, add more test cases, and update GPL year to 2026
lllincoln Jun 22, 2026
268f942
Split tests into separate methods
lllincoln Jun 23, 2026
9093623
Update javadoc, add @Nullable, and flip comparison for `isHostMatchin…
lllincoln Jun 23, 2026
36fba4a
Move tests to AddressUtilTest in correct package, import isHostMatchi…
lllincoln Jun 23, 2026
54d6816
Extract server resolution logic to separate method and optimize reada…
lllincoln Jun 23, 2026
845de16
Use method reference in lambda
lllincoln Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@
import com.velocitypowered.proxy.tablist.KeyedVelocityTabList;
import com.velocitypowered.proxy.tablist.VelocityTabList;
import com.velocitypowered.proxy.tablist.VelocityTabListLegacy;
import com.velocitypowered.proxy.util.AddressUtil;
import com.velocitypowered.proxy.util.ClosestLocaleMatcher;
import com.velocitypowered.proxy.util.DurationUtils;
import com.velocitypowered.proxy.util.TranslatableMapper;
Expand All @@ -111,6 +112,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
Expand Down Expand Up @@ -890,8 +892,17 @@ private Optional<RegisteredServer> getNextServerToTry(@Nullable RegisteredServer
String virtualHostStr = getVirtualHost().map(InetSocketAddress::getHostString)
.orElse("")
.toLowerCase(Locale.ROOT);
serversToTry = server.getConfiguration().getForcedHosts().getOrDefault(virtualHostStr,
Collections.emptyList());
serversToTry = server.getConfiguration().getForcedHosts().getOrDefault(
virtualHostStr,
server.getConfiguration()
.getForcedHosts()
.entrySet()
.stream()
.filter(entry -> AddressUtil.isHostMatchingPattern(entry.getKey(), virtualHostStr))
.map(Map.Entry::getValue)
.findFirst()
.orElse(Collections.emptyList())

@WouterGritter WouterGritter Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three nits:

1 - This is a pretty large method chain that's duplicated twice, it would probably make sense to extract this out into its own method.

2 - the "or default" value is always computed, even in the case where the exact virtualHostStr mapping is present. And the .orElse(Collections.emptyList()) also always returns an empty list. Fortunately this doesn't comput or allocate anything, but this means that all three cases - exact match, pattern match, empty list - are always computed, even when an exact match could abort the latter two, which is a smell

3 - Essentially this is a giant double if-else statement:

if (/* exact forced host defined */) {
  serversToTry = /* exact forced host */;
} else if (/* isHostMatchingPattern matches */) {
  serversToTry = /* isHostMatchingPattern result */;
} else {
  serversToTry = Collections.emptyList();
}

However this is disguised in getOrDefault (first two if and if else) and .orElse(Collections.emptyList() (last else). I would probably make this more explicit, either something similar to the above pseudo code or using an optional chain with 2x .orElseGet(), keeping it all in optional-land.

@lllincoln lllincoln Jun 23, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, I was thinking about this too. Where do you think the right place to hold this new method could be? I would have put it in ConnectedPlayer, but this same pattern is in ServerListPingHandler as well, which are two separate things from each other. Thoughts?

@WouterGritter WouterGritter Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AddressUtil seems like the right place, as you have already introduced the new isHostMatchingPattern there.

Consider making it return an Optional<String>, allowing the caller to change an empty result to an empty list (ConnectedPlayer) or server.getConfiguration().getAttemptConnectionOrder() (ServerListPingHandler). (with orElseGet() to avoid computing when the optional is present)

@lllincoln lllincoln Jun 23, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See 54d6816. I've implemented it using Optional<String> but not 2x .orElseGet() since I find using if/else and a for loop a bit more readable, but let me know if you think otherwise

);
}

if (serversToTry.isEmpty()) {
Expand Down
13 changes: 12 additions & 1 deletion proxy/src/main/java/com/velocitypowered/proxy/connection/util/ServerListPingHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@
import com.velocitypowered.proxy.config.PingPassthroughMode;
import com.velocitypowered.proxy.config.VelocityConfiguration;
import com.velocitypowered.proxy.server.VelocityRegisteredServer;
import com.velocitypowered.proxy.util.AddressUtil;
import java.net.InetSocketAddress;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -175,7 +177,16 @@ public CompletableFuture<ServerPing> getInitialPing(VelocityInboundConnection co
.map(str -> str.toLowerCase(Locale.ROOT))
.orElse("");
List<String> serversToTry = server.getConfiguration().getForcedHosts().getOrDefault(
virtualHostStr, server.getConfiguration().getAttemptConnectionOrder());
virtualHostStr,
server.getConfiguration()
.getForcedHosts()
.entrySet()
.stream()
.filter(entry -> AddressUtil.isHostMatchingPattern(entry.getKey(), virtualHostStr))
.map(Map.Entry::getValue)
.findFirst()
.orElse(server.getConfiguration().getAttemptConnectionOrder())
);
return attemptPingPassthrough(connection, passthroughMode, serversToTry, shownVersion, virtualHostStr);
}
}
Expand Down
35 changes: 35 additions & 0 deletions proxy/src/main/java/com/velocitypowered/proxy/util/AddressUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

import com.google.common.base.Preconditions;
import com.google.common.net.InetAddresses;

import javax.annotation.Nullable;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.URI;
Expand Down Expand Up @@ -74,4 +76,37 @@ public static InetSocketAddress parseAndResolveAddress(String ip) {
int port = uri.getPort() == -1 ? DEFAULT_MINECRAFT_PORT : uri.getPort();
return new InetSocketAddress(uri.getHost(), port);
}

/**
* Tests whether a host matches a pattern whose labels may be the
* wildcard {@code "*"}. Each {@code "*"} matches exactly one label; all other
* labels match case-insensitively.
*
* @param pattern the pattern to match against, for example, {@code *.example.com}
* @param host the virtual host to test, for example, {@code play.example.com}
* @return true if the host matches the pattern, false otherwise
*/
public static boolean isHostMatchingPattern(@Nullable String pattern, @Nullable String host) {
if (host == null || pattern == null) {
return false;
}

String[] patternDomains = pattern.split("\\.");
String[] strDomains = host.split("\\.");

if (patternDomains.length != strDomains.length) {
return false;
}

for (int i = 0; i < patternDomains.length; i++) {
String patternDomain = patternDomains[i];
String strDomain = strDomains[i];

if (!patternDomain.equals("*") && !strDomain.equalsIgnoreCase(patternDomain)) {
return false;
}
}

return true;
}
}
71 changes: 71 additions & 0 deletions proxy/src/test/java/com/velocitypowered/proxy/connection/client/HostPatternMatchingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* Copyright (C) 2018-2026 Velocity 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 <https://www.gnu.org/licenses/>.
*/

package com.velocitypowered.proxy.connection.client;

import com.velocitypowered.proxy.util.AddressUtil;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class HostPatternMatchingTest {
Comment thread
lllincoln marked this conversation as resolved.
Outdated
@Test
void testOneWildcardMatches() {
Assertions.assertTrue(AddressUtil.isHostMatchingPattern("*.example.com", "play.example.com"));
Assertions.assertTrue(AddressUtil.isHostMatchingPattern("*.example.com", "a.example.com"));
Assertions.assertTrue(AddressUtil.isHostMatchingPattern("b.*.example.com", "b.a.example.com"));
}

@Test
void testMultipleWildcardsMatch() {
Assertions.assertTrue(AddressUtil.isHostMatchingPattern("*.*.example.com", "a.b.example.com"));
}

@Test
void testDifferentNumberOfLabelsDoNotMatch() {
Assertions.assertFalse(AddressUtil.isHostMatchingPattern("*.example.com", "a.b.example.com"));
}

@Test
void testWildcardsDoNotMatchApexDomain() {
Assertions.assertFalse(AddressUtil.isHostMatchingPattern("*.example.com", "example.com"));
}

@Test
void testExactMatchesMatch() {
Assertions.assertTrue(AddressUtil.isHostMatchingPattern("example.com", "example.com"));
Assertions.assertTrue(AddressUtil.isHostMatchingPattern("play.example.com", "play.example.com"));
}

@Test
void testDifferentDomainsDoNotMatch() {
Assertions.assertFalse(AddressUtil.isHostMatchingPattern("otherdomain.com", "example.com"));
Assertions.assertFalse(AddressUtil.isHostMatchingPattern("a.otherdomain.com", "a.example.com"));
}

@Test
void testMalformedArgumentsDoNotMatch() {
Assertions.assertFalse(AddressUtil.isHostMatchingPattern(null, "example.com"));
Assertions.assertFalse(AddressUtil.isHostMatchingPattern("example.com", null));
Assertions.assertFalse(AddressUtil.isHostMatchingPattern(null, null));
}

@Test
void testCaseInsensitivityMatches() {
Assertions.assertTrue(AddressUtil.isHostMatchingPattern("Example.COM", "example.com"));
Assertions.assertTrue(AddressUtil.isHostMatchingPattern("*.Example.com", "play.EXAMPLE.com"));
}
}