From e470dca95f7ec6515807c0b4a54ab35d1c1deb8f Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Dec 2020 15:36:56 +0000 Subject: [PATCH 1/3] gdatetime: Remove floating point from seconds parsing code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rather than parsing the seconds in an ISO 8601 date/time using a pair of floating point numbers (numerator and denominator), use two integers instead. This avoids issues around floating point precision, and also makes it easier to check for potential overflow from overlong inputs. This last point means that the `isfinite()` check can be removed, as it was covering the case where a NAN was generated, which isn’t now possible using integer arithmetic. Signed-off-by: Philip Withnall --- glib/gdatetime.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/glib/gdatetime.c b/glib/gdatetime.c index 6f71e99cf..db54aed50 100644 --- a/glib/gdatetime.c +++ b/glib/gdatetime.c @@ -1181,7 +1181,7 @@ static gboolean get_iso8601_seconds (const gchar *text, gsize length, gdouble *value) { gsize i; - gdouble divisor = 1, v = 0; + guint64 divisor = 1, v = 0; if (length < 2) return FALSE; @@ -1208,17 +1208,15 @@ get_iso8601_seconds (const gchar *text, gsize length, gdouble *value) for (; i < length; i++) { const gchar c = text[i]; - if (c < '0' || c > '9') + if (c < '0' || c > '9' || + v > (G_MAXUINT64 - (c - '0')) / 10 || + divisor > G_MAXUINT64 / 10) return FALSE; v = v * 10 + (c - '0'); divisor *= 10; } - v = v / divisor; - if (!isfinite (v)) - return FALSE; - - *value = v; + *value = (gdouble) v / divisor; return TRUE; } From 72246a564d11f211928c590a3411dc372886eb6e Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Dec 2020 15:39:47 +0000 Subject: [PATCH 2/3] gdatetime: Use isnan() instead of !isfinite() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both are provided by libm, but `isnan()` is provided as a macro, whereas `isfinite()` is an actual function, and hence libm has to be available at runtime. That didn’t trivially work on FreeBSD, resulting in this refactor. `isfinite(x)` is equivalent to `!isnan(x) && !isinfinite(x)`. The case of `x` being (negative or positive) infinity is already handled by the range checks on the next line, so it’s safe to switch to `isnan()` here. Signed-off-by: Philip Withnall --- glib/gdatetime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/glib/gdatetime.c b/glib/gdatetime.c index db54aed50..02cc3bd01 100644 --- a/glib/gdatetime.c +++ b/glib/gdatetime.c @@ -1588,7 +1588,7 @@ g_date_time_new (GTimeZone *tz, day < 1 || day > days_in_months[GREGORIAN_LEAP (year)][month] || hour < 0 || hour > 23 || minute < 0 || minute > 59 || - !isfinite (seconds) || + isnan (seconds) || seconds < 0.0 || seconds >= 60.0) return NULL; From d8794aaf703ac21fc58fcfdbafbb44eadb2fbcec Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Dec 2020 15:41:42 +0000 Subject: [PATCH 3/3] tests: Add more tests for GDateTime ISO 8601 seconds parsing This should add a few more lines/branches to the test coverage. Signed-off-by: Philip Withnall --- glib/tests/gdatetime.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/glib/tests/gdatetime.c b/glib/tests/gdatetime.c index 56491a060..0203dd0c2 100644 --- a/glib/tests/gdatetime.c +++ b/glib/tests/gdatetime.c @@ -741,6 +741,14 @@ test_GDateTime_new_from_iso8601 (void) dt = g_date_time_new_from_iso8601 ("--0824T22:10:42Z", NULL); g_assert_null (dt); + /* Seconds must be two digits. */ + dt = g_date_time_new_from_iso8601 ("2016-08-10T22:10:4Z", NULL); + g_assert_null (dt); + + /* Seconds must all be digits. */ + dt = g_date_time_new_from_iso8601 ("2016-08-10T22:10:4aZ", NULL); + g_assert_null (dt); + /* Check subseconds work */ dt = g_date_time_new_from_iso8601 ("2016-08-24T22:10:42.123456Z", NULL); ASSERT_DATE (dt, 2016, 8, 24); @@ -757,6 +765,28 @@ test_GDateTime_new_from_iso8601 (void) ASSERT_TIME (dt, 22, 10, 42, 123456); g_date_time_unref (dt); + /* Subseconds must all be digits. */ + dt = g_date_time_new_from_iso8601 ("2016-08-10T22:10:42.5aZ", NULL); + g_assert_null (dt); + + /* Subseconds can be an arbitrary length, but must not overflow. + * The ASSERT_TIME() comparisons are constrained by only comparing up to + * microsecond granularity. */ + dt = g_date_time_new_from_iso8601 ("2016-08-10T22:10:09.222222222222222222Z", NULL); + ASSERT_DATE (dt, 2016, 8, 10); + ASSERT_TIME (dt, 22, 10, 9, 222222); + g_date_time_unref (dt); + dt = g_date_time_new_from_iso8601 ("2016-08-10T22:10:09.2222222222222222222Z", NULL); + g_assert_null (dt); + + /* Small numerator, large divisor when parsing the subseconds. */ + dt = g_date_time_new_from_iso8601 ("2016-08-10T22:10:00.0000000000000000001Z", NULL); + ASSERT_DATE (dt, 2016, 8, 10); + ASSERT_TIME (dt, 22, 10, 0, 0); + g_date_time_unref (dt); + dt = g_date_time_new_from_iso8601 ("2016-08-10T22:10:00.00000000000000000001Z", NULL); + g_assert_null (dt); + /* We don't support times without minutes / seconds (valid ISO 8601) */ dt = g_date_time_new_from_iso8601 ("2016-08-24T22Z", NULL); g_assert_null (dt); @@ -1277,8 +1307,18 @@ test_GDateTime_new_full (void) g_date_time_unref (dt); dt = g_date_time_new_utc (2016, 12, 32, 22, 10, 42); g_assert_null (dt); + + /* Seconds limits. */ dt = g_date_time_new_utc (2020, 12, 9, 14, 49, NAN); g_assert_null (dt); + dt = g_date_time_new_utc (2020, 12, 9, 14, 49, -0.1); + g_assert_null (dt); + dt = g_date_time_new_utc (2020, 12, 9, 14, 49, 60.0); + g_assert_null (dt); + + /* Year limits */ + dt = g_date_time_new_utc (10000, 1, 1, 0, 0, 0); + dt = g_date_time_new_utc (0, 1, 1, 0, 0, 0); } static void