Thread

Avoid corrupting DefElem nodes when parsing publication_names and publish options

sunil s <sunilfeb26@gmail.com> — 2025-12-24T11:25:12Z

Hello Hackers,

I noticed that two call sites of SplitIdentifierString() were not
following the API contract documented in varlena.c, which states:

   - rawstring: the input string; must be overwritable! On return, it's

               been modified to contain the separated identifiers.

   - namelist: filled with a palloc'd list of pointers to identifiers within

              rawstring.

The function modifies the input string in-place (replacing separators
with null terminators) and returns a list containing pointers directly
into that modified string.
The two problematic call sites were:
1. parse_publication_options() in publicationcmds.c - passed the result
   of defGetString() directly without copying.
2. pgoutput_startup() in pgoutput.c - passed strVal(defel->arg) directly
   without copying.
All other callers of SplitIdentifierString() in the codebase correctly
use pstrdup() or equivalent (like text_to_cstring()) before calling
the function.
While these two cases happen to work in practice because:
- In publicationcmds.c, the parsed list is used immediately within the
  same loop iteration and then discarded
- In pgoutput.c, the DefElem nodes remain valid for the connection lifetime
...it's still incorrect to rely on this, and the code would break if
someone later tried to use the results after freeing the options list,
or if the memory management changed.
The attached patch adds pstrdup() to both call sites for consistency
and correctness.

Thanks,
Sunil Seetharama