Re: Checking pgwin32_is_junction() errors

On Tue, Aug 9, 2022 at 10:59 PM <r.zharkov@postgrespro.ru> wrote:
> On 2022-08-09 05:44, Thomas Munro wrote:
> > On Tue, Aug 9, 2022 at 8:30 AM Thomas Munro <thomas.munro@gmail.com>
> > wrote:
> >> On Mon, Aug 8, 2022 at 8:23 PM <r.zharkov@postgrespro.ru> wrote:
> >> > "C:/HOME" is the junction point to the second volume on my hard drive -
> >> > "\??\Volume{GUID}\" which name pgreadlink() erroneously strips here:
> >> > https://github.com/postgres/postgres/blob/7e29a79a46d30dc236d097825ab849158929d977/src/port/dirmod.c#L357.
> >
> >> ... Would it
> >> be better to say: if it doesn't begin with "\??\X:", where X could be
> >> any letter, then don't modify it?
> >
> > Concretely, I wonder if this is a good fix at least in the short term.
> > Does this work for you, and do the logic and explanation make sense?
>
> Yes, this patch works well with my junction points.

Thanks for testing!  I did a bit more reading on this stuff, so that I
could update the comments with the correct terminology from Windows
APIs.  I also realised that the pattern we could accept to symlink()
and expect to work is not just "C:..." (could be
RtlPathTypeDriveRelative, which wouldn't actually work in a junction
point) but "C:\..." (RtlPathTypeDriveAbsolute).  I tweaked it a bit to
test for that.

> I checked a few variants:
>
> 21.07.2022  15:11    <JUNCTION>     HOME [\??\Volume{GUID}\]
> 09.08.2022  15:06    <JUNCTION>     Test1 [\\?\Volume{GUID}\]
> 09.08.2022  15:06    <JUNCTION>     Test2 [\\.\Volume{GUID}\]
> 09.08.2022  15:17    <JUNCTION>     Test3 [\??\Volume{GUID}\]
> 09.08.2022  15:27    <JUNCTION>     Test4 [C:\temp\1]
> 09.08.2022  15:28    <JUNCTION>     Test5 [C:\HOME\Temp\1]

One more thing I wondered about, now that we're following junctions
outside PGDATA: can a junction point to another junction?  If so, I
didn't allow for that: stat() gives up after one hop, because I
figured that was enough for the stuff we expect inside PGDATA and I
couldn't find any evidence in the man pages that referred to chains.
But if you *are* allowed to create a junction "c:\huey" that points to
junction "c:\dewey" that points to "c:\louie", and then you do initdb
-D c:\huey\pgdata, then I guess it would fail.  Would you mind
checking if that is a real possibility, and if so, testing this
chain-following patch to see if it fixes it?

> After hours of reading the documentation and debugging, it seems to me
> we can use REPARSE_GUID_DATA_BUFFER structure instead of our
> REPARSE_JUNCTION_DATA_BUFFER [1]. DataBuffer doesn't contain any
> prefixes,
> so we don't need to strip them. But we still need to construct a correct
> volume name if a junction point is a volume mount point. Is it worth to
> check this idea?

I don't know.  I think I prefer our current approach, because it can
handle anything (raw/full NT paths) and doesn't try to be very clever,
and I don't want to change to a different scheme for no real
benefit...