nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.22k stars 1.46k forks source link

`joinPath` sometimes has extra `.` in the beginning of the path #23712

Open alex65536 opened 2 weeks ago

alex65536 commented 2 weeks ago

Description

import std/os

echo joinPath(".", "a")
echo joinPath("b", "..", "a")
echo joinPath(".", "..")
echo joinPath("./", "a")
echo joinPath("./", "..")
echo joinPath(".", "a", "b")
echo joinPath("./a", "b")
echo joinPath("x/..", "/a", "/b", "../../..")
echo joinPath("x/../a/b", "../../..")

Nim Version

2.0.4 (also reproducible on the latest devel)

Current Output

./a
./a
./..
./a
./..
./a/b
a/b
./..
..

Expected Output

a
a
..
a
..
a/b
a/b
..
..

Possible Solution

No response

Additional Information

No response

alex65536 commented 2 weeks ago

While looking at the code, I have found out that there are two issues basically.

In joinPath(".", "a") and similar cases, the path becomes "." after first addNormalizePath and second addNormalizePath doesn't remove it. In test cases this one is called "controversial" because of how paths to executables behave on Unix systems: https://github.com/nim-lang/Nim/blob/0b5a938f571133f6e876938387e37199d4c2ec16/tests/stdlib/tos.nim#L590-L594 Retaining this behavior might be good to prevent older programs from breaking, but it starts to look much weirder in case of joinPath("b", "..", "a") or joinPath(".", ".."), so probably worth either fixing or documenting.

The second part is less controversial and more looks like a bug. The problem is in the line https://github.com/nim-lang/Nim/blob/0b5a938f571133f6e876938387e37199d4c2ec16/lib/pure/pathnorm.nim#L75 This is not the correct way to check if it's the first component of the path, as state == 0 holds also for such paths as . or ../... Thus, when handling joinPath("x/..", "/a", "/b", "../../..") it starts to think at some point that the path is absolute, because it receives "/" from PathIter. Though, by coincidence and only because the . is not removed because of the first issue, it seems that such incorrect state doesn't lead to wrong results (or at least I haven't been able to found the failing case).

juancarlospaco commented 2 weeks ago

But ./a is not the same as a. 🤔

alex65536 commented 2 weeks ago

But ./a is not the same as a. 🤔

Well, but

import std/[os, paths]

echo normalizedPath("./a")
echo "./a".Path == "a".Path

prints

a
true

And the documentation promises that joinPath https://github.com/nim-lang/Nim/blob/0b5a938f571133f6e876938387e37199d4c2ec16/lib/std/private/ospaths2.nim#L96-L98 but ./a is obviously not normalized.