On Thu, Mar 6, 2025 at 12:49 AM Mahendra Singh Thalor <mahi6...@gmail.com> wrote: > > Thanks Alvaro for feedback and review. > > On Wed, 5 Mar 2025 at 20:42, Álvaro Herrera <alvhe...@alvh.no-ip.org> wrote: > > > > Disclaimer: I didn't review these patches fully. > > > > On 2025-Mar-05, Mahendra Singh Thalor wrote: > > > > > On Wed, 5 Mar 2025 at 01:02, Álvaro Herrera <alvhe...@alvh.no-ip.org> > > > wrote: > > > > > > > A database name containing a newline breaks things for this patch: > > > > > > > > CREATE DATABASE "foo > > > > bar"; > > > > > I also reported this issue on 29-01-2025. This breaks even without this > > > patch also. > > > > Okay, we should probably fix that, but I think the new map.dat file your > > patch adds is going to make the problem worse, because it doesn't look > > like you handled that case in any particular way that would make it not > > fail. I think it would be good to avoid digging us up even deeper in > > that hole. More generally, the pg_upgrade tests contain some code to > > try database names with almost all possible ascii characters (see > > generate_db in pg_upgrade/t/002_pg_upgrade.pl); it would be good to > > ensure that this new functionality also works correctly for that -- > > perhaps add an equivalent test to the pg_dumpall test suite. > > In the attached patch, I tried to solve the problem of the map.dat > file. I will do more analysis based on dbnames in 002_pg_upgrade.pl > file. >
hi.
/*
* Append the given string to the shell command being built in the buffer,
* with shell-style quoting as needed to create exactly one argument.
*
* Forbid LF or CR characters, which have scant practical use beyond designing
* security breaches. The Windows command shell is unusable as a conduit for
* arguments containing LF or CR characters. A future major release should
* reject those characters in CREATE ROLE and CREATE DATABASE, because use
* there eventually leads to errors here.
*
* appendShellString() simply prints an error and dies if LF or CR appears.
* appendShellStringNoError() omits those characters from the result, and
* returns false if there were any.
*/
void
appendShellString(PQExpBuffer buf, const char *str)
per above comments,
we need to disallow LF/CR in database name and role name when issuing
shell command.
rolename LF/CR issue already being handled in
src/bin/pg_dump/pg_dumpall.c: while(getopt_long) code:
case 3:
use_role = pg_strdup(optarg);
appendPQExpBufferStr(pgdumpopts, " --role ");
appendShellString(pgdumpopts, use_role);
we can fail earlier also for database names in dumpDatabases, right
after executeQuery.
Please check attached, which is based on *v20*.
in V21, src/bin/pg_dump/pg_dumpall.c:
+#include "common_dumpall_restore.h"
happened within v21-0001 and v21-0002, it is being included twice.
v20-0001-pg_dumpall-deal-witth-newline-or-carriage-ret.no-cfbot
Description: Binary data
