Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote

On 2025-03-27 Th 7:57 AM, Mahendra Singh Thalor wrote:
> On Thu, 27 Mar 2025 at 16:16, Andrew Dunstan <andrew@dunslane.net> wrote:
>>
>> On 2025-03-26 We 8:52 AM, Srinath Reddy wrote:
>>
>> sorry for the noise ,previous response had my editor's formatting,just resending without that formatting.
>>
>> ./psql postgres
>>
>> Hi,
>>
>> On Wed, Mar 26, 2025 at 5:55 PM Andrew Dunstan <andrew@dunslane.net> wrote:
>>> You can still create a database with these using "CREATE DATABASE" though. Shouldn't we should really be preventing
that?
>>
>> yes, solution 1 which I mentioned prevents these while we are using "CREATE DATABASE".
>>
>> /*
>>    * Create a new database using the WAL_LOG strategy.
>> @@ -741,6 +742,13 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
>>    CreateDBStrategy dbstrategy = CREATEDB_WAL_LOG;
>>    createdb_failure_params fparms;
>>
>> + /* Report error if dbname have newline or carriage return in name. */
>> + if (is_name_contain_lfcr(dbname))
>> + ereport(ERROR,
>> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE)),
>> + errmsg("database name contains a newline or carriage return character"),
>> + errhint("newline or carriage return character is not allowed in database name"));
>> +
>>
>> psql (18devel)
>> Type "help" for help.
>>
>> postgres=# create database "test
>> postgres"# lines";
>> ERROR:  database name contains a newline or carriage return character
>> HINT:  newline or carriage return character is not allowed in database name
>>
>>
>>
>>
>> Yes, sorry, I misread the thread. I think we should proceed with options 1 and 3 i.e. prevent creation of new
databaseswith a CR or LF, and have pgdumpall exit with a more useful error message.
 
>>
>> Your invention of an is_name_contain_lfcr() function is unnecessary - we can just use the standard library function
strpbrk()to look for a CR or LF.
 
>>
>>
>> cheers
>>
> Thanks Andrew and Srinath for feedback.
>
> Yes, we should use the strpbrk function. Fixed.
>
> Here, I am attaching an updated patch which has check in createdb and
> RenameDatabase. For older versions, we can add more useful error
> message (like: rename database as database has \n\r")
>
> I will add some TAP tests and will make patches for older branches.
>


I don't think we can backpatch this. It's a behaviour change.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com