Add wildcard forced hosts (fix #1587)

[lllincoln]

I added wildcard support to [forced-hosts], resolving #1587. Unit tests pass and checkstyle passes and I've tested this against my own running proxy. I'd appreciate feedback or additional testing if needed, especially from those on #1587 who submitted the issue.

What this does

Consider the example from #1587

[forced-hosts]
# Soul Realms
"play.soulrealms.net" = [ "soulrealms-smp-1" ]
"yt.soulrealms.net"   = [ "soulrealms-smp-1" ]
"ip.soulrealms.net"   = [ "soulrealms-smp-1" ]
"soulrealms.net"      = [ "soulrealms-smp-1" ]

Now this can be changed to:

[forced-hosts]
# Soul Realms
"*.soulrealms.net" = [ "soulrealms-smp-1" ]
"soulrealms.net"   = [ "soulrealms-smp-1" ]

How matching works

I added AddressUtil.isHostMatchingPattern(pattern, host). Patterns are matched on each label where * matches exactly one label. To be matched, patterns must have the same number of labels as the host. Matching is case insensitive. For example:

• *.example.com matches a.example.com but not example.com (fewer labels) or b.a.example.com (more labels).
• b.*.example.com matches b.a.example.com

Matching order

In both ConnectedPlayer and ServerListPingHandler, forced-host lookup now follows a hierarchy where exact matches always win over wildcards, so adding a *.example.com entry won't change routing for any host you've already defined explicitly.

1. Exact match (unchanged).
2. Wildcard match (my addition).
3. Default (getAttemptConnectionOrder() in ServerListPingHandler, or an empty collection in ConnectedPlayer. this are both unchanged from what was before).

Tests

I added ForcedHostsTest covering isHostMatchingPattern cases for single and mid-pattern wildcards, label-count mismatches, and a non-matching domain.

Potential issue

If two different wildcard patterns could match the same host, the one chosen depends on config map iteration order. Would like feedback on what the behavior should be here

[WouterGritter]

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]

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]

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]

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