[Jython-checkins] jython: Fix #2083, to prevent os.unlink() deleting directory.

[jeff.allen]

http://hg.python.org/jython/rev/dc9b1aa30f6d
changeset:   7169:dc9b1aa30f6d
parent:      7167:b31e71644fa8
user:        Santoso Wijaya <santoso.wijaya at gmail.com>
date:        Sat Dec 21 20:18:51 2013 +0000
summary:
  Fix #2083, to prevent os.unlink() deleting directory.
Also adds test to test_os_jy and a NEWS entry (JA).

files:
  Lib/test/test_os_jy.py                        |  34 +++++++++-
  NEWS                                          |   1 +
  src/org/python/modules/posix/PosixModule.java |  15 ++--
  3 files changed, 39 insertions(+), 11 deletions(-)


diff --git a/Lib/test/test_os_jy.py b/Lib/test/test_os_jy.py
--- a/Lib/test/test_os_jy.py
+++ b/Lib/test/test_os_jy.py
@@ -6,7 +6,7 @@
 import unittest
 from test import test_support
 
-class OSTestCase(unittest.TestCase):
+class OSFileTestCase(unittest.TestCase):
 
     def setUp(self):
         open(test_support.TESTFN, 'w').close()
@@ -50,9 +50,37 @@
                 self.assertTrue(False)
 
 
+class OSDirTestCase(unittest.TestCase):
+
+    def setUp(self):
+        self.base = test_support.TESTFN
+        self.path = os.path.join(self.base, 'dir1', 'dir2', 'dir3')
+        os.makedirs(self.path)
+
+    def test_rmdir(self):
+        # Remove end directory
+        os.rmdir(self.path)
+        # Fail to remove a chain of directories
+        self.assertRaises(OSError, os.rmdir, self.base)
+
+    def test_issue2083(self):
+        # Should fail to remove/unlink directory
+        self.assertRaises(OSError, os.remove, self.path)
+        self.assertRaises(OSError, os.unlink, self.path)
+
+    def tearDown(self):
+        # Some dirs may have been deleted. Find the longest that exists.
+        p = self.path
+        while not os.path.exists(p) and p != self.base:
+            p = os.path.dirname(p)
+        os.removedirs(p)
+
+
 def test_main():
-    test_support.run_unittest(OSTestCase)
-
+    test_support.run_unittest(
+        OSFileTestCase, 
+        OSDirTestCase,
+    )
 
 if __name__ == '__main__':
     test_main()
diff --git a/NEWS b/NEWS
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,7 @@
     - [ 2060 ] Thread ident missing
     - [ 2071 ] datetime strftime %f does not work
     - [ 2082 ] Unexpected (Pdb) prompt during regression tests
+    - [ 2083 ] os.unlink() can delete directories
 
 Jython 2.7b1
   Bugs Fixed
diff --git a/src/org/python/modules/posix/PosixModule.java b/src/org/python/modules/posix/PosixModule.java
--- a/src/org/python/modules/posix/PosixModule.java
+++ b/src/org/python/modules/posix/PosixModule.java
@@ -722,19 +722,18 @@
         return posix.umask(mask);
     }
 
-    public static PyString __doc__unlink = new PyString(
-        "unlink(path)\n\n" +
-        "Remove a file (same as remove(path)).");
+    public static PyString __doc__unlink = new PyString("unlink(path)\n\n"
+            + "Remove a file (same as remove(path)).");
+
     public static void unlink(PyObject path) {
         String absolutePath = absolutePath(path);
         File file = new File(absolutePath);
-        if (!file.delete()) {
+        if (file.isDirectory()) {
+            throw Py.OSError(Errno.EISDIR, path);
+        } else if (!file.delete()) {
             // Something went wrong, does stat raise an error?
             posix.stat(absolutePath);
-            // It exists, is it a directory, or do we not have permissions?
-            if (file.isDirectory()) {
-                throw Py.OSError(Errno.EISDIR, path);
-            }
+            // It exists, do we not have permissions?
             if (!file.canWrite()) {
                 throw Py.OSError(Errno.EPERM, path);
             }

-- 
Repository URL: http://hg.python.org/jython