#8013 follow-up:

* In asctime and ctime, properly remove the newline if the year has more than four digits
* Consistent error message for both functions
* Fix the test comments and add a check for the removed newline

2 files changed, 33 insertions, 13 deletions

diff --git a/Lib/test/test_time.py b/Lib/test/test_time.py
index 5d2ae42..6451b27 100644
--- a/Lib/test/test_time.py
+++ b/Lib/test/test_time.py
@@ -123,19 +123,33 @@ class TimeTestCase(unittest.TestCase):
         time.asctime(time.gmtime(self.t))
         self.assertRaises(TypeError, time.asctime, 0)
         self.assertRaises(TypeError, time.asctime, ())
-        # XXX: Posix compiant asctime should refuse to convert
-        # year > 9999, but Linux implementation does not.
-        # self.assertRaises(ValueError, time.asctime,
-        #                  (12345, 1, 0, 0, 0, 0, 0, 0, 0))
-        # XXX: For now, just make sure we don't have a crash:
+        # XXX: POSIX-compliant asctime should refuse to convert year > 9999,
+        # but glibc implementation does not.  For now, just check it doesn't
+        # segfault as it did before, and the result contains no newline.
         try:
-            time.asctime((12345, 1, 0, 0, 0, 0, 0, 0, 0))
+            result = time.asctime((12345, 1, 0, 0, 0, 0, 0, 0, 0))
         except ValueError:
+            # for POSIX-compliant runtimes
             pass
+        else:
+            self.assertNotIn('\n', result)
 
     def test_asctime_bounding_check(self):
         self._bounds_checking(time.asctime)
 
+    def test_ctime(self):
+        # XXX: POSIX-compliant ctime should refuse to convert year > 9999,
+        # but glibc implementation does not.  For now, just check it doesn't
+        # segfault as it did before, and the result contains no newline.
+        try:
+            result = time.ctime(1e12)
+        except ValueError:
+            # for POSIX-compliant runtimes (or 32-bit systems, where time_t
+            # cannot hold timestamps with a five-digit year)
+            pass
+        else:
+            self.assertNotIn('\n', result)
+
     @unittest.skipIf(not hasattr(time, "tzset"),
         "time module has no attribute tzset")
     def test_tzset(self):
diff --git a/Modules/timemodule.c b/Modules/timemodule.c
index e8ea661..4a0bacb 100644
--- a/Modules/timemodule.c
+++ b/Modules/timemodule.c
@@ -611,7 +611,7 @@ time_asctime(PyObject *self, PyObject *args)
 {
     PyObject *tup = NULL;
     struct tm buf;
-    char *p;
+    char *p, *q;
     if (!PyArg_UnpackTuple(args, "asctime", 0, 1, &tup))
         return NULL;
     if (tup == NULL) {
@@ -621,11 +621,14 @@ time_asctime(PyObject *self, PyObject *args)
         return NULL;
     p = asctime(&buf);
     if (p == NULL) {
-        PyErr_SetString(PyExc_ValueError, "invalid time");
+        PyErr_SetString(PyExc_ValueError, "unconvertible time");
         return NULL;
     }
-    if (p[24] == '\n')
-        p[24] = '\0';
+    /* Replace a terminating newline by a null byte, normally at position 24.
+     * It can occur later if the year has more than four digits. */
+    for (q = p+24; *q != '\0'; q++)
+        if (*q == '\n')
+            *q = '\0';
     return PyUnicode_FromString(p);
 }
 
@@ -641,7 +644,7 @@ time_ctime(PyObject *self, PyObject *args)
 {
     PyObject *ot = NULL;
     time_t tt;
-    char *p;
+    char *p, *q;
 
     if (!PyArg_UnpackTuple(args, "ctime", 0, 1, &ot))
         return NULL;
@@ -660,8 +663,11 @@ time_ctime(PyObject *self, PyObject *args)
         PyErr_SetString(PyExc_ValueError, "unconvertible time");
         return NULL;
     }
-    if (p[24] == '\n')
-        p[24] = '\0';
+    /* Replace a terminating newline by a null byte, normally at position 24.
+     * It can occur later if the year has more than four digits. */
+    for (q = p+24; *q != '\0'; q++)
+        if (*q == '\n')
+            *q = '\0';
     return PyUnicode_FromString(p);
 }