Skip to content

Fix salt-ssh UnicodeError crash on long -E/--pcre targets in is_reachable_host#69757

Open
ggiesen wants to merge 1 commit into
saltstack:3006.xfrom
ggiesen:fix-57207
Open

Fix salt-ssh UnicodeError crash on long -E/--pcre targets in is_reachable_host#69757
ggiesen wants to merge 1 commit into
saltstack:3006.xfrom
ggiesen:fix-57207

Conversation

@ggiesen

@ggiesen ggiesen commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

salt.utils.network.is_reachable_host() only caught socket.gaierror, but socket.getaddrinfo() raises UnicodeError ("encoding with 'idna' codec failed (label too long)") -- which is NOT a subclass of socket.gaierror -- when a name contains a DNS label longer than 63 characters. This PR also catches UnicodeError and returns False, so an overlong name is reported as "not a reachable host" instead of raising.

What issues does this PR fix or reference?

Fixes #57207

Previous Behavior

Running salt-ssh with a long PCRE target crashed with an unhandled traceback:

salt-ssh -E 'some-host|some-host|some-host|some-host|some-host|some-host|some-host' cmd.run ls
...
  File ".../salt/client/ssh/__init__.py", line 383, in _expand_target
    salt.utils.network.is_reachable_host(hostname) and \
  File ".../salt/utils/network.py", line 303, in is_reachable_host
    assert type(socket.getaddrinfo(entity_name, 0, 0, 0, 0)) == list
UnicodeError: encoding with 'idna' codec failed (UnicodeError: label too long)

The -E/--pcre target has no dots, so the whole regex is one IDNA label; when it exceeds 63 characters getaddrinfo raises UnicodeError during IDNA encoding, before any network lookup. is_reachable_host did not catch it, so it propagated out of _expand_target and aborted salt-ssh.

New Behavior

is_reachable_host catches UnicodeError in addition to socket.gaierror and returns False for such names. A PCRE target is correctly treated as not a reachable host, so _expand_target proceeds normally and salt-ssh no longer crashes. The existing socket.gaierror path and successful-resolution path are unchanged.

Merge requirements satisfied?

  • Docs -- N/A (no user-facing docs; behavioural bugfix only)
  • Changelog -- added changelog/57207.fixed.md
  • Tests written/updated -- added to tests/pytests/unit/utils/test_network.py:
    • test_is_reachable_host_57207: production-exact reproduction using the verbatim issue target; real getaddrinfo raises UnicodeError deterministically (69-char label, no network) and the function must return False. Fails without the fix, passes with it.
    • test_is_reachable_host_gaierror_unchanged: must-not-regress -- the pre-existing socket.gaierror path still returns False (passes with and without the fix).
    • test_is_reachable_host_resolvable_returns_true: peripheral coverage of the success branch (a resolvable name still returns True), guarding against the broadened except swallowing a good lookup.

Commits signed with GPG?

No

salt.utils.network.is_reachable_host only caught socket.gaierror, but
socket.getaddrinfo raises UnicodeError (an idna "label too long" error,
which is not a subclass of gaierror) when a name contains a DNS label
longer than 63 characters. A long salt-ssh -E/--pcre target triggers
this, so _expand_target crashed instead of treating the target as not a
reachable host. Also catch UnicodeError and return False.

Fixes saltstack#57207
@ggiesen ggiesen requested a review from a team as a code owner July 10, 2026 17:25
@charzl charzl self-assigned this Jul 10, 2026
@dwoz dwoz added the test:full Run the full test suite label Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants