Merge pull request #10496 from voidsipp/fix/10465-server-address-ports

Match vanilla server-address port syntax

crates/horizon-protocol/examples/server_address_parse_probe.rs

@@ -0,0 +1,13 @@
+//! Host/port parser corpus, without DNS or a connection.
+use horizon_protocol::servers::parse_address;
+use std::{error::Error, fs::File};
+
+fn main() -> Result<(), Box<dyn Error>> {
+    let args: Vec<_> = std::env::args().collect();
+    let input = args.get(1).ok_or("expected input and output paths")?;
+    let output = args.get(2).ok_or("expected output path")?;
+    let addresses: Vec<String> = serde_json::from_reader(File::open(input)?)?;
+    let results: Vec<_> = addresses.iter().map(|text| parse_address(text)).collect();
+    serde_json::to_writer(File::create(output)?, &results)?;
+    Ok(())
+}

crates/horizon-protocol/src/servers.rs

@@ -166,21 +166,43 @@ pub fn parse_address(text: &str) -> Option<(String, u16)> {
         return None;
     }
     if let Some(rest) = text.strip_prefix('[') {
-        let (host, tail) = rest.split_once(']')?;
+        let (host, tail) = rest.rsplit_once(']')?;
+        // HostAndPort requires a colon inside a bracketed host; it does not
+        // accept a bracketed DNS name as an IPv6-style address.
+        if !host.contains(':') {
+            return None;
+        }
         let port = match tail.strip_prefix(':') {
-            Some(p) => p.parse().ok()?,
+            Some(p) if p.bytes().all(|byte| byte.is_ascii_digit()) => parse_port(p)?,
             None if tail.is_empty() => 25565,
-            None => return None,
+            _ => return None,
         };
         return Some((host.to_owned(), port));
     }
     match text.rsplit_once(':') {
-        Some((host, port)) if !host.contains(':') => Some((host.to_owned(), port.parse().ok()?)),
+        Some((host, port)) if !host.contains(':') => {
+            if host.is_empty() {
+                return None;
+            }
+            Some((host.to_owned(), parse_port(port)?))
+        }
         Some(_) => Some((text.to_owned(), 25565)),
         None => Some((text.to_owned(), 25565)),
     }
 }
 
+/// HostAndPort uses the default for an empty port, refuses a leading plus,
+/// and accepts signed zero on its non-bracketed path before checking range.
+fn parse_port(text: &str) -> Option<u16> {
+    if text.is_empty() {
+        Some(25565)
+    } else if text.starts_with('+') {
+        None
+    } else {
+        u16::try_from(text.parse::<i32>().ok()?).ok()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -256,4 +278,44 @@ mod tests {
         assert_eq!(parse_address("host:notaport"), None);
         assert_eq!(parse_address(""), None);
     }
+
+    #[test]
+    fn empty_explicit_ports_use_the_vanilla_default() {
+        for (input, host) in [
+            ("example.com:", "example.com"),
+            ("127.0.0.1:", "127.0.0.1"),
+            ("[::1]:", "::1"),
+            ("[2001:db8::1]:", "2001:db8::1"),
+            (" \t[::1]:\r\n", "::1"),
+        ] {
+            assert_eq!(parse_address(input), Some((host.into(), 25565)), "{input}");
+        }
+    }
+
+    #[test]
+    fn ports_and_brackets_keep_vanilla_validation() {
+        for input in [
+            ":",
+            ":25565",
+            "[]",
+            "[]:",
+            "[example.com]",
+            "[example.com]:",
+            "[::1]junk",
+            "example.com:+25565",
+            "[::1]:+25565",
+            "[::1]:-0",
+            "example.com:-1",
+            "example.com:65536",
+            "[::1]:65536",
+            "host:2147483648",
+            "host:1_000",
+            "host:\u{0661}\u{0662}",
+        ] {
+            assert_eq!(parse_address(input), None, "{input}");
+        }
+        assert_eq!(parse_address("host:-0"), Some(("host".into(), 0)));
+        assert_eq!(parse_address("host:0"), Some(("host".into(), 0)));
+        assert_eq!(parse_address("[::1]:65535"), Some(("::1".into(), 65535)));
+    }
 }

mods/horizon/src/test/java/dev/horizonlab/parity/VanillaServerAddressOracle.java

@@ -1,7 +1,12 @@
 package dev.horizonlab.parity;
 
+import com.google.gson.JsonArray;
+import com.google.gson.JsonNull;
+import com.google.gson.JsonParser;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.Optional;
 import net.minecraft.client.multiplayer.resolver.AddressCheck;
 import net.minecraft.client.multiplayer.resolver.ResolvedServerAddress;
@@ -13,6 +18,27 @@ import net.minecraft.client.multiplayer.resolver.ServerRedirectHandler;
 /** ConnectScreen.startConnecting uses the resolved socket's host and port for login. */
 public final class VanillaServerAddressOracle {
     public static void main(String[] args) throws Exception {
+        if (args.length == 2) {
+            JsonArray output = new JsonArray();
+            try (var reader = Files.newBufferedReader(Path.of(args[0]))) {
+                for (var input : JsonParser.parseReader(reader).getAsJsonArray()) {
+                    // Preserve Horizon's existing outer ASCII whitespace trim;
+                    // this corpus compares host/port syntax, not IDN mapping.
+                    String text = input.getAsString().trim();
+                    if (!ServerAddress.isValidAddress(text)) {
+                        output.add(JsonNull.INSTANCE);
+                    } else {
+                        var address = ServerAddress.parseString(text);
+                        JsonArray parsed = new JsonArray();
+                        parsed.add(address.getHost());
+                        parsed.add(address.getPort());
+                        output.add(parsed);
+                    }
+                }
+            }
+            Files.writeString(Path.of(args[1]), output.toString());
+            return;
+        }
         AddressCheck allow = new AddressCheck() {
             public boolean isAllowed(ResolvedServerAddress address) { return true; }
             public boolean isAllowed(ServerAddress address) { return true; }

tools/test_server_address_parity.py

@@ -1,7 +1,12 @@
 #!/usr/bin/env python3
-"""Login's resolved server host/port versus vanilla ServerNameResolver (no live DNS)."""
+"""Resolved login targets and trimmed ASCII host/port syntax against vanilla.
+
+No live DNS, server or game. IDN normalization and raw whitespace semantics are
+not claimed: the parsing corpus keeps Horizon's existing outer ASCII trim.
+"""
 import json
 import os
+import random
 import subprocess
 
 try:
@@ -11,6 +16,27 @@ except ImportError:
     import test_rust_parity as base
 
 
+def address_cases():
+    hosts = ['localhost', 'play.example.org', '127.0.0.1', 'EXAMPLE.COM', '[::1]',
+             '[2001:db8::1]', '[foo:bar]', '[example.com]', '[]', '[::1]junk', '']
+    ports = ['', '0', '1', '25565', '65535', '65536', '-0', '-00', '-1', '+0',
+             '+25565', '2147483647', '2147483648', '-2147483648', '-2147483649',
+             '00000000000000025565', ' 25565', '25565 ', '1_0', '0x10',
+             '\u0661\u0662\u0663', '\uff11\uff12\uff13', '++1', '--0']
+    cases = [host for host in hosts]
+    cases += [host + ':' + port for host in hosts for port in ports]
+    cases += ['::1', '2001:db8::1', '[::1', '[::1]::', '[::1]]:', '[[::1]]:',
+              ' \tplay.example.org:\r\n', ' \t[::1]:\r\n']
+    rng = random.Random(0x10465)
+    for _ in range(1000):
+        port = ''.join(rng.choices('0123456789+-_: abc\u0661\uff11', k=rng.randrange(13)))
+        cases.extend(host + ':' + port for host in ('localhost', '127.0.0.1', '[::1]', ''))
+    # Multiple unbracketed colons make the whole value a hostname, not a port.
+    # Unicode in that shape tests IDN normalization, outside this syntax corpus.
+    return [value for value in cases
+            if value.startswith('[') or value.count(':') <= 1 or value.isascii()]
+
+
 def main():
     lock, jars, libraries = base.verify_artifacts()
     build = lab.LOCAL / 'rust-server-address-parity'
@@ -20,6 +46,10 @@ def main():
     classes = build / 'classes'
     classes.mkdir(exist_ok=True)
     oracle = lab.ROOT / 'mods/horizon/src/test/java/dev/horizonlab/parity/VanillaServerAddressOracle.java'
+    sources = [oracle, lab.ROOT / 'crates/horizon-protocol/src/servers.rs',
+               lab.ROOT / 'crates/horizon-protocol/examples/server_address_parse_probe.rs',
+               lab.ROOT / 'tools/test_server_address_parity.py']
+    hashes = base.source_hashes(sources)
     _, game = lab.config()
     manifest = json.loads((game / 'manifest.json').read_text(encoding='utf-8'))
     for artifact in lab.client_library_artifacts(manifest):
@@ -44,8 +74,22 @@ def main():
                   'resolver::tests::login_handshake_address_matches_vanilla', '--', '--nocapture'], 'rust')
     if len(expected) != 3 or actual != expected:
         raise AssertionError(f'vanilla: {expected!r}; Horizon: {actual!r}')
-    report.write_text(json.dumps({'passed': True, 'cases': expected}, indent=2) + '\n', encoding='utf-8')
-    print(f'Server address parity: {len(expected)} cases passed')
+    inputs = address_cases()
+    lab.write_json(build / 'addresses.json', inputs)
+    run(['java', '-cp', str(classes) + os.pathsep + classpath,
+         'dev.horizonlab.parity.VanillaServerAddressOracle', build / 'addresses.json', build / 'java-parse.json'], 'vanilla-parse')
+    run(['cargo', 'run', '--locked', '-p', 'horizon-protocol', '--example', 'server_address_parse_probe', '--',
+         build / 'addresses.json', build / 'rust-parse.json'], 'rust-parse')
+    java = json.loads((build / 'java-parse.json').read_text(encoding='utf-8'))
+    rust = json.loads((build / 'rust-parse.json').read_text(encoding='utf-8'))
+    assert len(java) == len(rust) == len(inputs)
+    differences = [(text, native, result) for text, native, result in zip(inputs, java, rust) if native != result]
+    base.ensure_sources_unchanged(sources, hashes)
+    lab.write_json(report, {'passed': not differences, 'resolved_cases': expected,
+                           'parsing_cases': len(inputs), 'differences': differences,
+                           'sources_sha256': hashes, 'scope': __doc__})
+    assert not differences, f'{len(differences)} address differences: {differences[:5]}'
+    print(f'Server address parity: {len(expected)} resolved targets and {len(inputs)} parser cases passed')
 
 
 if __name__ == '__main__':