From f9a06683fecf01ee2f4478cdc36a12ca5b2db94e Mon Sep 17 00:00:00 2001 From: Peter Eisentraut 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