Skip to content

Commit

Permalink
bpo-36025: Fix PyDate_FromTimestamp API (pythonGH-11922)
Browse files Browse the repository at this point in the history
In the process of converting the date.fromtimestamp function to use
argument clinic in pythonGH-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.
  • Loading branch information
pganssle authored and berkerpeksag committed Apr 27, 2019
1 parent 5c403b2 commit 4d8c8c0
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 2 deletions.
35 changes: 35 additions & 0 deletions Lib/test/datetimetester.py
Original file line number Diff line number Diff line change
Expand Up @@ -5942,6 +5942,41 @@ class TZInfoSubclass(tzinfo):
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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
19 changes: 18 additions & 1 deletion Modules/_datetimemodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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, &timestamp)) {
result = date_fromtimestamp(cls, timestamp);
}

return result;
}

/* Return new date from proleptic Gregorian ordinal. Raises ValueError if
* the ordinal is out of range.
*/
Expand Down Expand Up @@ -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
};
Expand Down
69 changes: 68 additions & 1 deletion Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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, &macro)) {
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, &macro)) {
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
Expand Down Expand Up @@ -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},
Expand Down

0 comments on commit 4d8c8c0

Please sign in to comment.