Skip to content

Bugfix/1827 invalid cross device link #1829

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

Merged
20 changes: 15 additions & 5 deletions fsspec/implementations/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,12 +430,22 @@ def __getstate__(self):
def commit(self):
if self.autocommit:
raise RuntimeError("Can only commit if not already set to autocommit")
shutil.move(self.temp, self.path)
try:
mask = 0o666
os.chmod(self.path, mask & ~get_umask(mask))
except RuntimeError:
pass
shutil.move(self.temp, self.path)
except PermissionError as e:
# shutil.move raises PermissionError if os.rename
# and the default copy2 fallback with shutil.copystats fail.
# The file should be there nonetheless, but without copied permissions.
# If it doesn't exist, there was no permission to create the file.
if not os.path.exists(self.path):
raise e
else:
# If PermissionError is not raised, permissions can be set.
try:
mask = 0o666
os.chmod(self.path, mask & ~get_umask(mask))
except RuntimeError:
pass

def discard(self):
if self.autocommit:
Expand Down
20 changes: 20 additions & 0 deletions fsspec/implementations/tests/test_local.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import bz2
import errno
import gzip
import os
import os.path
Expand Down Expand Up @@ -562,6 +563,25 @@ def test_multiple_filesystems_use_umask_cache(tmpdir):
assert get_umask.cache_info().hits == 1


def test_transaction_cross_device_but_mock_temp_dir_on_wrong_device(tmpdir):
# If the temporary file for a transaction is not on the correct device,
# os.rename in shutil.move will raise EXDEV and lookup('chmod') will raise
# a PermissionError.
fs = LocalFileSystem()
with (
patch(
"os.rename",
side_effect=OSError(errno.EXDEV, "Invalid cross-device link"),
),
patch(
"os.chmod",
side_effect=PermissionError("Operation not permitted"),
),
):
with fs.transaction, fs.open(tmpdir + "/afile", "wb") as f:
f.write(b"data")


def test_make_path_posix():
cwd = os.getcwd()
if WIN:
Expand Down
Loading