Skip to content

Fixed dangerous path handling and install prefix

metacollin requested to merge github/fork/metacollin/master into master

NEVER indiscriminately include directories using lines like

install( DIRECTORY ./

What if the user installs somewhere in the repo directory? Perhaps they want to try it in a test directory and keep everything contained in the repo directory. Or perhaps they're incorporating the repo into the most popular package manager on macOS, homebrew, which by design, requires building things within a sub directory of the build/repo files. Because it builds everything in a randomly made /tmp directory so as not to litter build files everywhere. I would say that the assumption that someone might use ./as the install prefix is one anyone writing build scripts must hold true.

However, since you include every directory, this will include any sub directories. It doesn't matter the path, ./<anything>.wrl matches *.wrl. So if their prefix is ./test_install, this install script will, during the installation, finish the .3dshapes starting with T, reach test_install, and since

./test_install/models/packages3D/<all the 3D shape directories that have been copied already>.3Dshape/<whatever>.wrl

does, in fact, match

*.wrl

it will begin copying test_install/models/packages3D into a new dir test_install/models/packages3D/models/packages3D. And then test_install/models/packages3D/models/packages3D/models/packages3D ...... then test_install/models/packages3D/models/packages3D/models/packages3D/models/packages3D

and so on.

This of course continues until either all available disk space is consumed, or the user notices. The user noticing is not necessarily the clear winner here - you'd be amazed how fast an NVMe SSD can completely fill itself. And how many problems losing all available space on one's boot disk in 30 seconds can cause.

Anyway, at least its fixed. Not sure why it was allowed in the repo to begin with though.

Edited by Joel Guittet

Merge request reports