strptime %z: fix rounding, extend range to +/-9959 [BZ #16141]

Topic: strptime supports a %z input field descriptor, which parses a
time zone offset from UTC time into the broken-out time field tm_gmtoff.

Problems:

1) In the current implementation, the minutes portion calculation is
correct only for minutes evenly divisible by 3. This is because the
minutes value is converted to decimal time, but inadequate precision
leads to rounding which calculates results that are too low for
some values.

For example, due to rounding, a +1159 offset string results in an
incorrect tm_gmtoff of 43128 (== 11 * 3600 + 58.8 * 60) seconds,
instead of 43140 (== 11 * 3600 + 59 * 60) seconds. In contrast,
a +1157 offset (minutes divisible by 3) does not cause the bug,
and results in a correct tm_gmtoff of 43020.

2) strptime's %z specifier will not parse time offsets less than
-1200 or greater than +1200, or if only hour digits are present, less
than -12 or greater than +12. It will return NULL for offsets outside
that range. These limits do not meet historical and modern use cases:

  * Present day exceeds the +1200 limit:
    - Pacific/Auckland (New Zealand) summer time is +1300.
    - Pacific/Kiritimati (Christmas Island) is +1400.
    - Pacific/Apia (Samoa) summer time is +1400.
  * Historical offsets exceeded +1500/-1500.
  * POSIX supports -2459 to +2559.
  * Offsets up to +/-9959 may occasionally be useful.
  * Paul Eggert's notes provide additional detail:
    - https://sourceware.org/ml/libc-alpha/2014-12/msg00068.html
    - https://sourceware.org/ml/libc-alpha/2014-12/msg00072.html

3) tst-strptime2, part of the 'make check' test suite, does not test
for the above problems.

Corrective actions:

1) In time/strptime_l.c, calculate the offset from the hour and
minute portions directly, without the rounding errors introduced by
decimal time.

2) Remove the +/-1200 range limit, permitting strptime to parse offsets
from -9959 through +9959.

3) Add zone offset values to time/tst-strptime2.c.

  * Test minutes evenly divisible by three (+1157) and not evenly
    divisible by three (+1158 and +1159).
  * Test offsets near the old and new range limits (-1201, -1330, -2459,
    -2500, -99, -9959, +1201, +1330, +1400, +1401, +2559, +2600, +99,
    and +9959)

The revised strptime passes all old and new tst-strptime2 tests.

diff --git a/ChangeLog b/ChangeLog
index 287ab2db373da97d1d42cac446528a17460876e0..6c1f804fd24595c0d18c55346df738d5ddcf2681 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2015-08-28  James Perkins  <james@loowit.net>
+
+       [BZ #16141]
+       * time/strptime_l.c (__strptime_internal): Fix %z minutes
+       calculation, removing incorrect decimal time rounding, so that
+       all minute values result in a valid seconds value.
+       * time/strptime_l.c (__strptime_internal): Extend %z time zone
+       offset range limits to UTC-99:59 through UTC+99:59 to parse
+       current and historical use cases.
+       * time/tst-strptime2.c (tests): Modify and add tests for the
+       strptime %z input field descriptor, specifically conversion of
+       minutes to seconds and validating an offset range of -9959 to
+       +9959.
+
 2015-08-27  Adhemerval Zanella  <adhemerval.zanella@linaro.org>
            Dmitry V. Levin  <ldv@altlinux.org>
 

diff --git a/NEWS b/NEWS
index af9fd8de2b8796cd878b5484647c3c95b701e096..f70eb529729bb79583d8dff5a0abee6cc0007849 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -9,10 +9,10 @@ Version 2.23
 
 * The following bugs are resolved with this release:
 
-  2898, 14341, 15786, 16517, 16519, 16520, 16734, 16973, 17787, 17905,
-  18084, 18086, 18240, 18265, 18370, 18421, 18480, 18525, 18610, 18618,
-  18647, 18661, 18674, 18681, 18778, 18781, 18787, 18789, 18790, 18795,
-  18796, 18820, 18823, 18824, 18863.
+  2898, 14341, 15786, 16141, 16517, 16519, 16520, 16734, 16973, 17787,
+  17905, 18084, 18086, 18240, 18265, 18370, 18421, 18480, 18525, 18610,
+  18618, 18647, 18661, 18674, 18681, 18778, 18781, 18787, 18789, 18790,
+  18795, 18796, 18820, 18823, 18824, 18863.
 
 * The obsolete header <regexp.h> has been removed.  Programs that require
   this header must be updated to use <regex.h> instead.

diff --git a/time/strptime_l.c b/time/strptime_l.c
index 5640ccecced0ee7fbd1d6653fe7b421f5f2bda84..4203ad81a0f3b36c6d5feafac9eb42de49b472ee 100644 (file)
--- a/time/strptime_l.c
+++ b/time/strptime_l.c
@@ -770,16 +770,10 @@ __strptime_internal (rp, fmt, tmp, statep LOCALE_PARAM)
            else if (n != 4)
              /* Only two or four digits recognized.  */
              return NULL;
-           else
-             {
-               /* We have to convert the minutes into decimal.  */
-               if (val % 100 >= 60)
-                 return NULL;
-               val = (val / 100) * 100 + ((val % 100) * 50) / 30;
-             }
-           if (val > 1200)
+           else if (val % 100 >= 60)
+             /* Minutes valid range is 0 through 59.  */
              return NULL;
-           tm->tm_gmtoff = (val * 3600) / 100;
+           tm->tm_gmtoff = (val / 100) * 3600 + (val % 100) * 60;
            if (neg)
              tm->tm_gmtoff = -tm->tm_gmtoff;
          }

diff --git a/time/tst-strptime2.c b/time/tst-strptime2.c
index a08e6d7cb7dee6b9a33cbc9a0b27933216d3b28b..4db8321291ec37a1f84c8e6992ae291a3b2888d1 100644 (file)
--- a/time/tst-strptime2.c
+++ b/time/tst-strptime2.c
@@ -17,8 +17,25 @@ static const struct
     { "1113472456 -1030", -37800 },
     { "1113472456 +0030", 1800 },
     { "1113472456 -0030", -1800 },
-    { "1113472456 -1330", LONG_MAX },
-    { "1113472456 +1330", LONG_MAX },
+    { "1113472456 +1157", 43020 },
+    { "1113472456 +1158", 43080 },
+    { "1113472456 +1159", 43140 },
+    { "1113472456 +1200", 43200 },
+    { "1113472456 -1200", -43200 },
+    { "1113472456 +1201", 43260 },
+    { "1113472456 -1201", -43260 },
+    { "1113472456 +1330", 48600 },
+    { "1113472456 -1330", -48600 },
+    { "1113472456 +1400", 50400 },
+    { "1113472456 +1401", 50460 },
+    { "1113472456 -2459", -89940 },
+    { "1113472456 -2500", -90000 },
+    { "1113472456 +2559", 93540 },
+    { "1113472456 +2600", 93600 },
+    { "1113472456 -99", -356400 },
+    { "1113472456 +99", 356400 },
+    { "1113472456 -9959", -359940 },
+    { "1113472456 +9959", 359940 },
     { "1113472456 -1060", LONG_MAX },
     { "1113472456 +1060", LONG_MAX },
     { "1113472456  1030", LONG_MAX },