mimium-org / mimium

mimium (MInimal Musical medIUM) a programming language as an infrastructure for sound and music.
https://mimium.org
Mozilla Public License 2.0
273 stars 9 forks source link

Use std::filesystem::weakly_canonical() to avoid filesyste_error #62

Closed t-sin closed 3 years ago

t-sin commented 3 years ago

This PR fixes a bug about path normalization.

Bug behavior

This bug occurs when the users specify absolute path but it actually does not exist. When mimium command is passed such a path, the command fails like this:

$ $ mimium /path/does/not/exist/foo.mmm
filesystem error: cannot make canonical path: No such file or directory [../../../../path/does/not/exist/foo.mmm]

Additionally, if a path specified exists, mimium command normalizes the path and then continues its process.

I found this behavior in development of mimium mapckage manager.

The cause of this bug

mimium does path normalization at first of its process but it has not done correctly.

The path normalization has done at this point:

https://github.com/mimium-org/mimium/blob/263f357adc910e7b9ac1c503294c66c7ea5ad56a/src/frontend/genericapp.cpp#L64

According to this page, the function std::filesystem::canonical() throws filesystem_error when the path does not exist. That page also says that the function does fully existence check for all path elements.

How to fix

That page also says std::filesystem::weakly_canonical() does the path normalization partially. It means that the specified path so the path entirely may not exist.

If using weakly_canonical() instead of canonical(), the mimium command successfully normalizes at the point of the source code, checks input path existence then handles by the situation by itself, like this:

$ cmake --build ./build/ -j && ./build/src/mimium /path/does/not/exist/foo.mmm
File not found: /path/does/not/exist/foo.mmm

So, in this PR's case, using weakly_canonical() is better.

tomoyanonymous commented 3 years ago

@all-contributors please add @t-sin for code

allcontributors[bot] commented 3 years ago

@tomoyanonymous

I've put up a pull request to add @t-sin! :tada: