Raise when trying to acquire in R/O or missing folder (#96)

Author: gaborbernat

setup.cfg

@@ -50,6 +50,7 @@ testing =
     coverage>=4
     pytest>=4
     pytest-cov
+    pytest-timeout>=1.4.2
 
 [bdist_wheel]
 universal = true

src/filelock/_soft.py

@@ -1,17 +1,32 @@
 import os
+import sys
+from errno import EACCES, EEXIST, ENOENT
 
 from ._api import BaseFileLock
+from ._util import raise_on_exist_ro_file
 
 
 class SoftFileLock(BaseFileLock):
     """Simply watches the existence of the lock file."""
 
     def _acquire(self):
-        open_mode = os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_TRUNC
+        raise_on_exist_ro_file(self._lock_file)
+        # first check for exists and read-only mode as the open will mask this case as EEXIST
+        mode = (
+            os.O_WRONLY  # open for writing only
+            | os.O_CREAT
+            | os.O_EXCL  # together with above raise EEXIST if the file specified by filename exists
+            | os.O_TRUNC  # truncate the file to zero byte
+        )
         try:
-            fd = os.open(self._lock_file, open_mode)
-        except OSError:
-            pass
+            fd = os.open(self._lock_file, mode)
+        except OSError as exception:
+            if exception.errno == EEXIST:  # expected if cannot lock
+                pass
+            elif exception.errno == ENOENT:  # No such file or directory - parent directory is missing
+                raise
+            elif exception.errno == EACCES and sys.platform != "win32":  # Permission denied - parent dir is R/O
+                raise  # note windows does not allow you to make a folder r/o only files
         else:
             self._lock_file_fd = fd
 
@@ -20,8 +35,7 @@ def _release(self):
         self._lock_file_fd = None
         try:
             os.remove(self._lock_file)
-        # The file is already deleted and that's what we want.
-        except OSError:
+        except OSError:  # the file is already deleted and that's what we want
             pass
 
 

src/filelock/_util.py

@@ -0,0 +1,22 @@
+import os
+import stat
+import sys
+
+PermissionError = PermissionError if sys.version_info[0] == 3 else OSError
+
+
+def raise_on_exist_ro_file(filename):
+    try:
+        file_stat = os.stat(filename)  # use stat to do exists + can write to check without race condition
+    except OSError:
+        return None  # swallow does not exist or other errors
+
+    if file_stat.st_mtime != 0:  # if os.stat returns but modification is zero that's an invalid os.stat - ignore it
+        if not (file_stat.st_mode & stat.S_IWUSR):
+            raise PermissionError("Permission denied: {!r}".format(filename))
+
+
+__all__ = [
+    "raise_on_exist_ro_file",
+    "PermissionError",
+]

src/filelock/_windows.py

@@ -1,6 +1,8 @@
 import os
+from errno import ENOENT
 
 from ._api import BaseFileLock
+from ._util import raise_on_exist_ro_file
 
 try:
     import msvcrt
@@ -12,11 +14,17 @@ class WindowsFileLock(BaseFileLock):
     """Uses the :func:`msvcrt.locking` function to hard lock the lock file on windows systems."""
 
     def _acquire(self):
-        open_mode = os.O_RDWR | os.O_CREAT | os.O_TRUNC
+        raise_on_exist_ro_file(self._lock_file)
+        mode = (
+            os.O_RDWR  # open for read and write
+            | os.O_CREAT  # create file if not exists
+            | os.O_TRUNC  # truncate file  if not empty
+        )
         try:
-            fd = os.open(self._lock_file, open_mode)
-        except OSError:
-            pass
+            fd = os.open(self._lock_file, mode)
+        except OSError as exception:
+            if exception.errno == ENOENT:  # No such file or directory
+                raise
         else:
             try:
                 msvcrt.locking(fd, msvcrt.LK_NBLCK, 1)

tests/test_filelock.py

@@ -3,10 +3,13 @@
 import logging
 import sys
 import threading
+from contextlib import contextmanager
+from stat import S_IWGRP, S_IWOTH, S_IWUSR
 
 import pytest
 
 from filelock import FileLock, SoftFileLock, Timeout
+from filelock._util import PermissionError
 
 
 @pytest.mark.parametrize("lock_type", [FileLock, SoftFileLock])
@@ -29,6 +32,52 @@ def test_simple(lock_type, tmp_path, caplog):
     assert [r.levelno for r in caplog.records] == [logging.DEBUG, logging.DEBUG, logging.DEBUG, logging.DEBUG]
 
 
+@contextmanager
+def make_ro(path):
+    write = S_IWUSR | S_IWGRP | S_IWOTH
+    path.chmod(path.stat().st_mode & ~write)
+    yield
+    path.chmod(path.stat().st_mode | write)
+
+
+@pytest.fixture()
+def tmp_path_ro(tmp_path):
+    with make_ro(tmp_path):
+        yield tmp_path
+
+
+@pytest.mark.parametrize("lock_type", [FileLock, SoftFileLock])
+@pytest.mark.skipif(sys.platform == "win32", reason="Windows does not have read only folders")
+def test_ro_folder(lock_type, tmp_path_ro):
+    lock = lock_type(str(tmp_path_ro / "a"))
+    with pytest.raises(PermissionError, match="Permission denied"):
+        lock.acquire()
+
+
+@pytest.fixture()
+def tmp_file_ro(tmp_path):
+    filename = tmp_path / "a"
+    filename.write_text("")
+    with make_ro(filename):
+        yield filename
+
+
+@pytest.mark.parametrize("lock_type", [FileLock, SoftFileLock])
+def test_ro_file(lock_type, tmp_file_ro):
+    lock = lock_type(str(tmp_file_ro))
+    with pytest.raises(PermissionError, match="Permission denied"):
+        lock.acquire()
+
+
+@pytest.mark.parametrize("lock_type", [FileLock, SoftFileLock])
+def test_missing_directory(lock_type, tmp_path_ro):
+    lock_path = tmp_path_ro / "a" / "b"
+    lock = lock_type(str(lock_path))
+
+    with pytest.raises(OSError, match="No such file or directory:"):
+        lock.acquire()
+
+
 @pytest.mark.parametrize("lock_type", [FileLock, SoftFileLock])
 def test_nested_context_manager(lock_type, tmp_path):
     # lock is not released before the most outer with statement that locked the lock, is left
@@ -93,8 +142,8 @@ def test_nested_forced_release(lock_type, tmp_path):
 
 
 class ExThread(threading.Thread):
-    def __init__(self, target):
-        super(ExThread, self).__init__(target=target)
+    def __init__(self, target, name):
+        super(ExThread, self).__init__(target=target, name=name)
         self.ex = None
 
     def run(self):
@@ -106,6 +155,7 @@ def run(self):
     def join(self, timeout=None):
         super(ExThread, self).join(timeout=timeout)
         if self.ex is not None:
+            print("fail from thread {}".format(self.name))
             if sys.version_info[0] == 2:
                 wrapper_ex = self.ex[1]
                 raise (wrapper_ex.__class__, wrapper_ex, self.ex[2])
@@ -124,7 +174,7 @@ def thread_work():
             with lock:
                 assert lock.is_locked
 
-    threads = [ExThread(target=thread_work) for _ in range(100)]
+    threads = [ExThread(target=thread_work, name="t{}".format(i)) for i in range(100)]
     for thread in threads:
         thread.start()
     for thread in threads:
@@ -138,21 +188,21 @@ def test_threaded_lock_different_lock_obj(lock_type, tmp_path):
     # Runs multiple threads, which acquire the same lock file with a different FileLock object. When thread group 1
     # acquired the lock, thread group 2 must not hold their lock.
 
-    def thread_work_one():
+    def t_1():
         for _ in range(1000):
             with lock_1:
                 assert lock_1.is_locked
                 assert not lock_2.is_locked
 
-    def thread_work_two():
+    def t_2():
         for _ in range(1000):
             with lock_2:
                 assert not lock_1.is_locked
                 assert lock_2.is_locked
 
     lock_path = tmp_path / "a"
     lock_1, lock_2 = lock_type(str(lock_path)), lock_type(str(lock_path))
-    threads = [(ExThread(target=thread_work_one), ExThread(target=thread_work_two)) for i in range(10)]
+    threads = [(ExThread(t_1, "t1_{}".format(i)), ExThread(t_2, "t2_{}".format(i))) for i in range(10)]
 
     for thread_1, thread_2 in threads:
         thread_1.start()

tox.ini

@@ -113,3 +113,6 @@ max-line-length = 120
 
 [pep8]
 max-line-length = 120
+
+[pytest]
+timeout = 120

whitelist.txt

@@ -3,12 +3,18 @@ autodoc
 autosectionlabel
 caplog
 creat
+eacces
+eexist
+enoent
 exc
 fcntl
 filelock
 fmt
 intersphinx
 intervall
+iwgrp
+iwoth
+iwusr
 levelno
 lk
 lockfile
@@ -18,10 +24,12 @@ nitpicky
 param
 pygments
 rdwr
+ro
 skipif
 src
 tmp
 trunc
 typehints
 unlck
+util
 wronly