Unexpected Abort on `nix::canonPath(PathView, bool): Assertion` path != ""' failed.` #536

New issue

Closed

opened 2024-09-29 01:21:14 +00:00 by samueldr · 2 comments

samueldr commented

2024-09-29 01:21:14 +00:00

Member

Describe the bug

~ $ nix repl '<nixpkgs>'
Lix 2.91.0
Type :? for help.
Loading installable ''...
Added 22730 variables.
nix-repl> :p (hello // { drvPath = ""; })
nix: src/libutil/file-system.cc:39: nix::Path nix::canonPath(PathView, bool): Assertion `path != ""' failed.
Aborted (core dumped)

Steps To Reproduce

Sorry.

Expected behavior

Unsure. At least not aborting.

`nix --version` output

~ $ nix --version
nix (Lix, like Nix) 2.91.0

Additional context

Not a huge issue. Just surprising sudden loss of REPL :).

## Describe the bug ``` ~ $ nix repl '<nixpkgs>' Lix 2.91.0 Type :? for help. Loading installable ''... Added 22730 variables. nix-repl> :p (hello // { drvPath = ""; }) nix: src/libutil/file-system.cc:39: nix::Path nix::canonPath(PathView, bool): Assertion `path != ""' failed. Aborted (core dumped) ``` ## Steps To Reproduce Sorry. ## Expected behavior Unsure. At least not aborting. ## `nix --version` output ``` ~ $ nix --version nix (Lix, like Nix) 2.91.0 ``` ## Additional context Not a huge issue. Just surprising sudden loss of REPL :).

samueldr added the

bug

label

2024-09-29 01:21:14 +00:00

jade added the

E/reproducible

label

2024-09-29 05:16:19 +00:00

jade added the

crash 💥

label

2024-11-10 02:17:29 +00:00

gilice commented

2025-02-21 15:11:23 +00:00

Member

The minimal patch for this would be

diff --git a/lix/libutil/file-system.cc b/lix/libutil/file-system.cc
index 9322e96c1..a67bdc718 100644
--- a/lix/libutil/file-system.cc
+++ b/lix/libutil/file-system.cc
@@ -42,14 +42,12 @@ Path absPath(Path path, std::optional<PathView> dir, bool resolveSymlinks)
 
 Path canonPath(PathView path, bool resolveSymlinks)
 {
-    assert(path != "");
+    if (path == "" || path[0] != '/')
+        throw Error("not an absolute path: '%1%'", path);
 
     std::string s;
     s.reserve(256);
 
-    if (path[0] != '/')
-        throw Error("not an absolute path: '%1%'", path);
-
     std::string temp;
 
     /* Count the number of times we follow a symlink and stop at some
diff --git a/tests/unit/libutil/tests.cc b/tests/unit/libutil/tests.cc
index 00fcf8269..bbf285bea 100644
--- a/tests/unit/libutil/tests.cc
+++ b/tests/unit/libutil/tests.cc
@@ -89,7 +89,7 @@ namespace nix {
         ASSERT_ANY_THROW(canonPath("."));
         ASSERT_ANY_THROW(canonPath(".."));
         ASSERT_ANY_THROW(canonPath("../"));
-        ASSERT_DEATH({ canonPath(""); }, "path != \"\"");
+        ASSERT_ANY_THROW(canonPath(""));
     }
 
     /* ----------------------------------------------------------------------------

This passes all tests and results in the following repr:

$ nix repl '<nixpkgs>'
warning: future versions of Lix will require using `--file` to load a file
Lix 2.93.0-dev
Type :? for help.
Loading installable ''...
Added 23751 variables.
nix-repl> :p (hello // { drvPath = ""; })
error: not an absolute path: ''

I am however not sure if the rest of the codebase assumes anything about how canonPath works, so not filing a PR yet. (It is especially suspicious that there was a test asserting it will throw.)
Maybe someone else can weigh in?

The minimal patch for this would be ```diff diff --git a/lix/libutil/file-system.cc b/lix/libutil/file-system.cc index 9322e96c1..a67bdc718 100644 --- a/lix/libutil/file-system.cc +++ b/lix/libutil/file-system.cc @@ -42,14 +42,12 @@ Path absPath(Path path, std::optional<PathView> dir, bool resolveSymlinks) Path canonPath(PathView path, bool resolveSymlinks) { - assert(path != ""); + if (path == "" || path[0] != '/') + throw Error("not an absolute path: '%1%'", path); std::string s; s.reserve(256); - if (path[0] != '/') - throw Error("not an absolute path: '%1%'", path); - std::string temp; /* Count the number of times we follow a symlink and stop at some diff --git a/tests/unit/libutil/tests.cc b/tests/unit/libutil/tests.cc index 00fcf8269..bbf285bea 100644 --- a/tests/unit/libutil/tests.cc +++ b/tests/unit/libutil/tests.cc @@ -89,7 +89,7 @@ namespace nix { ASSERT_ANY_THROW(canonPath(".")); ASSERT_ANY_THROW(canonPath("..")); ASSERT_ANY_THROW(canonPath("../")); - ASSERT_DEATH({ canonPath(""); }, "path != \"\""); + ASSERT_ANY_THROW(canonPath("")); } /* ---------------------------------------------------------------------------- ``` This passes all tests and results in the following repr: ```bash $ nix repl '<nixpkgs>' warning: future versions of Lix will require using `--file` to load a file Lix 2.93.0-dev Type :? for help. Loading installable ''... Added 23751 variables. nix-repl> :p (hello // { drvPath = ""; }) error: not an absolute path: '' ``` I am however not sure if the rest of the codebase assumes anything about how `canonPath` works, so not filing a PR yet. (It is especially suspicious that there was a test asserting it will throw.) Maybe someone else can weigh in?

lix-bot commented

2025-04-12 10:41:39 +00:00

Member

This issue was mentioned on Gerrit on the following CLs:

commit message in cl/2986 ("libutil: canonPath: error instead of panic on empty path")

This issue was mentioned on Gerrit on the following CLs: * commit message in [cl/2986](https://gerrit.lix.systems/c/lix/+/2986) ("libutil: canonPath: error instead of panic on empty path")