Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filesystem watching crashes with symlinks pointing outside the asset folder #5689

Open
golddranks opened this issue Aug 14, 2022 · 5 comments
Labels
A-Assets Load files from disk to use for things like images, models, and sounds P-Crash A sudden unexpected crash

Comments

@golddranks
Copy link

Bevy version

0.8.0

What you did

I have a symlink pointing to a directory containing some assets, that was located outside of Bevy's assets directory.

What went wrong

As I modify a file in this directory, Bevy panics here: https://github.com/bevyengine/bevy/blob/main/crates/bevy_asset/src/io/file_asset_io.rs#L191 with StripPrefixError.

Additional information

My attempt to debug this problem showed that (at least on macOS 12.5) the notify crate which Bevy uses for filesystem watching, returns an event with asset path /Users/kon/repos/bomberman-of-the-hill/rounds/1/1.wasm (the absolute path of the asset, after having followed the symlink) as a response to modifying this asset, whereas asset_io.root_path is set to /Users/kon/repos/bomberman-of-the-hill/crates/bomber_game/assets/. As you can see from this Playground snippet, this produces a StripPrefixError: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=e4662b33f004e4f1a1f7d226a5dbe6c8

An interesting twist is that the asset would be reachable via path /Users/kon/repos/bomberman-of-the-hill/crates/bomber_game/assets/rounds/1/1.wasm and Bevy actually finds and uses this path when using the normal loading functions of AssetServer instead of filesystem watching. Of course, Bevy doesn't realize that the assets with differing paths are actually the same.

@golddranks golddranks added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Aug 14, 2022
@DJMcNab
Copy link
Member

DJMcNab commented Aug 14, 2022

I'm not sure we should be loading assets behind symlinks, at least by default.

@golddranks
Copy link
Author

That is for Bevy developers to decide; but I could see that sometimes symlinking some assets would be a helpful thing to do. (You might not want to store everything under your code repository, for example.) But it would be nice if at least instead of crashing, it would do something nicer about it. I'm not very well-versed in Bevy's logging and error reporting facilities, but maybe some warning about not being able to load it?

@Nilirad Nilirad added A-Assets Load files from disk to use for things like images, models, and sounds P-Crash A sudden unexpected crash and removed C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Aug 14, 2022
@golddranks
Copy link
Author

golddranks commented Aug 15, 2022

Okay, learned new stuff about this! I made a simple test program with notify ( https://github.com/golddranks/notify-test ) and turns out, it didn't follow/detect events behind symlinks at all (even on Recursive watch mode). Checking the repo issues a bit more, it seems that the notify crate doesn't provide that capability at the moment. The reason I was tricked into thinking that it would support symlinks, was that Bevy's directory walking followed them, and added each of the found files as a separate file to watch. This is why notify detected Modify-events from files outside of the assets directory. However, it was not able to detect new files being added!

Now, since notify doesn't support following symlinks, and because Bevy, too isn't prepared to handle paths outside of the assets dir, the simplest and most straightforward thing would be, in my mind, to explicitly not support following symlinks, and consider a support for that an additional feature that could be added if there's demand and justification for it. So maybe the fix here could be just fixing the Bevy directory walking not to follow symlinks.

@golddranks
Copy link
Author

Then there's the problem that even though directory walking didn't follow symlinks, the users can still provide paths to load, load_folder etc. calls that go through symlinks or contain .. segments, which breaks stuff.

@golddranks
Copy link
Author

golddranks commented Aug 15, 2022

#5705 Is an attempt to fix this by at least providing consistent behaviour that "fails fast" but not unexpectedly instead of sometimes working and sometimes not, creating confusion and possibly making downstream to rely on buggy behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds P-Crash A sudden unexpected crash
Projects
None yet
Development

No branches or pull requests

3 participants