Skip to content

fix: handle bytes/string mismatch in replace at startup #1577

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bobo-xxx wants to merge 1 commit into
base: main
Choose a base branch
from
+17 −12
Open

fix: handle bytes/string mismatch in replace at startup #1577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 15 additions & 12 deletions pgcli/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1301,21 +1301,24 @@ def get_completions(self, text, cursor_positition):
return self.completer.get_completions(Document(text=text, cursor_position=cursor_positition), None)

def get_prompt(self, string):
# should be before replacing \\d
string = string.replace("\\dsn_alias", self.dsn_alias or "")
string = string.replace("\\t", self.now.strftime("%x %X"))
string = string.replace("\\u", self.pgexecute.user or "(none)")
string = string.replace("\\H", self.pgexecute.host or "(none)")
string = string.replace("\\h", self.pgexecute.short_host or "(none)")
string = string.replace("\\d", self.pgexecute.dbname or "(none)")
# should be before replacing \d
def _to_str(val):
return val.decode("utf-8") if isinstance(val, bytes) else val or ""
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

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

New bytes->str handling in get_prompt() is important for startup stability, but there are existing prompt-format tests that only cover str values. Please add a regression test where pgexecute.user/host/dbname (and/or transaction_indicator) are bytes to ensure the TypeError is fixed and stays fixed (e.g., cli.get_prompt("\\u@\\h") works with byte inputs).

Suggested change
return val.decode("utf-8") if isinstance(val, bytes) else val or ""
if not val:
return ""
if isinstance(val, bytes):
return val.decode("utf-8")
if isinstance(val, str):
return val
return str(val)

Copilot uses AI. Check for mistakes.

string = string.replace("\dsn_alias", self.dsn_alias or "")
string = string.replace("\t", self.now.strftime("%x %X"))
string = string.replace("\u", _to_str(self.pgexecute.user) or "(none)")
string = string.replace("\H", _to_str(self.pgexecute.host) or "(none)")
string = string.replace("\h", _to_str(self.pgexecute.short_host) or "(none)")
string = string.replace("\d", _to_str(self.pgexecute.dbname) or "(none)")
Comment on lines +1308 to +1313
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

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

The replacement tokens were changed from literal backslash sequences (e.g. "\u", "\H") to Python escape sequences (e.g. "\u", "\t"). This breaks prompt substitution and also makes the file invalid: "\u" is parsed as a Unicode escape and will raise a SyntaxError at import time. Use escaped backslashes in the string literals (e.g. "\u", "\H", "\t", "\d", "\dsn_alias") so the function matches the prompt format tokens used elsewhere (see tests using "\T\u").

Copilot uses AI. Check for mistakes.
string = string.replace(
"\\p",
"\p",
str(self.pgexecute.port) if self.pgexecute.port is not None else "5432",
)
string = string.replace("\\i", str(self.pgexecute.pid) or "(none)")
string = string.replace("\\#", "#" if self.pgexecute.superuser else ">")
string = string.replace("\\n", "\n")
string = string.replace("\\T", self.pgexecute.transaction_indicator)
string = string.replace("\i", str(self.pgexecute.pid) or "(none)")
string = string.replace("\#", "#" if self.pgexecute.superuser else ">")
string = string.replace("\n", "\n")
string = string.replace("\T", _to_str(self.pgexecute.transaction_indicator))
Comment on lines 1314 to +1321
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

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

More prompt tokens were switched from literal backslash sequences to escape sequences ("\p", "\i", "#", "\T", and especially "\n"). For example, "\n" is a newline character, so string.replace("\n", "\n") is effectively a no-op and will no longer replace the "\n" token in prompt formats. These should remain literal tokens ("\p", "\i", "\#", "\n", "\T") to preserve existing behavior and keep tests passing.

Copilot uses AI. Check for mistakes.
return string

def get_last_query(self):
Expand Down
2 changes: 2 additions & 0 deletions pgcli/pgcompleter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ def __init__(self, smart_completion=True, pgspecial=None, settings=None):
self.all_completions = set(self.keywords + self.functions)

def escape_name(self, name):
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

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

escape_name() now accepts bytes, but there are no tests covering this path. Please add a unit test asserting that a bytes identifier (e.g. b"my_schema" and a name that requires quoting) is decoded and handled correctly, preventing the TypeError: cannot use a string pattern on a bytes-like object regression.

Suggested change
def escape_name(self, name):
def escape_name(self, name):
"""Quote an identifier when needed.
Regression tests for bytes input:
>>> completer = PGCompleter()
>>> completer.escape_name(b"my_schema")
'my_schema'
>>> completer.escape_name(b"needs space")
'"needs space"'
"""

Copilot uses AI. Check for mistakes.
if isinstance(name, bytes):
name = name.decode("utf-8")
if name and ((not self.name_pattern.match(name)) or (name.upper() in self.reserved_words) or (name.upper() in self.functions)):
name = '"%s"' % name

Expand Down
Loading