Raise exception if trying to create file in read-only directory

mrbean-bremen · web-flow · commit bc80ff8d0136 · 2017-06-16T18:30:43.000+02:00
- raise exception if trying to create file in read-only directory under Posix OS - fixed assertRaisesIOError and assertRaisesOSError - never allow adding of already existing file or directory - do not allow FakeFile without filesystem - fixes #203
diff --git a/CHANGES.md b/CHANGES.md
@@ -12,7 +12,7 @@ The release versions are PyPi releases.
 #### Infrastructure
 
 #### Fixes
-
+  * Creating files in read-only directory was possible ([#203](../../issues/203))
 
 ## [Version 3.2](https://pypi.python.org/pypi/pyfakefs/3.2)
 
diff --git a/fake_filesystem_test.py b/fake_filesystem_test.py
@@ -59,12 +59,14 @@ def assertModeEqual(self, expected, actual):
     def assertRaisesIOError(self, subtype, expression, *args, **kwargs):
         try:
             expression(*args, **kwargs)
+            self.fail('No exception was raised, IOError expected')
         except IOError as exc:
             self.assertEqual(exc.errno, subtype)
 
     def assertRaisesOSError(self, subtype, expression, *args, **kwargs):
         try:
             expression(*args, **kwargs)
+            self.fail('No exception was raised, OSError expected')
         except OSError as exc:
             if self.is_windows and sys.version_info < (3, 0):
                 self.assertEqual(exc.winerror, subtype)
@@ -76,8 +78,11 @@ class FakeDirectoryUnitTest(TestCase):
     def setUp(self):
         self.orig_time = time.time
         time.time = _DummyTime(10, 1)
-        self.fake_file = fake_filesystem.FakeFile('foobar', contents='dummy_file')
-        self.fake_dir = fake_filesystem.FakeDirectory('somedir')
+        self.filesystem = fake_filesystem.FakeFilesystem(path_separator='/')
+        self.fake_file = fake_filesystem.FakeFile(
+            'foobar', contents='dummy_file', filesystem=self.filesystem)
+        self.fake_dir = fake_filesystem.FakeDirectory(
+            'somedir', filesystem=self.filesystem)
 
     def tearDown(self):
         time.time = self.orig_time
@@ -96,6 +101,11 @@ def testGetEntry(self):
         self.fake_dir.AddEntry(self.fake_file)
         self.assertEqual(self.fake_file, self.fake_dir.GetEntry('foobar'))
 
+    def testGetPath(self):
+        self.filesystem.root.AddEntry(self.fake_dir)
+        self.fake_dir.AddEntry(self.fake_file)
+        self.assertEqual('/somedir/foobar', self.fake_file.GetPath())
+
     def testRemoveEntry(self):
         self.fake_dir.AddEntry(self.fake_file)
         self.assertEqual(self.fake_file, self.fake_dir.GetEntry('foobar'))
@@ -170,7 +180,8 @@ def testOrderedDirs(self):
 
 class SetLargeFileSizeTest(TestCase):
     def setUp(self):
-        self.fake_file = fake_filesystem.FakeFile('foobar')
+        filesystem = fake_filesystem.FakeFilesystem()
+        self.fake_file = fake_filesystem.FakeFile('foobar', filesystem=filesystem)
 
     def testShouldThrowIfSizeIsNotInteger(self):
         self.assertRaisesIOError(errno.ENOSPC, self.fake_file.SetLargeFileSize, 0.1)
@@ -246,9 +257,12 @@ class FakeFilesystemUnitTest(TestCase):
     def setUp(self):
         self.filesystem = fake_filesystem.FakeFilesystem(path_separator='/')
         self.root_name = '/'
-        self.fake_file = fake_filesystem.FakeFile('foobar')
-        self.fake_child = fake_filesystem.FakeDirectory('foobaz')
-        self.fake_grandchild = fake_filesystem.FakeDirectory('quux')
+        self.fake_file = fake_filesystem.FakeFile(
+            'foobar', filesystem=self.filesystem)
+        self.fake_child = fake_filesystem.FakeDirectory(
+            'foobaz', filesystem=self.filesystem)
+        self.fake_grandchild = fake_filesystem.FakeDirectory(
+            'quux', filesystem=self.filesystem)
 
     def testNewFilesystem(self):
         self.assertEqual('/', self.filesystem.path_separator)
@@ -427,6 +441,21 @@ def testCreateDirectoryAlreadyExistsError(self):
         self.filesystem.CreateDirectory(path)
         self.assertRaisesOSError(errno.EEXIST, self.filesystem.CreateDirectory, path)
 
+    def testCreateFileInReadOnlyDirectoryRaisesInPosix(self):
+        self.filesystem.is_windows_fs = False
+        dir_path = '/foo/bar'
+        self.filesystem.CreateDirectory(dir_path, perm_bits=0o555)
+        file_path = dir_path + '/baz'
+        self.assertRaisesOSError(errno.EACCES, self.filesystem.CreateFile, file_path)
+
+    def testCreateFileInReadOnlyDirectoryPossibleInWindows(self):
+        self.filesystem.is_windows_fs = True
+        dir_path = 'C:/foo/bar'
+        self.filesystem.CreateDirectory(dir_path, perm_bits=0o555)
+        file_path = dir_path + '/baz'
+        self.filesystem.CreateFile(file_path)
+        self.assertTrue(self.filesystem.Exists(file_path))
+
     def testCreateFileInCurrentDirectory(self):
         path = 'foo'
         contents = 'dummy data'
@@ -1418,6 +1447,7 @@ def testMkdirRaisesWithDoubleDots(self):
 
     def testMkdirRaisesIfParentIsReadOnly(self):
         """mkdir raises exception if parent is read only."""
+        self.filesystem.is_windows_fs = False
         directory = '/a'
         self.os.mkdir(directory)
 
@@ -1446,6 +1476,7 @@ def testMakedirsRaisesIfParentIsFile(self):
     def testMakedirsRaisesIfAccessDenied(self):
         """makedirs raises exception if access denied."""
         directory = '/a'
+        self.filesystem.is_windows_fs = False
         self.os.mkdir(directory)
 
         # Change directory permissions to be read only.
diff --git a/pyfakefs/fake_filesystem.py b/pyfakefs/fake_filesystem.py
@@ -347,7 +347,7 @@ def __init__(self, name, st_mode=stat.S_IFREG | PERM_DEF_FILE,
           contents:  the contents of the filesystem object; should be a string or byte object for
             regular files, and a list of other FakeFile or FakeDirectory objects
             for FakeDirectory objects
-          filesystem: if set, the fake filesystem where the file is created.
+          filesystem: the fake filesystem where the file is created.
             New in pyfakefs 2.9.
           encoding: if contents is a unicode string, the encoding used for serialization
           errors: the error mode used for encoding/decoding errors
@@ -361,8 +361,12 @@ def __init__(self, name, st_mode=stat.S_IFREG | PERM_DEF_FILE,
         self._byte_contents = self._encode_contents(contents)
         self.stat_result.st_size = (
             len(self._byte_contents) if self._byte_contents is not None else 0)
+        # to be backwards compatible regarding argument order, we raise on None
+        if filesystem is None:
+            raise ValueError('filesystem shall not be None')
         self.filesystem = filesystem
         self.epoch = 0
+        self.parent_dir = None
 
     @property
     def byte_contents(self):
@@ -400,8 +404,7 @@ def SetLargeFileSize(self, st_size):
                           self.name)
         if self.st_size:
             self.SetSize(0)
-        if self.filesystem:
-            self.filesystem.ChangeDiskUsage(st_size, self.name, self.st_dev)
+        self.filesystem.ChangeDiskUsage(st_size, self.name, self.st_dev)
         self.st_size = st_size
         self._byte_contents = None
 
@@ -433,8 +436,7 @@ def _set_initial_contents(self, contents):
         if self._byte_contents:
             self.SetSize(0)
         current_size = self.st_size or 0
-        if self.filesystem:
-            self.filesystem.ChangeDiskUsage(st_size - current_size, self.name, self.st_dev)
+        self.filesystem.ChangeDiskUsage(st_size - current_size, self.name, self.st_dev)
         self._byte_contents = contents
         self.st_size = st_size
         self.epoch += 1
@@ -465,6 +467,16 @@ def GetSize(self):
         """
         return self.st_size
 
+    def GetPath(self):
+        """Return the full path of the current object."""
+        names = []
+        obj = self
+        while obj:
+            names.insert(0, obj.name)
+            obj = obj.parent_dir
+        sep = self.filesystem._path_separator(self.name)
+        return self.filesystem.NormalizePath(sep.join(names[1:]))
+
     def SetSize(self, st_size):
         """Resizes file content, padding with nulls if new size exceeds the old.
 
@@ -483,8 +495,7 @@ def SetSize(self, st_size):
                           self.name)
 
         current_size = self.st_size or 0
-        if self.filesystem:
-            self.filesystem.ChangeDiskUsage(st_size - current_size, self.name, self.st_dev)
+        self.filesystem.ChangeDiskUsage(st_size - current_size, self.name, self.st_dev)
         if self._byte_contents:
             if st_size < current_size:
                 self._byte_contents = self._byte_contents[:st_size]
@@ -620,12 +631,25 @@ def AddEntry(self, path_object):
         """Adds a child FakeFile to this directory.
 
         Args:
-          path_object:  FakeFile instance to add as a child of this directory.
+            path_object:  FakeFile instance to add as a child of this directory.
+
+        Raises:
+            OSError: if the directory has no write permission (Posix only)
+            OSError: if the file or directory to be added already exists
         """
+        if not self.st_mode & PERM_WRITE and not self.filesystem.is_windows_fs:
+            raise OSError(errno.EACCES, 'Permission Denied', self.GetPath())
+
+        if path_object.name in self.contents:
+            raise OSError(errno.EEXIST,
+                          'Object already exists in fake filesystem',
+                          self.GetPath())
+
         self.contents[path_object.name] = path_object
+        path_object.parent_dir = self
         path_object.st_nlink += 1
         path_object.st_dev = self.st_dev
-        if self.filesystem and path_object.st_nlink == 1:
+        if path_object.st_nlink == 1:
             self.filesystem.ChangeDiskUsage(path_object.GetSize(), path_object.name, self.st_dev)
 
     def GetEntry(self, pathname_name):
@@ -659,12 +683,12 @@ def RemoveEntry(self, pathname_name, recursive=True):
         if entry.st_mode & PERM_WRITE == 0:
             raise OSError(errno.EACCES, 'Trying to remove object without write permission',
                           pathname_name)
-        if self.filesystem and self.filesystem.is_windows_fs and self.filesystem.HasOpenFile(entry):
+        if self.filesystem.is_windows_fs and self.filesystem.HasOpenFile(entry):
             raise OSError(errno.EACCES, 'Trying to remove an open file', pathname_name)
         if recursive and isinstance(entry, FakeDirectory):
             while entry.contents:
                 entry.RemoveEntry(list(entry.contents)[0])
-        elif self.filesystem and entry.st_nlink == 1:
+        elif entry.st_nlink == 1:
             self.filesystem.ChangeDiskUsage(-entry.GetSize(), pathname_name, entry.st_dev)
 
         entry.st_nlink -= 1
@@ -1872,15 +1896,22 @@ def CreateDirectory(self, directory_path, perm_bits=PERM_DEF):
         path_components = self.GetPathComponents(directory_path)
         current_dir = self.root
 
+        new_dirs = []
         for component in path_components:
             directory = self._DirectoryContent(current_dir, component)[1]
             if not directory:
-                new_dir = FakeDirectory(component, perm_bits, filesystem=self)
+                new_dir = FakeDirectory(component, filesystem=self)
+                new_dirs.append(new_dir)
                 current_dir.AddEntry(new_dir)
                 current_dir = new_dir
             else:
                 current_dir = directory
 
+        # set the permission after creating the directories
+        # to allow directory creation inside a read-only directory
+        for new_dir in new_dirs:
+            new_dir.st_mode = stat.S_IFDIR | perm_bits
+
         self.last_ino += 1
         current_dir.SetIno(self.last_ino)
         return current_dir
@@ -2197,12 +2228,9 @@ def MakeDirectory(self, dir_name, mode=PERM_DEF):
         if self.Exists(dir_name):
             raise OSError(errno.EEXIST, 'Fake object already exists', dir_name)
         head, tail = self.SplitPath(dir_name)
-        directory_object = self.GetObject(head)
-        if not directory_object.st_mode & PERM_WRITE:
-            raise OSError(errno.EACCES, 'Permission Denied', dir_name)
 
         self.AddObject(
-            head, FakeDirectory(tail, mode & ~self.umask))
+            head, FakeDirectory(tail, mode & ~self.umask, filesystem=self))
 
     def MakeDirectories(self, dir_name, mode=PERM_DEF, exist_ok=False):
         """Create a leaf Fake directory and create any non-existent parent dirs.
@@ -2228,10 +2256,7 @@ def MakeDirectories(self, dir_name, mode=PERM_DEF, exist_ok=False):
         current_dir = self.root
         for component in path_components:
             if component not in current_dir.contents:
-                if not current_dir.st_mode & PERM_WRITE:
-                    raise OSError(errno.EACCES, 'Permission Denied', dir_name)
-                else:
-                    break
+                break
             else:
                 current_dir = current_dir.contents[component]
         try:
