Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions changelog/57207.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed salt-ssh crashing with an uncaught UnicodeError when a long ``-E``/``--pcre`` target produces an overlong IDNA label in ``is_reachable_host``
4 changes: 3 additions & 1 deletion salt/utils/network.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,9 @@ def is_reachable_host(entity_name):
try:
assert type(socket.getaddrinfo(entity_name, 0, 0, 0, 0)) == list
ret = True
except socket.gaierror:
except (socket.gaierror, UnicodeError):
# UnicodeError is raised (not a subclass of socket.gaierror) when the
# name has an overlong IDNA label, e.g. a long salt-ssh -E/--pcre target.
ret = False

return ret
Expand Down
45 changes: 45 additions & 0 deletions tests/pytests/unit/utils/test_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -1635,3 +1635,48 @@ def test_ip_addrs(linux_interfaces_dict):
):
ret = network.ip_addrs6("eth0")
assert ret == ["fe80::e23f:49ff:fe85:6aaf"]


def test_is_reachable_host_57207():
"""
A long salt-ssh -E/--pcre target has a single DNS label longer than 63
characters. socket.getaddrinfo raises UnicodeError ("label too long") from
the idna codec before any network lookup. UnicodeError is not a subclass of
socket.gaierror, so it must be caught explicitly and reported as not
reachable rather than crashing salt-ssh's _expand_target.
"""
# Production-exact call: _expand_target passes the raw target as the single
# positional arg, e.g. salt.utils.network.is_reachable_host(hostname).
# This is the verbatim -E target from the issue; the real getaddrinfo raises
# UnicodeError deterministically (69-char label, no network needed).
assert (
network.is_reachable_host(
"some-host|some-host|some-host|some-host|some-host|some-host|some-host"
)
is False
)


def test_is_reachable_host_gaierror_unchanged():
"""
Inverse / must-not-regress: broadening the except clause to also catch
UnicodeError must not change the pre-existing socket.gaierror path. A name
that fails normal resolution still returns False. Passes with and without
the fix because the gaierror branch is untouched.
"""
with patch.object(socket, "getaddrinfo", MagicMock(side_effect=socket.gaierror)):
assert network.is_reachable_host("nope.invalid") is False


def test_is_reachable_host_resolvable_returns_true():
"""
Peripheral coverage of the success branch: when getaddrinfo returns a list
the host is reported reachable. Guards against the fix accidentally swallowing
a successful lookup.
"""
with patch.object(
socket,
"getaddrinfo",
MagicMock(return_value=[(socket.AF_INET, 0, 0, "", ("127.0.0.1", 0))]),
):
assert network.is_reachable_host("localhost") is True
Loading