These functions are used in psql to assemble tables to print.
They would sometimes overwrite the string argument they are passed, namely via mbvalidate(), which removes invalid UTF-8 characters (or potentially analogously in other encodings, but that is not implemented). However, many callers are not expecting that. In many callers, the input value comes directly from libpq structures, such as from PQgetvalue() or PQsslAttribute(). The latter actually has a const char * return type, and that was cast away. But even the former is not expecting its return value to be modified.
Fix that by making these arguments const. Internally, we add a separate function that does only the checking part of mbvalidate(). Only if the validation returns a negative result, we make a copy and run mbvalidate() on the copy. printTableAddCell() already had internal infrastructure for keeping track of what values needed to be freed. We add the same for printTableAddHeader().
In passing, also simplify the code a bit. There were essentially duplicate mechanisms for keeping track of the most recently added cell (fields .cell and .cellsadded). Make that consistent by using an integer counter for everything. That makes the code arguably easier to read than with the "current pointer" approaches.
The first three patches are preparation patches to further clean up the nearby code a bit.
From 3ecde93fd9cfe1fcdbec0f678311c945d783339b Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Tue, 4 Aug 2026 09:44:53 +0200 Subject: [PATCH 1/4] Remove useless/confusing const qualifiers The const qualifiers removed here did not qualify what is being pointed to, which is the normal use in function prototypes. Instead, they qualify the variable itself, so that it cannot be changed inside the function. That is itself not wrong, but it's not a normal style, and it seems distracting here. --- src/fe_utils/print.c | 18 +++++++++--------- src/include/fe_utils/print.h | 18 +++++++++--------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c index 006c026294b..3cb2c5341e8 100644 --- a/src/fe_utils/print.c +++ b/src/fe_utils/print.c @@ -3206,8 +3206,8 @@ ClosePager(FILE *pagerpipe) * table. */ void -printTableInit(printTableContent *const content, const printTableOpt *opt, - const char *title, const int ncolumns, const int nrows) +printTableInit(printTableContent *content, const printTableOpt *opt, + const char *title, int ncolumns, int nrows) { uint64 total_cells; @@ -3254,8 +3254,8 @@ printTableInit(printTableContent *const content, const printTableOpt *opt, * column. */ void -printTableAddHeader(printTableContent *const content, char *header, - const bool translate, const char align) +printTableAddHeader(printTableContent *content, char *header, + bool translate, char align) { #ifndef ENABLE_NLS (void) translate; /* unused parameter */ @@ -3294,8 +3294,8 @@ printTableAddHeader(printTableContent *const content, char *header, * Note: Automatic freeing of translatable strings is not supported. */ void -printTableAddCell(printTableContent *const content, char *cell, - const bool translate, const bool mustfree) +printTableAddCell(printTableContent *content, char *cell, + bool translate, bool mustfree) { uint64 total_cells; @@ -3344,7 +3344,7 @@ printTableAddCell(printTableContent *const content, char *cell, * translated as a whole. */ void -printTableAddFooter(printTableContent *const content, const char *footer) +printTableAddFooter(printTableContent *content, const char *footer) { printTableFooter *f; @@ -3369,7 +3369,7 @@ printTableAddFooter(printTableContent *const content, const char *footer) * around. */ void -printTableSetFooter(printTableContent *const content, const char *footer) +printTableSetFooter(printTableContent *content, const char *footer) { if (content->footers != NULL) { @@ -3387,7 +3387,7 @@ printTableSetFooter(printTableContent *const content, const char *footer) * printTableInit() again. */ void -printTableCleanup(printTableContent *const content) +printTableCleanup(printTableContent *content) { if (content->cellmustfree) { diff --git a/src/include/fe_utils/print.h b/src/include/fe_utils/print.h index 94f6a593619..c437707d5c9 100644 --- a/src/include/fe_utils/print.h +++ b/src/include/fe_utils/print.h @@ -212,18 +212,18 @@ extern void ClosePager(FILE *pagerpipe); extern void html_escaped_print(const char *in, FILE *fout); -extern void printTableInit(printTableContent *const content, +extern void printTableInit(printTableContent *content, const printTableOpt *opt, const char *title, - const int ncolumns, const int nrows); -extern void printTableAddHeader(printTableContent *const content, - char *header, const bool translate, const char align); -extern void printTableAddCell(printTableContent *const content, - char *cell, const bool translate, const bool mustfree); -extern void printTableAddFooter(printTableContent *const content, + int ncolumns, int nrows); +extern void printTableAddHeader(printTableContent *content, + char *header, bool translate, char align); +extern void printTableAddCell(printTableContent *content, + char *cell, bool translate, bool mustfree); +extern void printTableAddFooter(printTableContent *content, const char *footer); -extern void printTableSetFooter(printTableContent *const content, +extern void printTableSetFooter(printTableContent *content, const char *footer); -extern void printTableCleanup(printTableContent *const content); +extern void printTableCleanup(printTableContent *content); extern void printTable(const printTableContent *cont, FILE *fout, bool is_pager, FILE *flog); extern void printQuery(const PGresult *result, const printQueryOpt *opt, base-commit: fef160d8b0ddf72e1089133300d30109293f5a71 -- 2.55.0
From c1c037b5a4011535bccaa5bf654b6ab233998b24 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Tue, 4 Aug 2026 09:44:53 +0200 Subject: [PATCH 2/4] Remove useless ENABLE_NLS conditionals This already works even without those conditionals, since _() expands to a no-op if NLS is disabled. --- src/fe_utils/print.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c index 3cb2c5341e8..87481008c74 100644 --- a/src/fe_utils/print.c +++ b/src/fe_utils/print.c @@ -3257,10 +3257,6 @@ void printTableAddHeader(printTableContent *content, char *header, bool translate, char align) { -#ifndef ENABLE_NLS - (void) translate; /* unused parameter */ -#endif - if (content->header >= content->headers + content->ncolumns) { fprintf(stderr, _("Cannot add header to table content: " @@ -3271,10 +3267,9 @@ printTableAddHeader(printTableContent *content, char *header, *content->header = (char *) mbvalidate((unsigned char *) header, content->opt->encoding); -#ifdef ENABLE_NLS if (translate) *content->header = _(*content->header); -#endif + content->header++; *content->align = align; @@ -3299,10 +3294,6 @@ printTableAddCell(printTableContent *content, char *cell, { uint64 total_cells; -#ifndef ENABLE_NLS - (void) translate; /* unused parameter */ -#endif - total_cells = (uint64) content->ncolumns * content->nrows; if (content->cellsadded >= total_cells) { @@ -3314,10 +3305,8 @@ printTableAddCell(printTableContent *content, char *cell, *content->cell = (char *) mbvalidate((unsigned char *) cell, content->opt->encoding); -#ifdef ENABLE_NLS if (translate) *content->cell = _(*content->cell); -#endif if (mustfree) { -- 2.55.0
From b5d78e378aba0b212e7ca407af715fcc9b28caf0 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Tue, 4 Aug 2026 09:44:53 +0200 Subject: [PATCH 3/4] Use frontend logging API in fe_utils/print.c This simplifies error message printing a bit. Also adjust the error messages a bit for style, while we're here. --- src/fe_utils/print.c | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c index 87481008c74..ae7b365616c 100644 --- a/src/fe_utils/print.c +++ b/src/fe_utils/print.c @@ -30,6 +30,7 @@ #endif #include "catalog/pg_type_d.h" +#include "common/logging.h" #include "fe_utils/mbprint.h" #include "fe_utils/print.h" @@ -3219,14 +3220,12 @@ printTableInit(printTableContent *content, const printTableOpt *opt, content->headers = pg_malloc0_array(const char *, (ncolumns + 1)); total_cells = (uint64) ncolumns * nrows; + /* Catch possible overflow. Using >= here allows adding 1 below */ if (total_cells >= SIZE_MAX / sizeof(*content->cells)) - { - fprintf(stderr, _("Cannot print table contents: number of cells %" PRIu64 " is equal to or exceeds maximum %zu.\n"), - total_cells, - SIZE_MAX / sizeof(*content->cells)); - exit(EXIT_FAILURE); - } + pg_fatal("cannot print table contents: number of cells %" PRIu64 " is equal to or exceeds maximum %zu", + total_cells, SIZE_MAX / sizeof(*content->cells)); + content->cells = pg_malloc0_array(const char *, (total_cells + 1)); content->cellmustfree = NULL; @@ -3258,12 +3257,8 @@ printTableAddHeader(printTableContent *content, char *header, bool translate, char align) { if (content->header >= content->headers + content->ncolumns) - { - fprintf(stderr, _("Cannot add header to table content: " - "column count of %d exceeded.\n"), - content->ncolumns); - exit(EXIT_FAILURE); - } + pg_fatal("cannot add header to table content: column count of %d exceeded", + content->ncolumns); *content->header = (char *) mbvalidate((unsigned char *) header, content->opt->encoding); @@ -3295,12 +3290,10 @@ printTableAddCell(printTableContent *content, char *cell, uint64 total_cells; total_cells = (uint64) content->ncolumns * content->nrows; + if (content->cellsadded >= total_cells) - { - fprintf(stderr, _("Cannot add cell to table content: total cell count of %" PRIu64 " exceeded.\n"), - total_cells); - exit(EXIT_FAILURE); - } + pg_fatal("cannot add cell to table content: total cell count of %" PRIu64 " exceeded", + total_cells); *content->cell = (char *) mbvalidate((unsigned char *) cell, content->opt->encoding); @@ -3723,9 +3716,8 @@ printTable(const printTableContent *cont, print_troff_ms_text(cont, fout); break; default: - fprintf(stderr, _("invalid output format (internal error): %d"), - cont->opt->format); - exit(EXIT_FAILURE); + pg_fatal_internal("invalid output format (internal error): %d", + cont->opt->format); } if (is_local_pager) -- 2.55.0
From f9a06683fecf01ee2f4478cdc36a12ca5b2db94e Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Tue, 4 Aug 2026 11:28:09 +0200 Subject: [PATCH 4/4] Make printTableAddCell/printTableAddHeader string argument const These functions would sometimes overwrite the string argument they are passed, namely via mbvalidate(), which removes invalid UTF-8 characters (or potentially analogously in other encodings, but that is not implemented). However, many callers are not expecting that. In many callers, the input value comes directly from libpq structures, such as from PQgetvalue() or PQsslAttribute(). The latter actually has a const char * return type, and that was cast away. But even the former is not expecting its return value to be modified. Fix that by making these arguments const. Internally, we add a separate function that does only the checking part of mbvalidate(). Only if the validation returns a negative result, we make a copy and run mbvalidate() on the copy. printTableAddCell() already had internal infrastructure for keeping track of what values needed to be freed. We add the same for printTableAddHeader(). In passing, also simplify the code a bit. There were essentially duplicate mechanisms for keeping track of the most recently added cell (fields .cell and .cellsadded). Make that consistent by using an integer counter for everything. That makes the code arguably easier to read than with the "current pointer" approaches. --- src/bin/psql/command.c | 20 ++++---- src/fe_utils/mbprint.c | 33 +++++++++++++ src/fe_utils/print.c | 88 ++++++++++++++++++++++++++-------- src/include/fe_utils/mbprint.h | 1 + src/include/fe_utils/print.h | 11 ++--- 5 files changed, 116 insertions(+), 37 deletions(-) diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index ee85c05a00d..2170cab0f30 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -800,7 +800,7 @@ exec_command_conninfo(PsqlScanState scan_state, bool active_branch) password_used, gssapi_used; int version_num; - char *paramval; + const char *paramval; if (!active_branch) return PSQL_CMD_SKIP_LINE; @@ -905,19 +905,19 @@ exec_command_conninfo(PsqlScanState scan_state, bool active_branch) /* SSL Information */ if (ssl_in_use) { - char *library, + const char *library, *protocol, *key_bits, *cipher, *compression, *alpn; - library = (char *) PQsslAttribute(pset.db, "library"); - protocol = (char *) PQsslAttribute(pset.db, "protocol"); - key_bits = (char *) PQsslAttribute(pset.db, "key_bits"); - cipher = (char *) PQsslAttribute(pset.db, "cipher"); - compression = (char *) PQsslAttribute(pset.db, "compression"); - alpn = (char *) PQsslAttribute(pset.db, "alpn"); + library = PQsslAttribute(pset.db, "library"); + protocol = PQsslAttribute(pset.db, "protocol"); + key_bits = PQsslAttribute(pset.db, "key_bits"); + cipher = PQsslAttribute(pset.db, "cipher"); + compression = PQsslAttribute(pset.db, "compression"); + alpn = PQsslAttribute(pset.db, "alpn"); printTableAddCell(&cont, _("SSL Library"), false, false); printTableAddCell(&cont, library ? library : _("unknown"), false, false); @@ -939,11 +939,11 @@ exec_command_conninfo(PsqlScanState scan_state, bool active_branch) printTableAddCell(&cont, (alpn && alpn[0] != '\0') ? alpn : _("none"), false, false); } - paramval = (char *) PQparameterStatus(pset.db, "is_superuser"); + paramval = PQparameterStatus(pset.db, "is_superuser"); printTableAddCell(&cont, "Superuser", false, false); printTableAddCell(&cont, paramval ? paramval : _("unknown"), false, false); - paramval = (char *) PQparameterStatus(pset.db, "in_hot_standby"); + paramval = PQparameterStatus(pset.db, "in_hot_standby"); printTableAddCell(&cont, "Hot Standby", false, false); printTableAddCell(&cont, paramval ? paramval : _("unknown"), false, false); diff --git a/src/fe_utils/mbprint.c b/src/fe_utils/mbprint.c index dbfa1cab597..882213b9e5d 100644 --- a/src/fe_utils/mbprint.c +++ b/src/fe_utils/mbprint.c @@ -164,6 +164,24 @@ mb_utf_validate(unsigned char *pwcs) *p = '\0'; } + +static bool +mb_utf_is_valid(const unsigned char *pwcs) +{ + while (*pwcs) + { + int len; + + if ((len = utf_charcheck(pwcs)) > 0) + pwcs += len; + else + return false; + + } + return true; +} + + /* * public functions : wcswidth and mbvalidate */ @@ -403,3 +421,18 @@ mbvalidate(unsigned char *pwcs, int encoding) return pwcs; } + +bool +mb_is_valid(const unsigned char *pwcs, int encoding) +{ + if (encoding == PG_UTF8) + return mb_utf_is_valid(pwcs); + else + { + /* + * other encodings needing validation should add their own routines + * here + */ + return true; + } +} diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c index ae7b365616c..4d9b12a7e1c 100644 --- a/src/fe_utils/print.c +++ b/src/fe_utils/print.c @@ -3218,6 +3218,8 @@ printTableInit(printTableContent *content, const printTableOpt *opt, content->nrows = nrows; content->headers = pg_malloc0_array(const char *, (ncolumns + 1)); + content->headersadded = 0; + content->headermustfree = NULL; total_cells = (uint64) ncolumns * nrows; @@ -3227,17 +3229,14 @@ printTableInit(printTableContent *content, const printTableOpt *opt, total_cells, SIZE_MAX / sizeof(*content->cells)); content->cells = pg_malloc0_array(const char *, (total_cells + 1)); - + content->cellsadded = 0; content->cellmustfree = NULL; + content->footers = NULL; content->aligns = pg_malloc0_array(char, (ncolumns + 1)); - content->header = content->headers; - content->cell = content->cells; content->footer = content->footers; - content->align = content->aligns; - content->cellsadded = 0; } /* @@ -3253,22 +3252,45 @@ printTableInit(printTableContent *content, const printTableOpt *opt, * column. */ void -printTableAddHeader(printTableContent *content, char *header, +printTableAddHeader(printTableContent *content, const char *header, bool translate, char align) { - if (content->header >= content->headers + content->ncolumns) + bool mustfree = false; + + if (content->headersadded >= content->ncolumns) pg_fatal("cannot add header to table content: column count of %d exceeded", content->ncolumns); - *content->header = (char *) mbvalidate((unsigned char *) header, - content->opt->encoding); if (translate) - *content->header = _(*content->header); + header = _(header); - content->header++; + /* + * Note: Translated strings are not checked for encoding validity. These + * are provided by ourselves, so they had better be ok. And if they were + * not, running mbvalidate on them could overwrite gettext-owned memory. + */ + if (!translate && !mb_is_valid((unsigned char *) header, content->opt->encoding)) + { + char *header2; - *content->align = align; - content->align++; + header2 = pg_strdup(header); + header = (char *) mbvalidate((unsigned char *) header2, content->opt->encoding); + mustfree = true; + } + + content->headers[content->headersadded] = header; + content->aligns[content->headersadded] = align; + + if (mustfree) + { + if (content->headermustfree == NULL) + content->headermustfree = + pg_malloc0_array(bool, (content->ncolumns + 1)); + + content->headermustfree[content->headersadded] = true; + } + + content->headersadded++; } /* @@ -3284,7 +3306,7 @@ printTableAddHeader(printTableContent *content, char *header, * Note: Automatic freeing of translatable strings is not supported. */ void -printTableAddCell(printTableContent *content, char *cell, +printTableAddCell(printTableContent *content, const char *cell, bool translate, bool mustfree) { uint64 total_cells; @@ -3295,11 +3317,26 @@ printTableAddCell(printTableContent *content, char *cell, pg_fatal("cannot add cell to table content: total cell count of %" PRIu64 " exceeded", total_cells); - *content->cell = (char *) mbvalidate((unsigned char *) cell, - content->opt->encoding); + Assert(!(translate && mustfree)); if (translate) - *content->cell = _(*content->cell); + cell = _(cell); + + /* + * Note: Translated strings are not checked for encoding validity. These + * are provided by ourselves, so they had better be ok. And if they were + * not, running mbvalidate on them could overwrite gettext-owned memory. + */ + if (!translate && !mb_is_valid((unsigned char *) cell, content->opt->encoding)) + { + char *cell2; + + cell2 = pg_strdup(cell); + cell = (char *) mbvalidate((unsigned char *) cell2, content->opt->encoding); + mustfree = true; + } + + content->cells[content->cellsadded] = cell; if (mustfree) { @@ -3309,7 +3346,7 @@ printTableAddCell(printTableContent *content, char *cell, content->cellmustfree[content->cellsadded] = true; } - content->cell++; + content->cellsadded++; } @@ -3371,6 +3408,16 @@ printTableSetFooter(printTableContent *content, const char *footer) void printTableCleanup(printTableContent *content) { + if (content->headermustfree) + { + for (uint64 i = 0; i < content->ncolumns; i++) + { + if (content->headermustfree[i]) + free(unconstify(char *, content->headers[i])); + } + free(content->headermustfree); + content->headermustfree = NULL; + } if (content->cellmustfree) { uint64 total_cells; @@ -3393,9 +3440,8 @@ printTableCleanup(printTableContent *content) content->headers = NULL; content->cells = NULL; content->aligns = NULL; - content->header = NULL; - content->cell = NULL; - content->align = NULL; + content->headersadded = 0; + content->cellsadded = 0; if (content->footers) { diff --git a/src/include/fe_utils/mbprint.h b/src/include/fe_utils/mbprint.h index c5e9c8ed7e2..ba4820b6253 100644 --- a/src/include/fe_utils/mbprint.h +++ b/src/include/fe_utils/mbprint.h @@ -20,6 +20,7 @@ struct lineptr }; extern unsigned char *mbvalidate(unsigned char *pwcs, int encoding); +extern bool mb_is_valid(const unsigned char *pwcs, int encoding); extern int pg_wcswidth(const char *pwcs, size_t len, int encoding); extern void pg_wcsformat(const unsigned char *pwcs, size_t len, int encoding, struct lineptr *lines, int count); diff --git a/src/include/fe_utils/print.h b/src/include/fe_utils/print.h index c437707d5c9..138ec2f2007 100644 --- a/src/include/fe_utils/print.h +++ b/src/include/fe_utils/print.h @@ -167,17 +167,16 @@ typedef struct printTableContent int ncolumns; /* Specified in Init() */ int nrows; /* Specified in Init() */ const char **headers; /* NULL-terminated array of header strings */ - const char **header; /* Pointer to the last added header */ + uint64 headersadded; /* Number of headers added this far */ + bool *headermustfree; /* true for headers that need to be free()d */ const char **cells; /* NULL-terminated array of cell content * strings */ - const char **cell; /* Pointer to the last added cell */ uint64 cellsadded; /* Number of cells added this far */ bool *cellmustfree; /* true for cells that need to be free()d */ printTableFooter *footers; /* Pointer to the first footer */ printTableFooter *footer; /* Pointer to the last added footer */ char *aligns; /* Array of alignment specifiers; 'l' or 'r', - * one per column */ - char *align; /* Pointer to the last added alignment */ + * one per column; counted by headersadded */ } printTableContent; typedef struct printQueryOpt @@ -216,9 +215,9 @@ extern void printTableInit(printTableContent *content, const printTableOpt *opt, const char *title, int ncolumns, int nrows); extern void printTableAddHeader(printTableContent *content, - char *header, bool translate, char align); + const char *header, bool translate, char align); extern void printTableAddCell(printTableContent *content, - char *cell, bool translate, bool mustfree); + const char *cell, bool translate, bool mustfree); extern void printTableAddFooter(printTableContent *content, const char *footer); extern void printTableSetFooter(printTableContent *content, -- 2.55.0
