Reject "23:59:60.nnn" in datetime input.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 4 Jun 2020 20:42:08 +0000 (16:42 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 4 Jun 2020 20:42:23 +0000 (16:42 -0400)
It's intentional that we don't allow values greater than 24 hours,
while we do allow "24:00:00" as well as "23:59:60" as inputs.
However, the range check was miscoded in such a way that it would
accept "23:59:60.nnn" with a nonzero fraction.  For time or timetz,
the stored result would then be greater than "24:00:00" which would
fail dump/reload, not to mention possibly confusing other operations.

Fix by explicitly calculating the result and making sure it does not
exceed 24 hours.  (This calculation is redundant with what will happen
later in tm2time or tm2timetz.  Maybe someday somebody will find that
annoying enough to justify refactoring to avoid the duplication; but
that seems too invasive for a back-patched bug fix, and the cost is
probably unmeasurable anyway.)

Note that this change also rejects such input as the time portion
of a timestamp(tz) value.

Back-patch to v10.  The bug is far older, but to change this pre-v10
we'd need to ensure that the logic behaves sanely with float timestamps,
which is possibly nontrivial due to roundoff considerations.
Doesn't really seem worth troubling with.

Per report from Christoph Berg.

Discussion: https://postgr.es/m/20200520125807.GB296739@msg.df7cb.de

src/backend/utils/adt/date.c
src/backend/utils/adt/datetime.c
src/backend/utils/adt/timestamp.c
src/include/utils/date.h
src/test/regress/expected/time.out
src/test/regress/expected/timetz.out
src/test/regress/sql/time.sql
src/test/regress/sql/timetz.sql

index 0c55b68fbf63181191e7b7a93c3a5abc52d8ae17..eaaffa7137dc7919d89983d09830752934e2ece5 100644 (file)
@@ -18,6 +18,7 @@
 #include <ctype.h>
 #include <limits.h>
 #include <float.h>
+#include <math.h>
 #include <time.h>
 
 #include "access/xact.h"
@@ -1270,6 +1271,65 @@ tm2time(struct pg_tm *tm, fsec_t fsec, TimeADT *result)
    return 0;
 }
 
+/* time_overflows()
+ * Check to see if a broken-down time-of-day is out of range.
+ */
+bool
+time_overflows(int hour, int min, int sec, fsec_t fsec)
+{
+   /* Range-check the fields individually. */
+   if (hour < 0 || hour > HOURS_PER_DAY ||
+       min < 0 || min >= MINS_PER_HOUR ||
+       sec < 0 || sec > SECS_PER_MINUTE ||
+       fsec < 0 || fsec > USECS_PER_SEC)
+       return true;
+
+   /*
+    * Because we allow, eg, hour = 24 or sec = 60, we must check separately
+    * that the total time value doesn't exceed 24:00:00.
+    */
+   if ((((((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
+          + sec) * USECS_PER_SEC) + fsec) > USECS_PER_DAY)
+       return true;
+
+   return false;
+}
+
+/* float_time_overflows()
+ * Same, when we have seconds + fractional seconds as one "double" value.
+ */
+bool
+float_time_overflows(int hour, int min, double sec)
+{
+   /* Range-check the fields individually. */
+   if (hour < 0 || hour > HOURS_PER_DAY ||
+       min < 0 || min >= MINS_PER_HOUR)
+       return true;
+
+   /*
+    * "sec", being double, requires extra care.  Cope with NaN, and round off
+    * before applying the range check to avoid unexpected errors due to
+    * imprecise input.  (We assume rint() behaves sanely with infinities.)
+    */
+   if (isnan(sec))
+       return true;
+   sec = rint(sec * USECS_PER_SEC);
+   if (sec < 0 || sec > SECS_PER_MINUTE * USECS_PER_SEC)
+       return true;
+
+   /*
+    * Because we allow, eg, hour = 24 or sec = 60, we must check separately
+    * that the total time value doesn't exceed 24:00:00.  This must match the
+    * way that callers will convert the fields to a time.
+    */
+   if (((((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
+         * USECS_PER_SEC) + (int64) sec) > USECS_PER_DAY)
+       return true;
+
+   return false;
+}
+
+
 /* time2tm()
  * Convert time data type to POSIX time structure.
  *
@@ -1374,12 +1434,8 @@ make_time(PG_FUNCTION_ARGS)
    double      sec = PG_GETARG_FLOAT8(2);
    TimeADT     time;
 
-   /* This should match the checks in DecodeTimeOnly */
-   if (tm_hour < 0 || tm_min < 0 || tm_min > MINS_PER_HOUR - 1 ||
-       sec < 0 || sec > SECS_PER_MINUTE ||
-       tm_hour > HOURS_PER_DAY ||
-   /* test for > 24:00:00 */
-       (tm_hour == HOURS_PER_DAY && (tm_min > 0 || sec > 0)))
+   /* Check for time overflow */
+   if (float_time_overflows(tm_hour, tm_min, sec))
        ereport(ERROR,
                (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW),
                 errmsg("time field value out of range: %d:%02d:%02g",
@@ -1387,7 +1443,7 @@ make_time(PG_FUNCTION_ARGS)
 
    /* This should match tm2time */
    time = (((tm_hour * MINS_PER_HOUR + tm_min) * SECS_PER_MINUTE)
-           * USECS_PER_SEC) + rint(sec * USECS_PER_SEC);
+           * USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
 
    PG_RETURN_TIMEADT(time);
 }
index 1914138a24a595be282195b6e9319a2654baf518..0b6dfb248cbcb74431766a8ecd2d343f18535a39 100644 (file)
@@ -936,14 +936,9 @@ DecodeDateTime(char **field, int *ftype, int nf,
                if (dterr)
                    return dterr;
 
-               /*
-                * Check upper limit on hours; other limits checked in
-                * DecodeTime()
-                */
-               /* test for > 24:00:00 */
-               if (tm->tm_hour > HOURS_PER_DAY ||
-                   (tm->tm_hour == HOURS_PER_DAY &&
-                    (tm->tm_min > 0 || tm->tm_sec > 0 || *fsec > 0)))
+               /* check for time overflow */
+               if (time_overflows(tm->tm_hour, tm->tm_min, tm->tm_sec,
+                                  *fsec))
                    return DTERR_FIELD_OVERFLOW;
                break;
 
@@ -2218,16 +2213,8 @@ DecodeTimeOnly(char **field, int *ftype, int nf,
    else if (mer == PM && tm->tm_hour != HOURS_PER_DAY / 2)
        tm->tm_hour += HOURS_PER_DAY / 2;
 
-   /*
-    * This should match the checks in make_timestamp_internal
-    */
-   if (tm->tm_hour < 0 || tm->tm_min < 0 || tm->tm_min > MINS_PER_HOUR - 1 ||
-       tm->tm_sec < 0 || tm->tm_sec > SECS_PER_MINUTE ||
-       tm->tm_hour > HOURS_PER_DAY ||
-   /* test for > 24:00:00 */
-       (tm->tm_hour == HOURS_PER_DAY &&
-        (tm->tm_min > 0 || tm->tm_sec > 0 || *fsec > 0)) ||
-       *fsec < INT64CONST(0) || *fsec > USECS_PER_SEC)
+   /* check for time overflow */
+   if (time_overflows(tm->tm_hour, tm->tm_min, tm->tm_sec, *fsec))
        return DTERR_FIELD_OVERFLOW;
 
    if ((fmask & DTK_TIME_M) != DTK_TIME_M)
index 7ea97d0c8e5ed505482c3c7e284c7811c0e945c6..5fe304cea7530463c2aa2c3d13b483dfc00d869f 100644 (file)
@@ -32,6 +32,7 @@
 #include "parser/scansup.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
+#include "utils/date.h"
 #include "utils/datetime.h"
 #include "utils/float.h"
 
@@ -581,18 +582,8 @@ make_timestamp_internal(int year, int month, int day,
 
    date = date2j(tm.tm_year, tm.tm_mon, tm.tm_mday) - POSTGRES_EPOCH_JDATE;
 
-   /*
-    * This should match the checks in DecodeTimeOnly, except that since we're
-    * dealing with a float "sec" value, we also explicitly reject NaN.  (An
-    * infinity input should get rejected by the range comparisons, but we
-    * can't be sure how those will treat a NaN.)
-    */
-   if (hour < 0 || min < 0 || min > MINS_PER_HOUR - 1 ||
-       isnan(sec) ||
-       sec < 0 || sec > SECS_PER_MINUTE ||
-       hour > HOURS_PER_DAY ||
-   /* test for > 24:00:00 */
-       (hour == HOURS_PER_DAY && (min > 0 || sec > 0)))
+   /* Check for time overflow */
+   if (float_time_overflows(hour, min, sec))
        ereport(ERROR,
                (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW),
                 errmsg("time field value out of range: %d:%02d:%02g",
@@ -600,7 +591,7 @@ make_timestamp_internal(int year, int month, int day,
 
    /* This should match tm2time */
    time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
-           * USECS_PER_SEC) + rint(sec * USECS_PER_SEC);
+           * USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
 
    result = date * USECS_PER_DAY + time;
    /* check for major overflow */
index 91c8b36e666c03f927a3b6b8989c387daf2460df..4cdb1f97cc8c42b1c6f6340035e0bdc951dd0d66 100644 (file)
@@ -80,6 +80,8 @@ extern int    time2tm(TimeADT time, struct pg_tm *tm, fsec_t *fsec);
 extern int timetz2tm(TimeTzADT *time, struct pg_tm *tm, fsec_t *fsec, int *tzp);
 extern int tm2time(struct pg_tm *tm, fsec_t fsec, TimeADT *result);
 extern int tm2timetz(struct pg_tm *tm, fsec_t fsec, int tz, TimeTzADT *result);
+extern bool time_overflows(int hour, int min, int sec, fsec_t fsec);
+extern bool float_time_overflows(int hour, int min, double sec);
 extern void AdjustTimeForTypmod(TimeADT *time, int32 typmod);
 
 #endif                         /* DATE_H */
index 8e0afe69e016d7b42b6324faaed86b3a4965b62e..780d7f54557a1c82d10b14626f3c58c8ecab5be6 100644 (file)
@@ -73,6 +73,47 @@ SELECT f1 AS "Eight" FROM TIME_TBL WHERE f1 >= '00:00';
  15:36:39
 (10 rows)
 
+-- Check edge cases
+SELECT '23:59:59.999999'::time;
+      time       
+-----------------
+ 23:59:59.999999
+(1 row)
+
+SELECT '23:59:59.9999999'::time;  -- rounds up
+   time   
+----------
+ 24:00:00
+(1 row)
+
+SELECT '23:59:60'::time;  -- rounds up
+   time   
+----------
+ 24:00:00
+(1 row)
+
+SELECT '24:00:00'::time;  -- allowed
+   time   
+----------
+ 24:00:00
+(1 row)
+
+SELECT '24:00:00.01'::time;  -- not allowed
+ERROR:  date/time field value out of range: "24:00:00.01"
+LINE 1: SELECT '24:00:00.01'::time;
+               ^
+SELECT '23:59:60.01'::time;  -- not allowed
+ERROR:  date/time field value out of range: "23:59:60.01"
+LINE 1: SELECT '23:59:60.01'::time;
+               ^
+SELECT '24:01:00'::time;  -- not allowed
+ERROR:  date/time field value out of range: "24:01:00"
+LINE 1: SELECT '24:01:00'::time;
+               ^
+SELECT '25:00:00'::time;  -- not allowed
+ERROR:  date/time field value out of range: "25:00:00"
+LINE 1: SELECT '25:00:00'::time;
+               ^
 --
 -- TIME simple math
 --
index 482a3463b3c48d4630d9e33a2f02955a74991578..6be408f5282e591e8224168d2a991f7f57d0f9a1 100644 (file)
@@ -90,6 +90,47 @@ SELECT f1 AS "Ten" FROM TIMETZ_TBL WHERE f1 >= '00:00-07';
  15:36:39-04
 (12 rows)
 
+-- Check edge cases
+SELECT '23:59:59.999999'::timetz;
+       timetz       
+--------------------
+ 23:59:59.999999-07
+(1 row)
+
+SELECT '23:59:59.9999999'::timetz;  -- rounds up
+   timetz    
+-------------
+ 24:00:00-07
+(1 row)
+
+SELECT '23:59:60'::timetz;  -- rounds up
+   timetz    
+-------------
+ 24:00:00-07
+(1 row)
+
+SELECT '24:00:00'::timetz;  -- allowed
+   timetz    
+-------------
+ 24:00:00-07
+(1 row)
+
+SELECT '24:00:00.01'::timetz;  -- not allowed
+ERROR:  date/time field value out of range: "24:00:00.01"
+LINE 1: SELECT '24:00:00.01'::timetz;
+               ^
+SELECT '23:59:60.01'::timetz;  -- not allowed
+ERROR:  date/time field value out of range: "23:59:60.01"
+LINE 1: SELECT '23:59:60.01'::timetz;
+               ^
+SELECT '24:01:00'::timetz;  -- not allowed
+ERROR:  date/time field value out of range: "24:01:00"
+LINE 1: SELECT '24:01:00'::timetz;
+               ^
+SELECT '25:00:00'::timetz;  -- not allowed
+ERROR:  date/time field value out of range: "25:00:00"
+LINE 1: SELECT '25:00:00'::timetz;
+               ^
 --
 -- TIME simple math
 --
index 99a1562ed23d512a5cf1a855ad6db0328b1fd35e..ea5f8b639f0d67340c27b29bfe18a73f3e200d1b 100644 (file)
@@ -30,6 +30,16 @@ SELECT f1 AS "None" FROM TIME_TBL WHERE f1 < '00:00';
 
 SELECT f1 AS "Eight" FROM TIME_TBL WHERE f1 >= '00:00';
 
+-- Check edge cases
+SELECT '23:59:59.999999'::time;
+SELECT '23:59:59.9999999'::time;  -- rounds up
+SELECT '23:59:60'::time;  -- rounds up
+SELECT '24:00:00'::time;  -- allowed
+SELECT '24:00:00.01'::time;  -- not allowed
+SELECT '23:59:60.01'::time;  -- not allowed
+SELECT '24:01:00'::time;  -- not allowed
+SELECT '25:00:00'::time;  -- not allowed
+
 --
 -- TIME simple math
 --
index 2ad4948e85008e0a4c78a08ace8c6432ca6c76b3..a1fa4ef3b7fbaa28f743f2b54e730616de4febf5 100644 (file)
@@ -35,6 +35,16 @@ SELECT f1 AS "None" FROM TIMETZ_TBL WHERE f1 < '00:00-07';
 
 SELECT f1 AS "Ten" FROM TIMETZ_TBL WHERE f1 >= '00:00-07';
 
+-- Check edge cases
+SELECT '23:59:59.999999'::timetz;
+SELECT '23:59:59.9999999'::timetz;  -- rounds up
+SELECT '23:59:60'::timetz;  -- rounds up
+SELECT '24:00:00'::timetz;  -- allowed
+SELECT '24:00:00.01'::timetz;  -- not allowed
+SELECT '23:59:60.01'::timetz;  -- not allowed
+SELECT '24:01:00'::timetz;  -- not allowed
+SELECT '25:00:00'::timetz;  -- not allowed
+
 --
 -- TIME simple math
 --