Skip to content

Avoid depending on a string match into host_info property, in status output.#1656

Merged
rolandwalker merged 1 commit intomainfrom
RW/dont-depend-on-string-match-into-host-info
Feb 28, 2026
Merged

Avoid depending on a string match into host_info property, in status output.#1656
rolandwalker merged 1 commit intomainfrom
RW/dont-depend-on-string-match-into-host-info

Conversation

@rolandwalker
Copy link
Contributor

Description

Detecting the unix_socket property should be more robust.

Same as #1598 .

Checklist

  • I added this contribution to the changelog.md file.
  • I added my name to the AUTHORS file (or it's already there).
  • To lint and format the code, I ran
    uv run ruff check && uv run ruff format && uv run mypy --install-types .

@rolandwalker rolandwalker self-assigned this Feb 28, 2026
@github-actions
Copy link

  1. Correctness regression: TCP connections can be misclassified as Unix-socket connections.
    In mycli/packages/special/dbcommands.py:150, the new condition uses getattr(cur.connection, "unix_socket", None) is not None. In this codebase, TCP paths often pass socket="" (empty string), which is not None, so this branch incorrectly prints UNIX socket and skips TCP port.
    Action: switch to a truthiness check (for example if getattr(cur.connection, "unix_socket", None):) or normalize empty string to None before branching.

  2. Missing regression test: no test covers status() with unix_socket="" (TCP connection).
    Action: add a unit test in test/test_dbspecial.py that mocks cur.connection.unix_socket = "" and verifies status() includes TCP port (and not UNIX socket).

No new security concerns identified in this diff.

I couldn’t run tests locally because pytest is not installed in this environment.

@rolandwalker rolandwalker force-pushed the RW/dont-depend-on-string-match-into-host-info branch 2 times, most recently from 64913cb to 2587266 Compare February 28, 2026 14:30
Copy link
Contributor

@scottnemes scottnemes left a comment

Choose a reason for hiding this comment

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

Verified tcp/socket status output works as expected

property, in "status" output.

Detecting the "unix_socket" property should be more robust.
@rolandwalker rolandwalker force-pushed the RW/dont-depend-on-string-match-into-host-info branch from 2587266 to becc670 Compare February 28, 2026 23:15
@rolandwalker rolandwalker merged commit 7e7b495 into main Feb 28, 2026
8 checks passed
@rolandwalker rolandwalker deleted the RW/dont-depend-on-string-match-into-host-info branch February 28, 2026 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants