Skip to content

Decode text results defensively when the client encoding cannot decode them (SQL_ASCII) - #1628

Open
DiegoDAF wants to merge 1 commit into
dbcli:mainfrom
DiegoDAF:pr-sql-ascii-clean
Open

Decode text results defensively when the client encoding cannot decode them (SQL_ASCII)#1628
DiegoDAF wants to merge 1 commit into
dbcli:mainfrom
DiegoDAF:pr-sql-ascii-clean

Conversation

@DiegoDAF

@DiegoDAF DiegoDAF commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Fixes #1518. Fixes #1484.

When the client encoding cannot decode a text value (most commonly SQL_ASCII), psycopg returns text columns as raw bytes instead of str. Three code paths were passing those values through unguarded, producing the crashes reported in #1518 and #1484:

  • The socket directory query result is used to build the prompt, so a Unix socket connection raised TypeError: expected str, not bytes.
  • The show time zone result is printed in the startup message, showing a b'...'-prefixed value.
  • The function metadata rows feed the background completion refresher; parsing their bytes with parse_defaults() killed the refresh thread with TypeError: can only concatenate str (not "int") to str.

The fix decodes those values defensively when they come back as bytes, with the same guard the completion metadata fix for #1405 already uses (decode("utf-8", "replace")): a _decode_if_bytes() helper, plus a _decode_row() variant for the function metadata rows, which contain array columns.

Deliberately narrow in scope: function_definition(), search_path(), schemata() and databases() also return text and are still unguarded, but they are not involved in the reported crashes. Guarding them can be a follow-up if anyone hits them.

Reproduced against a real SQL_ASCII cluster (3396 catalog functions; all metadata fields come back as plain str after the fix). 6 unit tests: 3 fail without the fix, 3 pin the already-working str path so it stays unchanged.

Checklist

  • I've added this contribution to the changelog.rst.
  • I've added my name to the AUTHORS file (or it's already there).
  • I installed pre-commit hooks (pip install pre-commit && pre-commit install).
  • Please squash merge this pull request (uncheck if you'd like us to merge as multiple commits)

Sorry about the missing checklist, that was me writing the description by hand instead of starting from the template.

The third box is honestly unchecked, with a small finding attached: .pre-commit-config.yaml pins ruff-pre-commit at v0.11.7, while [testenv:style] in tox.ini installs ruff unpinned (0.15.x today). The two disagree on formatting, so running the hook reformats code that the CI style job then wants reformatted back. I ran ruff check and ruff format matching the CI instead, both clean. Happy to open a separate PR bumping the pre-commit rev if you would like them to agree.

function metadata

When the client encoding cannot decode a text value (most commonly
SQL_ASCII), psycopg returns text columns as raw bytes. Decode those
values defensively, using the same guard as the completion metadata
fix for dbcli#1405:

- get_socket_directory() and get_timezone() now decode their results,
  so the prompt no longer raises a TypeError on Unix socket
  connections and the timezone startup message no longer shows a
  b'...' value.
- functions() now decodes every metadata row before yielding, so the
  background completion refresh no longer dies in parse_defaults with
  a TypeError.

Closes dbcli#1484 and dbcli#1518. Related: dbcli#1405.
@j-bennet

j-bennet commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@DiegoDAF nit: the PR checklist may seem boilerplate, but we prefer to have it. It gives us some confidence that the contributor familiarized themselves with our requirements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants