[Rseding91] [0.17.23] Factorio deletes files outside the factorio directory

This subforum contains all the issues which we already resolved.
Post Reply
mrvn
Smart Inserter
Smart Inserter
Posts: 3104
Joined: Mon Sep 05, 2016 9:10 am
Contact:

[Rseding91] [0.17.23] Factorio deletes files outside the factorio directory

Post by mrvn » Wed Apr 03, 2019 5:34 pm

A while back when discussing hot to best maintain mods in git and play with them someone suggested placing a symlink in factorio/mods/name_version -> somewhere_else/name. That way one doesn't have to constantly rename the git repository but only change the link.

But when I accidentally updated the mod I had linked that way in game factorio happily went and followed the symlink and deleted everything recursively. So the whole git repository OUTSIDE the factorio dir is gone. Now imagine someone creates a symlink to /. I could include that in my evil_mod.zip and suggest to people they should unzip it to improve loading times. Next time factorio updates the mod the whole system gets deleted.

Please use lstat() instead of stat() when checking for directories and ensure that symlinks don't leave the factorio directory. Probably best to not follow any symlink to outside of the base directory of a mod ( factorio/mods/name_version) or one mod could still delete another mods files.

Rseding91
Factorio Staff
Factorio Staff
Posts: 9023
Joined: Wed Jun 11, 2014 5:23 am
Contact:

Re: [0.17.23] Factorio deletes files outside the factorio directory

Post by Rseding91 » Thu Apr 04, 2019 12:06 pm

mrvn wrote:
Wed Apr 03, 2019 5:34 pm
Please use lstat() instead of stat() when checking for directories and ensure that symlinks don't leave the factorio directory.
We don't use either. We use <filesystem> https://en.cppreference.com/w/cpp/header/filesystem
If you want to get ahold of me I'm almost always on Discord.

mrvn
Smart Inserter
Smart Inserter
Posts: 3104
Joined: Mon Sep 05, 2016 9:10 am
Contact:

Re: [0.17.23] Factorio deletes files outside the factorio directory

Post by mrvn » Thu Apr 04, 2019 12:17 pm

Rseding91 wrote:
Thu Apr 04, 2019 12:06 pm
mrvn wrote:
Wed Apr 03, 2019 5:34 pm
Please use lstat() instead of stat() when checking for directories and ensure that symlinks don't leave the factorio directory.
We don't use either. We use <filesystem> https://en.cppreference.com/w/cpp/header/filesystem
Then that would be symlink_status instead of status(). And I guess the recursive_directory_iterator is out.

Rseding91
Factorio Staff
Factorio Staff
Posts: 9023
Joined: Wed Jun 11, 2014 5:23 am
Contact:

Re: [Rseding91] [0.17.23] Factorio deletes files outside the factorio directory

Post by Rseding91 » Thu Apr 04, 2019 12:52 pm

Ok, after talking with Oxyd about the solution it's now fixed for the next version of 0.17.
If you want to get ahold of me I'm almost always on Discord.

quyxkh
Filter Inserter
Filter Inserter
Posts: 710
Joined: Sun May 08, 2016 9:01 am
Contact:

Re: [0.17.23] Factorio deletes files outside the factorio directory

Post by quyxkh » Thu Apr 04, 2019 1:52 pm

Rseding91 wrote:
Thu Apr 04, 2019 12:06 pm
mrvn wrote:
Wed Apr 03, 2019 5:34 pm
Please use lstat() instead of stat() when checking for directories and ensure that symlinks don't leave the factorio directory.
We don't use either. We use <filesystem> https://en.cppreference.com/w/cpp/header/filesystem
According to fs.op.remove_all,
27.10.15.31 Remove all [fs.op.remove_all]
uintmax_t remove_all(const path& p);
uintmax_t remove_all(const path& p, error_code& ec) noexcept;
  • Effects: Recursively deletes the contents of p if it exists, then deletes file p itself, as if by POSIX
    remove(). [ Note: A symbolic link is itself removed, rather than the file it resolves to. — end note ]
  • Postcondition: !exists(symlink_status(p)).
  • Returns: The number of files removed. The signature with argument ec returns static_cast<
    uintmax_t>(-1) if an error occurs.
  • Throws: As specified in Error reporting (27.10.7).
and I get the same behavior on Linux, installing a mod from the portal doesn't just delete the mod directory's installed symlink to my development repo, it deletes my development repo.

So the only way I can see to get that is you're removing the contents _and_ the entry, rather than just remove_all'ing the entry.

mrvn
Smart Inserter
Smart Inserter
Posts: 3104
Joined: Mon Sep 05, 2016 9:10 am
Contact:

Re: [0.17.23] Factorio deletes files outside the factorio directory

Post by mrvn » Thu Apr 04, 2019 2:44 pm

quyxkh wrote:
Thu Apr 04, 2019 1:52 pm
Rseding91 wrote:
Thu Apr 04, 2019 12:06 pm
mrvn wrote:
Wed Apr 03, 2019 5:34 pm
Please use lstat() instead of stat() when checking for directories and ensure that symlinks don't leave the factorio directory.
We don't use either. We use <filesystem> https://en.cppreference.com/w/cpp/header/filesystem
According to fs.op.remove_all,
27.10.15.31 Remove all [fs.op.remove_all]
uintmax_t remove_all(const path& p);
uintmax_t remove_all(const path& p, error_code& ec) noexcept;
  • Effects: Recursively deletes the contents of p if it exists, then deletes file p itself, as if by POSIX
    remove(). [ Note: A symbolic link is itself removed, rather than the file it resolves to. — end note ]
  • Postcondition: !exists(symlink_status(p)).
  • Returns: The number of files removed. The signature with argument ec returns static_cast<
    uintmax_t>(-1) if an error occurs.
  • Throws: As specified in Error reporting (27.10.7).
and I get the same behavior on Linux, installing a mod from the portal doesn't just delete the mod directory's installed symlink to my development repo, it deletes my development repo.

So the only way I can see to get that is you're removing the contents _and_ the entry, rather than just remove_all'ing the entry.
The problem is
Recursively deletes the contents of p
. p is a symlink to a directory so all the contents are deleted recursively and then the symlink but not what the symlink points too. Imho it should not recurse into symlinks or have a flag for that.

quyxkh
Filter Inserter
Filter Inserter
Posts: 710
Joined: Sun May 08, 2016 9:01 am
Contact:

Re: [0.17.23] Factorio deletes files outside the factorio directory

Post by quyxkh » Thu Apr 04, 2019 11:00 pm

mrvn wrote:
Thu Apr 04, 2019 2:44 pm
The problem is […] p is a symlink to a directory so all the contents are deleted recursively and then the symlink but not what the symlink points too. Imho it should not recurse into symlinks or have a flag for that.
Well, they fixed it, but I'm curious: what evidence were you looking at that led you to say so?

Code: Select all

#include <filesystem>
int main(int c, char **v)
{
	using namespace std::filesystem;
	create_directories("testdir/testcontents");
	create_directory_symlink("testdir","testlink");
	remove_all("testlink");
	return 0;
}
deletes the symlink and leaves the directories behind, as required, so I don't see what led you to say what you did.

User avatar
Therax
Filter Inserter
Filter Inserter
Posts: 382
Joined: Sun May 21, 2017 6:28 pm
Contact:

Re: [Rseding91] [0.17.23] Factorio deletes files outside the factorio directory

Post by Therax » Fri Apr 05, 2019 2:50 am

I've had the same experience, but I'm on Windows. Windows "symlink" story is so confused that I wouldn't expect filesystem to get it right in all cases. Starting Factorio with a symlink in the mods directory pointing to a git repository, and then updating the mod via the in-game interface downloads the latest version from the mod portal in zip format, and then proceeds to recursively remove everything in the git repository directory, and finally remove the symlink.
Miniloader — UPS-friendly 1x1 loaders
Bulk Rail Loaders — Rapid train loading and unloading
Beltlayer & Pipelayer — Route items and fluids freely underground

mrvn
Smart Inserter
Smart Inserter
Posts: 3104
Joined: Mon Sep 05, 2016 9:10 am
Contact:

Re: [0.17.23] Factorio deletes files outside the factorio directory

Post by mrvn » Sat Apr 06, 2019 11:26 am

quyxkh wrote:
Thu Apr 04, 2019 11:00 pm
mrvn wrote:
Thu Apr 04, 2019 2:44 pm
The problem is […] p is a symlink to a directory so all the contents are deleted recursively and then the symlink but not what the symlink points too. Imho it should not recurse into symlinks or have a flag for that.
Well, they fixed it, but I'm curious: what evidence were you looking at that led you to say so?

Code: Select all

#include <filesystem>
int main(int c, char **v)
{
	using namespace std::filesystem;
	create_directories("testdir/testcontents");
	create_directory_symlink("testdir","testlink");
	remove_all("testlink");
	return 0;
}
deletes the symlink and leaves the directories behind, as required, so I don't see what led you to say what you did.
Factorio didn't leave the subdirs behind. And if factorio was using remove_all ... Do you have the same STL library as factorio is using? Maybe they fixed the STL? Or factorio wasn't yet using remove_all but is now?

Post Reply

Return to “Resolved Problems and Bugs”

Who is online

Users browsing this forum: zero318