Обсуждение: "current directory" in a server error message

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

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

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

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

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