bpo-33106: change dbm key deletion error for readonly file from KeyError to dbm.error (#6295)

9 files changed, 57 insertions, 9 deletions

diff --git a/Doc/library/dbm.rst b/Doc/library/dbm.rst
index ab45cac..57ae547 100644
--- a/Doc/library/dbm.rst
+++ b/Doc/library/dbm.rst
@@ -73,6 +73,10 @@ available, as well as :meth:`get` and :meth:`setdefault`.
 .. versionchanged:: 3.2
    :meth:`get` and :meth:`setdefault` are now available in all database modules.
 
+.. versionchanged:: 3.8
+   Deleting a key from a read-only database raises database module specific error
+   instead of :exc:`KeyError`.
+
 Key and values are always stored as bytes. This means that when
 strings are used they are implicitly converted to the default encoding before
 being stored.
diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst
index 0d0d1ca..b38a1a7 100644
--- a/Doc/whatsnew/3.8.rst
+++ b/Doc/whatsnew/3.8.rst
@@ -483,6 +483,12 @@ Changes in the Python API
   external entities by default.
   (Contributed by Christian Heimes in :issue:`17239`.)
 
+* Deleting a key from a read-only :mod:`dbm` database (:mod:`dbm.dumb`,
+  :mod:`dbm.gnu` or :mod:`dbm.ndbm`) raises :attr:`error` (:exc:`dbm.dumb.error`,
+  :exc:`dbm.gnu.error` or :exc:`dbm.ndbm.error`) instead of :exc:`KeyError`.
+  (Contributed by Xiang Zhang in :issue:`33106`.)
+
+
 CPython bytecode changes
 ------------------------
 
diff --git a/Lib/dbm/dumb.py b/Lib/dbm/dumb.py
index e5c17f5..6cef72a 100644
--- a/Lib/dbm/dumb.py
+++ b/Lib/dbm/dumb.py
@@ -185,7 +185,7 @@ class _Database(collections.abc.MutableMapping):
 
     def __setitem__(self, key, val):
         if self._readonly:
-            raise ValueError('The database is opened for reading only')
+            raise error('The database is opened for reading only')
         if isinstance(key, str):
             key = key.encode('utf-8')
         elif not isinstance(key, (bytes, bytearray)):
@@ -222,7 +222,7 @@ class _Database(collections.abc.MutableMapping):
 
     def __delitem__(self, key):
         if self._readonly:
-            raise ValueError('The database is opened for reading only')
+            raise error('The database is opened for reading only')
         if isinstance(key, str):
             key = key.encode('utf-8')
         self._verify_open()
diff --git a/Lib/test/test_dbm_dumb.py b/Lib/test/test_dbm_dumb.py
index 58b9d17..e8e827e 100644
--- a/Lib/test/test_dbm_dumb.py
+++ b/Lib/test/test_dbm_dumb.py
@@ -82,10 +82,10 @@ class DumbDBMTestCase(unittest.TestCase):
         self.init_db()
         f = dumbdbm.open(_fname, 'r')
         self.read_helper(f)
-        with self.assertRaisesRegex(ValueError,
+        with self.assertRaisesRegex(dumbdbm.error,
                                    'The database is opened for reading only'):
             f[b'g'] = b'x'
-        with self.assertRaisesRegex(ValueError,
+        with self.assertRaisesRegex(dumbdbm.error,
                                    'The database is opened for reading only'):
             del f[b'a']
         # get() works as in the dict interface
diff --git a/Lib/test/test_dbm_gnu.py b/Lib/test/test_dbm_gnu.py
index 16b7fe6..f1c7d34 100644
--- a/Lib/test/test_dbm_gnu.py
+++ b/Lib/test/test_dbm_gnu.py
@@ -131,6 +131,17 @@ class TestGdbm(unittest.TestCase):
             self.assertEqual(db['Unicode key \U0001f40d'],
                              'Unicode value \U0001f40d'.encode())
 
+    def test_write_readonly_file(self):
+        with gdbm.open(filename, 'c') as db:
+            db[b'bytes key'] = b'bytes value'
+        with gdbm.open(filename, 'r') as db:
+            with self.assertRaises(gdbm.error):
+                del db[b'not exist key']
+            with self.assertRaises(gdbm.error):
+                del db[b'bytes key']
+            with self.assertRaises(gdbm.error):
+                db[b'not exist key'] = b'not exist value'
+
     @unittest.skipUnless(TESTFN_NONASCII,
                          'requires OS support of non-ASCII encodings')
     def test_nonascii_filename(self):
diff --git a/Lib/test/test_dbm_ndbm.py b/Lib/test/test_dbm_ndbm.py
index bd411da..7ac75c5 100644
--- a/Lib/test/test_dbm_ndbm.py
+++ b/Lib/test/test_dbm_ndbm.py
@@ -90,6 +90,17 @@ class DbmTestCase(unittest.TestCase):
             self.assertEqual(db['Unicode key \U0001f40d'],
                              'Unicode value \U0001f40d'.encode())
 
+    def test_write_readonly_file(self):
+        with dbm.ndbm.open(self.filename, 'c') as db:
+            db[b'bytes key'] = b'bytes value'
+        with dbm.ndbm.open(self.filename, 'r') as db:
+            with self.assertRaises(error):
+                del db[b'not exist key']
+            with self.assertRaises(error):
+                del db[b'bytes key']
+            with self.assertRaises(error):
+                db[b'not exist key'] = b'not exist value'
+
     @unittest.skipUnless(support.TESTFN_NONASCII,
                          'requires OS support of non-ASCII encodings')
     def test_nonascii_filename(self):
diff --git a/Misc/NEWS.d/next/Library/2018-03-30-01-20-35.bpo-33106.zncfvW.rst b/Misc/NEWS.d/next/Library/2018-03-30-01-20-35.bpo-33106.zncfvW.rst
new file mode 100644
index 0000000..859c09c
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2018-03-30-01-20-35.bpo-33106.zncfvW.rst
@@ -0,0 +1,2 @@
+Deleting a key from a read-only dbm database raises module specfic error
+instead of KeyError.
diff --git a/Modules/_dbmmodule.c b/Modules/_dbmmodule.c
index 081184a..21784ae 100644
--- a/Modules/_dbmmodule.c
+++ b/Modules/_dbmmodule.c
@@ -36,6 +36,7 @@ class _dbm.dbm "dbmobject *" "&Dbmtype"
 
 typedef struct {
     PyObject_HEAD
+    int flags;
     int di_size;        /* -1 means recompute */
     DBM *di_dbm;
 } dbmobject;
@@ -60,6 +61,7 @@ newdbmobject(const char *file, int flags, int mode)
     if (dp == NULL)
         return NULL;
     dp->di_size = -1;
+    dp->flags = flags;
     /* See issue #19296 */
     if ( (dp->di_dbm = dbm_open((char *)file, flags, mode)) == 0 ) {
         PyErr_SetFromErrnoWithFilename(DbmError, file);
@@ -143,13 +145,20 @@ dbm_ass_sub(dbmobject *dp, PyObject *v, PyObject *w)
     if (w == NULL) {
         if ( dbm_delete(dp->di_dbm, krec) < 0 ) {
             dbm_clearerr(dp->di_dbm);
-            PyErr_SetObject(PyExc_KeyError, v);
+            /* we might get a failure for reasons like file corrupted,
+               but we are not able to distinguish it */
+            if (dp->flags & O_RDWR) {
+                PyErr_SetObject(PyExc_KeyError, v);
+            }
+            else {
+                PyErr_SetString(DbmError, "cannot delete item from database");
+            }
             return -1;
         }
     } else {
         if ( !PyArg_Parse(w, "s#", &drec.dptr, &tmp_size) ) {
             PyErr_SetString(PyExc_TypeError,
-                 "dbm mappings have byte or string elements only");
+                 "dbm mappings have bytes or string elements only");
             return -1;
         }
         drec.dsize = tmp_size;
@@ -335,7 +344,7 @@ _dbm_dbm_setdefault_impl(dbmobject *self, const char *key,
     else {
         if ( !PyArg_Parse(default_value, "s#", &val.dptr, &tmp_size) ) {
             PyErr_SetString(PyExc_TypeError,
-                "dbm mappings have byte string elements only");
+                "dbm mappings have bytes or string elements only");
             return NULL;
         }
         val.dsize = tmp_size;
diff --git a/Modules/_gdbmmodule.c b/Modules/_gdbmmodule.c
index 46bb59a..cc94e60 100644
--- a/Modules/_gdbmmodule.c
+++ b/Modules/_gdbmmodule.c
@@ -186,14 +186,19 @@ dbm_ass_sub(dbmobject *dp, PyObject *v, PyObject *w)
     dp->di_size = -1;
     if (w == NULL) {
         if (gdbm_delete(dp->di_dbm, krec) < 0) {
-            PyErr_SetObject(PyExc_KeyError, v);
+            if (gdbm_errno == GDBM_ITEM_NOT_FOUND) {
+                PyErr_SetObject(PyExc_KeyError, v);
+            }
+            else {
+                PyErr_SetString(DbmError, gdbm_strerror(gdbm_errno));
+            }
             return -1;
         }
     }
     else {
         if (!PyArg_Parse(w, "s#", &drec.dptr, &drec.dsize)) {
             PyErr_SetString(PyExc_TypeError,
-                            "gdbm mappings have byte or string elements only");
+                            "gdbm mappings have bytes or string elements only");
             return -1;
         }
         errno = 0;