On Wed, Nov 10, 2021 at 05:43:31PM -0500, Tom Lane wrote:
> Another thing I happened to notice is that join_path_components
> is going out of its way to not generate "foo/./bar", but if
> we are fixing canonicalize_path to be able to delete the "./",
> that seems like a waste of code now.
>
> I am not entirely convinced that 0002 isn't re-introducing the
> security hole that the existing code seeks to plug. That one
> is going to require more justification.
At the same time, do we have any need for doing 0002 at all if
we do 0001? The paths are canonicalized before checking them in
path_contains_parent_reference().
> I concur with the upthread comments that there's little chance
> we'll commit 0003 as-is; the code-to-benefit ratio is too high.
> Instead, you might consider adding test_canonicalize_path in
> src/test/regress/regress.c, and then adding a smaller number
> of regression test cases using that.
Sounds like a good idea to me. I would move these in misc.source for
anything that require an absolute path.
0001 is indeed in need of more comments and documentation so as one
does not get lost if reading through this code in the future. Changes
in trim_directory(), for example, should explain what is returned and
why.
+ isabs = is_absolute_path(path);
+ tmppath = strdup(path);
If possible, it would be nice to cut any need for malloc() allocations
in this code.
--
Michael