MIME-Version: 1.0
References: 
 <CAKYtNAof2Rj0a7SZ3zCWQwNJSsciuPxQdJ4GAsX8GqDVKgf+9A@mail.gmail.com>
 <202503051512.tr7vcirldfns@alvherre.pgsql>
 <CAKYtNAp+GqP66ORsihV+esFLG7hg9q6AvRNcJikELwVjezTW2Q@mail.gmail.com>
In-Reply-To: 
 <CAKYtNAp+GqP66ORsihV+esFLG7hg9q6AvRNcJikELwVjezTW2Q@mail.gmail.com>
From: jian he <jian.universality@gmail.com>
Date: Mon, 10 Mar 2025 15:54:45 +0800
Message-ID: 
 <CACJufxGYwhnaKk3X0uAjq4WbNym7k7axXAk95PhHNpxCV+cFAA@mail.gmail.com>
Subject: Re: Non-text mode for pg_dumpall
To: Mahendra Singh Thalor <mahi6run@gmail.com>
Cc: =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@alvh.no-ip.org>,
	Srinath Reddy <srinath2133@gmail.com>, pgsql-hackers@lists.postgresql.org
Content-Type: multipart/mixed; boundary="0000000000006627db062ff84b88"
Archived-At: 
 <https://www.postgresql.org/message-id/CACJufxGYwhnaKk3X0uAjq4WbNym7k7axXAk95PhHNpxCV%2BcFAA%40mail.gmail.com>
Precedence: bulk

--0000000000006627db062ff84b88
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Mar 6, 2025 at 12:49=E2=80=AFAM Mahendra Singh Thalor
<mahi6run@gmail.com> wrote:
>
> Thanks Alvaro for feedback and review.
>
> On Wed, 5 Mar 2025 at 20:42, =C3=81lvaro Herrera <alvherre@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, =C3=81lvaro Herrera <alvherre@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 th=
is
> > > patch also.
> >
> > Okay, we should probably fix that, but I think the new map.dat file you=
r
> > 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 design=
ing
 * security breaches.  The Windows command shell is unusable as a conduit f=
or
 * 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 =3D 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.

--0000000000006627db062ff84b88
Content-Type: application/octet-stream;
	name="v20-0001-pg_dumpall-deal-witth-newline-or-carriage-ret.no-cfbot"
Content-Disposition: attachment;
	filename="v20-0001-pg_dumpall-deal-witth-newline-or-carriage-ret.no-cfbot"
Content-Transfer-Encoding: base64
Content-ID: <f_m82ro8qm0>
X-Attachment-Id: f_m82ro8qm0

