From 4d8c8c0ad6163c24136d3419eb04f310b31f7e64 Mon Sep 17 00:00:00 2001 From: Paul Ganssle Date: Sat, 27 Apr 2019 15:39:40 -0400 Subject: [PATCH] bpo-36025: Fix PyDate_FromTimestamp API (GH-11922) In the process of converting the date.fromtimestamp function to use argument clinic in GH-8535, the C API for PyDate_FromTimestamp was inadvertently changed to expect a timestamp object rather than an argument tuple. This PR fixes this backwards-incompatible change by adding a new wrapper function for the C API function that unwraps the argument tuple and passes it to the underlying function. This PR also adds tests for both PyDate_FromTimestamp and PyDateTime_FromTimestamp to prevent any further regressions. --- Lib/test/datetimetester.py | 35 ++++++++++ .../2019-02-19-08-23-42.bpo-36025.tnwylQ.rst | 5 ++ Modules/_datetimemodule.c | 19 ++++- Modules/_testcapimodule.c | 69 ++++++++++++++++++- 4 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2019-02-19-08-23-42.bpo-36025.tnwylQ.rst diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 715f0ea6b40..617bf9a2558 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -5942,6 +5942,41 @@ class CapiTest(unittest.TestCase): with self.subTest(arg=arg, exact=exact): self.assertFalse(is_tzinfo(arg, exact)) + def test_date_from_timestamp(self): + ts = datetime(1995, 4, 12).timestamp() + + for macro in [0, 1]: + with self.subTest(macro=macro): + d = _testcapi.get_date_fromtimestamp(int(ts), macro) + + self.assertEqual(d, date(1995, 4, 12)) + + def test_datetime_from_timestamp(self): + ts0 = datetime(1995, 4, 12).timestamp() + ts1 = datetime(1995, 4, 12, 12, 30).timestamp() + + cases = [ + ((1995, 4, 12), None, False), + ((1995, 4, 12), None, True), + ((1995, 4, 12), timezone(timedelta(hours=1)), True), + ((1995, 4, 12, 14, 30), None, False), + ((1995, 4, 12, 14, 30), None, True), + ((1995, 4, 12, 14, 30), timezone(timedelta(hours=1)), True), + ] + + from_timestamp = _testcapi.get_datetime_fromtimestamp + for case in cases: + for macro in [0, 1]: + with self.subTest(case=case, macro=macro): + dtup, tzinfo, usetz = case + dt_orig = datetime(*dtup, tzinfo=tzinfo) + ts = int(dt_orig.timestamp()) + + dt_rt = from_timestamp(ts, tzinfo, usetz, macro) + + self.assertEqual(dt_orig, dt_rt) + + def load_tests(loader, standard_tests, pattern): standard_tests.addTest(ZoneInfoCompleteTest()) return standard_tests diff --git a/Misc/NEWS.d/next/C API/2019-02-19-08-23-42.bpo-36025.tnwylQ.rst b/Misc/NEWS.d/next/C API/2019-02-19-08-23-42.bpo-36025.tnwylQ.rst new file mode 100644 index 00000000000..b00a33d109c --- /dev/null +++ b/Misc/NEWS.d/next/C API/2019-02-19-08-23-42.bpo-36025.tnwylQ.rst @@ -0,0 +1,5 @@ +Fixed an accidental change to the datetime C API where the arguments to the +:c:func:`PyDate_FromTimestamp` function were incorrectly interpreted as a +single timestamp rather than an arguments tuple, which causes existing code to +start raising :exc:`TypeError`. The backwards-incompatible change was only +present in alpha releases of Python 3.8. Patch by Paul Ganssle. diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index c1557b5e6f4..b3954c98ee7 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -2921,6 +2921,23 @@ datetime_date_fromtimestamp(PyTypeObject *type, PyObject *timestamp) return date_fromtimestamp((PyObject *) type, timestamp); } +/* bpo-36025: This is a wrapper for API compatibility with the public C API, + * which expects a function that takes an *args tuple, whereas the argument + * clinic generates code that takes METH_O. + */ +static PyObject * +datetime_date_fromtimestamp_capi(PyObject *cls, PyObject *args) +{ + PyObject *timestamp; + PyObject *result = NULL; + + if (PyArg_UnpackTuple(args, "fromtimestamp", 1, 1, ×tamp)) { + result = date_fromtimestamp(cls, timestamp); + } + + return result; +} + /* Return new date from proleptic Gregorian ordinal. Raises ValueError if * the ordinal is out of range. */ @@ -6275,7 +6292,7 @@ static PyDateTime_CAPI CAPI = { new_delta_ex, new_timezone, datetime_fromtimestamp, - date_fromtimestamp, + datetime_date_fromtimestamp_capi, new_datetime_ex2, new_time_ex2 }; diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 6f4eb53d7e9..c52e3499638 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2340,6 +2340,71 @@ get_timezone_utc_capi(PyObject* self, PyObject *args) { } } +static PyObject * +get_date_fromtimestamp(PyObject* self, PyObject *args) +{ + PyObject *tsargs = NULL, *ts = NULL, *rv = NULL; + int macro = 0; + + if (!PyArg_ParseTuple(args, "O|p", &ts, ¯o)) { + return NULL; + } + + // Construct the argument tuple + if ((tsargs = PyTuple_Pack(1, ts)) == NULL) { + return NULL; + } + + // Pass along to the API function + if (macro) { + rv = PyDate_FromTimestamp(tsargs); + } + else { + rv = PyDateTimeAPI->Date_FromTimestamp( + (PyObject *)PyDateTimeAPI->DateType, tsargs + ); + } + + Py_DECREF(tsargs); + return rv; +} + +static PyObject * +get_datetime_fromtimestamp(PyObject* self, PyObject *args) +{ + int macro = 0; + int usetz = 0; + PyObject *tsargs = NULL, *ts = NULL, *tzinfo = Py_None, *rv = NULL; + if (!PyArg_ParseTuple(args, "OO|pp", &ts, &tzinfo, &usetz, ¯o)) { + return NULL; + } + + // Construct the argument tuple + if (usetz) { + tsargs = PyTuple_Pack(2, ts, tzinfo); + } + else { + tsargs = PyTuple_Pack(1, ts); + } + + if (tsargs == NULL) { + return NULL; + } + + // Pass along to the API function + if (macro) { + rv = PyDateTime_FromTimestamp(tsargs); + } + else { + rv = PyDateTimeAPI->DateTime_FromTimestamp( + (PyObject *)PyDateTimeAPI->DateTimeType, tsargs, NULL + ); + } + + Py_DECREF(tsargs); + return rv; +} + /* test_thread_state spawns a thread of its own, and that thread releases * `thread_done` when it's finished. The driver code has to know when the @@ -4769,7 +4834,9 @@ static PyMethodDef TestMethods[] = { {"datetime_check_tzinfo", datetime_check_tzinfo, METH_VARARGS}, {"make_timezones_capi", make_timezones_capi, METH_NOARGS}, {"get_timezones_offset_zero", get_timezones_offset_zero, METH_NOARGS}, - {"get_timezone_utc_capi", get_timezone_utc_capi, METH_VARARGS}, + {"get_timezone_utc_capi", get_timezone_utc_capi, METH_VARARGS}, + {"get_date_fromtimestamp", get_date_fromtimestamp, METH_VARARGS}, + {"get_datetime_fromtimestamp", get_datetime_fromtimestamp, METH_VARARGS}, {"test_list_api", test_list_api, METH_NOARGS}, {"test_dict_iteration", test_dict_iteration, METH_NOARGS}, {"dict_getitem_knownhash", dict_getitem_knownhash, METH_VARARGS},