"current directory" in a server error message

Started by Kyotaro Horiguchialmost 3 years ago5 messages
#1Kyotaro Horiguchi
horikyota.ntt@gmail.com
1 attachment(s)

Hello.

When I ran pg_ls_dir('..'), the error message I received was somewhat
difficult to understand.

postgres=> select * from pg_ls_dir('..');
ERROR: path must be in or below the current directory

As far as I know the concept of a "current directory" doesn't apply to
the server side. In fact, the function comment for
convert_and_check_filename explicitly states that:

* Filename may be absolute or relative to the DataDir

Thus I think that the message should read "path must be in or below
the data directory" instead.

What do you think about making this change?

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachments:

change_an_error_message.difftext/x-patch; charset=us-asciiDownload
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 75588eebb3..f281ce9806 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -86,7 +86,7 @@ convert_and_check_filename(text *arg)
 	else if (!path_is_relative_and_below_cwd(filename))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("path must be in or below the current directory")));
+				 errmsg("path must be in or below the data directory")));
 
 	return filename;
 }
#2Bharath Rupireddy
bharath.rupireddyforpostgres@gmail.com
In reply to: Kyotaro Horiguchi (#1)
Re: "current directory" in a server error message

On Thu, Mar 16, 2023 at 7:47 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:

Hello.

When I ran pg_ls_dir('..'), the error message I received was somewhat
difficult to understand.

postgres=> select * from pg_ls_dir('..');
ERROR: path must be in or below the current directory

As far as I know the concept of a "current directory" doesn't apply to
the server side. In fact, the function comment for
convert_and_check_filename explicitly states that:

* Filename may be absolute or relative to the DataDir

Thus I think that the message should read "path must be in or below
the data directory" instead.

What do you think about making this change?

Well yes. As far as postgres processes are concerned their working
directory is set to data directory by the postmaster in
ChangeToDataDir() and all the children will inherit that setting. So,
I see nothing wrong in being explicit about it in the error messages.

BTW, adminpack too has the same error message.

FWIW, here are the steps to generate the error:
create role foo with nosuperuser;
grant execute on function pg_ls_dir(text) to foo;
set role foo;
select * from pg_ls_dir('..');

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

#3Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Bharath Rupireddy (#2)
Re: "current directory" in a server error message

At Thu, 16 Mar 2023 09:32:05 +0530, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote in

On Thu, Mar 16, 2023 at 7:47 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:

Thus I think that the message should read "path must be in or below
the data directory" instead.

What do you think about making this change?

Well yes. As far as postgres processes are concerned their working
directory is set to data directory by the postmaster in
ChangeToDataDir() and all the children will inherit that setting. So,
I see nothing wrong in being explicit about it in the error messages.

Yeah, you're right.

BTW, adminpack too has the same error message.

I somehow dropped them. Thanks for pointing.

FWIW, here are the steps to generate the error:
create role foo with nosuperuser;
grant execute on function pg_ls_dir(text) to foo;
set role foo;
select * from pg_ls_dir('..');

Oh, thank you for the clarification about the reproduction method.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Kyotaro Horiguchi (#3)
Re: "current directory" in a server error message

Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:

At Thu, 16 Mar 2023 09:32:05 +0530, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote in

On Thu, Mar 16, 2023 at 7:47 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:

Thus I think that the message should read "path must be in or below
the data directory" instead.

BTW, adminpack too has the same error message.

I somehow dropped them. Thanks for pointing.

Agreed, this is an improvement. I fixed adminpack too and pushed it.

regards, tom lane

#5Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Tom Lane (#4)
Re: "current directory" in a server error message

At Thu, 16 Mar 2023 12:05:32 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in

Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:

At Thu, 16 Mar 2023 09:32:05 +0530, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote in

On Thu, Mar 16, 2023 at 7:47 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:

Thus I think that the message should read "path must be in or below
the data directory" instead.

BTW, adminpack too has the same error message.

I somehow dropped them. Thanks for pointing.

Agreed, this is an improvement. I fixed adminpack too and pushed it.

Oh, thanks for committing this.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center