Issue #18829: csv.Dialect() now checks type for delimiter, escapechar and

quotechar fields.  Original patch by Vajrasky Kok.

3 files changed, 62 insertions, 19 deletions

diff --git a/Lib/test/test_csv.py b/Lib/test/test_csv.py
index 5c95fe3..e031170 100644
--- a/Lib/test/test_csv.py
+++ b/Lib/test/test_csv.py
@@ -870,6 +870,7 @@ class TestDialectValidity(unittest.TestCase):
             lineterminator = '\r\n'
             quoting = csv.QUOTE_NONE
         d = mydialect()
+        self.assertEqual(d.quoting, csv.QUOTE_NONE)
 
         mydialect.quoting = None
         self.assertRaises(csv.Error, mydialect)
@@ -878,12 +879,21 @@ class TestDialectValidity(unittest.TestCase):
         mydialect.quoting = csv.QUOTE_ALL
         mydialect.quotechar = '"'
         d = mydialect()
+        self.assertEqual(d.quoting, csv.QUOTE_ALL)
+        self.assertEqual(d.quotechar, '"')
+        self.assertTrue(d.doublequote)
 
         mydialect.quotechar = "''"
-        self.assertRaises(csv.Error, mydialect)
+        with self.assertRaises(csv.Error) as cm:
+            mydialect()
+        self.assertEqual(str(cm.exception),
+                         '"quotechar" must be an 1-character string')
 
         mydialect.quotechar = 4
-        self.assertRaises(csv.Error, mydialect)
+        with self.assertRaises(csv.Error) as cm:
+            mydialect()
+        self.assertEqual(str(cm.exception),
+                         '"quotechar" must be string, not int')
 
     def test_delimiter(self):
         class mydialect(csv.Dialect):
@@ -894,12 +904,31 @@ class TestDialectValidity(unittest.TestCase):
             lineterminator = '\r\n'
             quoting = csv.QUOTE_NONE
         d = mydialect()
+        self.assertEqual(d.delimiter, ";")
 
         mydialect.delimiter = ":::"
-        self.assertRaises(csv.Error, mydialect)
+        with self.assertRaises(csv.Error) as cm:
+            mydialect()
+        self.assertEqual(str(cm.exception),
+                         '"delimiter" must be an 1-character string')
+
+        mydialect.delimiter = ""
+        with self.assertRaises(csv.Error) as cm:
+            mydialect()
+        self.assertEqual(str(cm.exception),
+                         '"delimiter" must be an 1-character string')
+
+        mydialect.delimiter = u","
+        with self.assertRaises(csv.Error) as cm:
+            mydialect()
+        self.assertEqual(str(cm.exception),
+                         '"delimiter" must be string, not unicode')
 
         mydialect.delimiter = 4
-        self.assertRaises(csv.Error, mydialect)
+        with self.assertRaises(csv.Error) as cm:
+            mydialect()
+        self.assertEqual(str(cm.exception),
+                         '"delimiter" must be string, not int')
 
     def test_lineterminator(self):
         class mydialect(csv.Dialect):
@@ -910,12 +939,17 @@ class TestDialectValidity(unittest.TestCase):
             lineterminator = '\r\n'
             quoting = csv.QUOTE_NONE
         d = mydialect()
+        self.assertEqual(d.lineterminator, '\r\n')
 
         mydialect.lineterminator = ":::"
         d = mydialect()
+        self.assertEqual(d.lineterminator, ":::")
 
         mydialect.lineterminator = 4
-        self.assertRaises(csv.Error, mydialect)
+        with self.assertRaises(csv.Error) as cm:
+            mydialect()
+        self.assertEqual(str(cm.exception),
+                         '"lineterminator" must be a string')
 
 
 class TestSniffer(unittest.TestCase):
diff --git a/Misc/NEWS b/Misc/NEWS
index ec5a483..35da6ab 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -27,6 +27,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #18829: csv.Dialect() now checks type for delimiter, escapechar and
+  quotechar fields.  Original patch by Vajrasky Kok.
+
 - Issue #19855: uuid.getnode() on Unix now looks on the PATH for the
   executables used to find the mac address, with /sbin and /usr/sbin as
   fallbacks.
diff --git a/Modules/_csv.c b/Modules/_csv.c
index fd6121f..00f5d00 100644
--- a/Modules/_csv.c
+++ b/Modules/_csv.c
@@ -239,19 +239,24 @@ _set_char(const char *name, char *target, PyObject *src, char dflt)
     if (src == NULL)
         *target = dflt;
     else {
-        if (src == Py_None || PyString_Size(src) == 0)
-            *target = '\0';
-        else if (!PyString_Check(src) || PyString_Size(src) != 1) {
-            PyErr_Format(PyExc_TypeError,
-                         "\"%s\" must be an 1-character string",
-                         name);
-            return -1;
-        }
-        else {
-            char *s = PyString_AsString(src);
-            if (s == NULL)
+        *target = '\0';
+        if (src != Py_None) {
+            Py_ssize_t len;
+            if (!PyString_Check(src)) {
+                PyErr_Format(PyExc_TypeError,
+                    "\"%s\" must be string, not %.200s", name,
+                    src->ob_type->tp_name);
                 return -1;
-            *target = s[0];
+            }
+            len = PyString_GET_SIZE(src);
+            if (len > 1) {
+                PyErr_Format(PyExc_TypeError,
+                    "\"%s\" must be an 1-character string",
+                    name);
+                return -1;
+            }
+            if (len > 0)
+                *target = *PyString_AS_STRING(src);
         }
     }
     return 0;
@@ -267,7 +272,7 @@ _set_str(const char *name, PyObject **target, PyObject *src, const char *dflt)
             *target = NULL;
         else if (!IS_BASESTRING(src)) {
             PyErr_Format(PyExc_TypeError,
-                         "\"%s\" must be an string", name);
+                         "\"%s\" must be a string", name);
             return -1;
         }
         else {
@@ -426,7 +431,8 @@ dialect_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
     if (dialect_check_quoting(self->quoting))
         goto err;
     if (self->delimiter == 0) {
-        PyErr_SetString(PyExc_TypeError, "delimiter must be set");
+        PyErr_SetString(PyExc_TypeError,
+                        "\"delimiter\" must be an 1-character string");
         goto err;
     }
     if (quotechar == Py_None && quoting == NULL)