RnJvbSBiZjBkYjNhZmI5Y2I3ZDM0MDMzZmU1MDAzMTViZTgwMzNlZmZmNDllIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBqaWFuIGhlIDxqaWFuLnVuaXZlcnNhbGl0eUBnbWFpbC5jb20+
CkRhdGU6IE1vbiwgMTAgTWFyIDIwMjUgMTU6Mjc6MzIgKzA4MDAKU3ViamVjdDogW1BBVENIIHYy
MCAxLzFdIHBnX2R1bXBhbGwgZGVhbCB3aXR0aCBuZXdsaW5lIG9yIGNhcnJpYWdlIHJldHVybgoK
cGdfZHVtcGFsbDogZmFpbCBlYXJsaWVyIGlmIGFueSBkYXRhYmFzZSBuYW1lIGNvbnRhaW4gbmV3
IGxpbmUuCndlIG1heSBhbHNvIG5lZWQgZGVhbCB3aXRoIHJvbGUgbmFtZSBoYXZlIG5ld2xpbmUg
b3IgY2FycmlhZ2UgcmV0dXJuLgoKYWxzbyBzZWUgY29tbWVudHMgaW4gYXBwZW5kU2hlbGxTdHJp
bmcuCi0tLQogc3JjL2Jpbi9wZ19kdW1wL2NvbW1vbl9kdW1wYWxsX3Jlc3RvcmUuYyB8IDE4ICsr
KysrKysrKysrKysrKysrKwogc3JjL2Jpbi9wZ19kdW1wL2NvbW1vbl9kdW1wYWxsX3Jlc3RvcmUu
aCB8ICAyICsrCiBzcmMvYmluL3BnX2R1bXAvcGdfZHVtcGFsbC5jICAgICAgICAgICAgIHwgMTMg
KysrKysrKysrKysrKwogMyBmaWxlcyBjaGFuZ2VkLCAzMyBpbnNlcnRpb25zKCspCgpkaWZmIC0t
Z2l0IGEvc3JjL2Jpbi9wZ19kdW1wL2NvbW1vbl9kdW1wYWxsX3Jlc3RvcmUuYyBiL3NyYy9iaW4v
cGdfZHVtcC9jb21tb25fZHVtcGFsbF9yZXN0b3JlLmMKaW5kZXggOTJmNTJiNzIzOWEuLjRlODEx
NDIzNzNmIDEwMDY0NAotLS0gYS9zcmMvYmluL3BnX2R1bXAvY29tbW9uX2R1bXBhbGxfcmVzdG9y
ZS5jCisrKyBiL3NyYy9iaW4vcGdfZHVtcC9jb21tb25fZHVtcGFsbF9yZXN0b3JlLmMKQEAgLTI4
NywzICsyODcsMjEgQEAgZXhlY3V0ZVF1ZXJ5KFBHY29ubiAqY29ubiwgY29uc3QgY2hhciAqcXVl
cnkpCiAKIAlyZXR1cm4gcmVzOwogfQorCisvKgorICogYXBwZW5kIHN0ciB0byBidWYsIGV4aXQg
aWYgc3RyaW5nIGNvbnRhaW4gbmV3bGluZSBvciBjYXJyaWFnZSByZXR1cm4KKyovCit2b2lkCitz
dHJpbmdfY29udGFpbl9sZmNyKFBRRXhwQnVmZmVyIGJ1ZiwgY29uc3QgY2hhciAqc3RyLCBjb25z
dCBjaGFyICpraW5kKQoreworCUFzc2VydChraW5kICE9IE5VTEwpOworCWlmICghYXBwZW5kU2hl
bGxTdHJpbmdOb0Vycm9yKGJ1Ziwgc3RyKSkKKwl7CisKKwkJcGdfbG9nX2Vycm9yKCIlcyBjb250
YWlucyBhIG5ld2xpbmUgb3IgY2FycmlhZ2UgcmV0dXJuOiBcIiVzXCIiLCBraW5kLCBzdHIpOwor
CQlwZ19sb2dfZXJyb3JfaGludCgiSWYgeW91IHdhbnQgdG8gZHVtcCBkYXRhIG9uIFwiJXNcIiwg
IgorCQkJCQkJICAieW91IG1heSBuZWVkIHJlbmFtZSBpdCB0byBtYWtlIHN1cmUgaXQgZG9lcyBj
b250YWluIG5ld2xpbmUgb3IgY2FycmlhZ2UgcmV0dXJuIiwKKwkJCQkJCSAgc3RyKTsKKwkJZXhp
dF9uaWNlbHkoMSk7CisJfQorfQpkaWZmIC0tZ2l0IGEvc3JjL2Jpbi9wZ19kdW1wL2NvbW1vbl9k
dW1wYWxsX3Jlc3RvcmUuaCBiL3NyYy9iaW4vcGdfZHVtcC9jb21tb25fZHVtcGFsbF9yZXN0b3Jl
LmgKaW5kZXggN2ZlMWMwMGFiNzEuLmM0NThhYmQwZWVmIDEwMDY0NAotLS0gYS9zcmMvYmluL3Bn
X2R1bXAvY29tbW9uX2R1bXBhbGxfcmVzdG9yZS5oCisrKyBiL3NyYy9iaW4vcGdfZHVtcC9jb21t
b25fZHVtcGFsbF9yZXN0b3JlLmgKQEAgLTE1LDEwICsxNSwxMiBAQAogI2RlZmluZSBDT01NT05f
RFVNUEFMTF9SRVNUT1JFX0gKIAogI2luY2x1ZGUgInBnX2JhY2t1cC5oIgorI2luY2x1ZGUgInBx
ZXhwYnVmZmVyLmgiCiAKIGV4dGVybiBQR2Nvbm4gKmNvbm5lY3REYXRhYmFzZShjb25zdCBjaGFy
ICpkYm5hbWUsIGNvbnN0IGNoYXIgKmNvbm5lY3Rpb25fc3RyaW5nLCBjb25zdCBjaGFyICpwZ2hv
c3QsCiAJCQkJCQkJICAgY29uc3QgY2hhciAqcGdwb3J0LCBjb25zdCBjaGFyICpwZ3VzZXIsCiAJ
CQkJCQkJICAgdHJpdmFsdWUgcHJvbXB0X3Bhc3N3b3JkLCBib29sIGZhaWxfb25fZXJyb3IsCiAJ
CQkJCQkJICAgY29uc3QgY2hhciAqcHJvZ25hbWUsIGNvbnN0IGNoYXIgKipjb25uc3RyLCBpbnQg
KnNlcnZlcl92ZXJzaW9uKTsKIGV4dGVybiAgUEdyZXN1bHQgKmV4ZWN1dGVRdWVyeShQR2Nvbm4g
KmNvbm4sIGNvbnN0IGNoYXIgKnF1ZXJ5KTsKK2V4dGVybiAgdm9pZCBzdHJpbmdfY29udGFpbl9s
ZmNyKFBRRXhwQnVmZmVyIGJ1ZiwgY29uc3QgY2hhciAqc3RyLCBjb25zdCBjaGFyICpraW5kKTsK
ICNlbmRpZiAgICAgICAgICAgICAgICAgICAgICAgICAgLyogQ09NTU9OX0RVTVBBTExfUkVTVE9S
RV9IICovCmRpZmYgLS1naXQgYS9zcmMvYmluL3BnX2R1bXAvcGdfZHVtcGFsbC5jIGIvc3JjL2Jp
bi9wZ19kdW1wL3BnX2R1bXBhbGwuYwppbmRleCAwNzUyYzQ0ODk2Zi4uZjY2MTRjZTNiYzIgMTAw
NjQ0Ci0tLSBhL3NyYy9iaW4vcGdfZHVtcC9wZ19kdW1wYWxsLmMKKysrIGIvc3JjL2Jpbi9wZ19k
dW1wL3BnX2R1bXBhbGwuYwpAQCAtMTYwMSw2ICsxNjAxLDcgQEAgZHVtcERhdGFiYXNlcyhQR2Nv
bm4gKmNvbm4sIEFyY2hpdmVGb3JtYXQgYXJjaER1bXBGb3JtYXQpCiAJY2hhcgkJZGJfc3ViZGly
W01BWFBHUEFUSF07CiAJY2hhcgkJZGJmaWxlcGF0aFtNQVhQR1BBVEhdOwogCUZJTEUgICAgICAg
Km1hcF9maWxlID0gTlVMTDsKKwlQUUV4cEJ1ZmZlckRhdGEgdGVzdF9kYm5hbWU7CiAKIAkvKgog
CSAqIFNraXAgZGF0YWJhc2VzIG1hcmtlZCBub3QgZGF0YWxsb3djb25uLCBzaW5jZSB3ZSdkIGJl
IHVuYWJsZSB0byBjb25uZWN0CkBAIC0xNjIyLDYgKzE2MjMsMTggQEAgZHVtcERhdGFiYXNlcyhQ
R2Nvbm4gKmNvbm4sIEFyY2hpdmVGb3JtYXQgYXJjaER1bXBGb3JtYXQpCiAJaWYgKFBRbnR1cGxl
cyhyZXMpID4gMCkKIAkJZnByaW50ZihPUEYsICItLVxuLS0gRGF0YWJhc2VzXG4tLVxuXG4iKTsK
IAorCS8qCisJICogZXhpdCBlYXJsaWVyIGlmIGRhdGFiYXNlIG5hbWUgY29udGFpbiBuZXdsaW5l
IG9yIGNhcnJpYWdlIHJldHVybi4KKwkgKiBhbHNvIHNlZSBhcHBlbmRTaGVsbFN0cmluZyBjb21t
ZW50cy4KKwkqLworCWluaXRQUUV4cEJ1ZmZlcigmdGVzdF9kYm5hbWUpOworCWZvciAoaSA9IDA7
IGkgPCBQUW50dXBsZXMocmVzKTsgaSsrKQorCXsKKwkJY2hhcgkgICAqZGJuYW1lID0gUFFnZXR2
YWx1ZShyZXMsIGksIDApOworCQlzdHJpbmdfY29udGFpbl9sZmNyKCZ0ZXN0X2RibmFtZSwgZGJu
YW1lLCAiZGF0YWJhc2UgbmFtZSIpOworCX0KKwl0ZXJtUFFFeHBCdWZmZXIoJnRlc3RfZGJuYW1l
KTsKKwogCS8qCiAJICogSWYgZGlyZWN0b3J5L3Rhci9jdXN0b20gZm9ybWF0IGlzIHNwZWNpZmll
ZCB0aGVuIGNyZWF0ZSBhIHN1YmRpcmVjdG9yeQogCSAqIHVuZGVyIHRoZSBtYWluIGRpcmVjdG9y
eSBhbmQgZWFjaCBkYXRhYmFzZSBkdW1wIGZpbGUgc3ViZGlyZWN0b3J5IHdpbGwKLS0gCjIuMzQu
MQoK
--0000000000006627db062ff84b88--