[Bug libc/16141] New: strptime %z offset restriction

[Bug libc/16141] New: strptime %z offset restriction

[f22raptorf22 at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16141

            Bug ID: 16141
           Summary: strptime %z offset restriction
           Product: glibc
           Version: 2.18
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
          Assignee: unassigned at sourceware dot org
          Reporter: f22raptorf22 at gmail dot com
                CC: drepper.fsp at gmail dot com

In time/strptime_l.c:

if (val > 1200)
    return NULL;


Per these resources:
http://www.timeanddate.com/time/map/
http://en.wikipedia.org/wiki/Line_Islands

Kiribati is among the select few that are part of the world's farthest forward
time zone, UTC+14:00.

strptime currently restricts the numeric part of the offset to 1200.


I propose this restriction be raised to a minimum of 1400, or even allow up to
an offset of a full day (2400) for future compatibility.

Thanks.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16141] strptime %z offset restriction

[f22raptorf22 at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16141

--- Comment #1 from f22raptorf22 at gmail dot com ---
Created attachment 7271
  --> https://sourceware.org/bugzilla/attachment.cgi?id=7271&action=edit
tst-strptime2 -- modified

Not that there are to-the-minute timezones, but it can also be observed that
the conversion to decimal does not properly parse the minutes.  This test
program also illustrates the 1200 cap for the offset.


This:    { "1113472456  +1159", 43140 }
Yields: round 3: tm_gmtoff is 43128

This:    { "1113472456  +1201", 43260 }
Yields: round 4: strptime unexpectedly failed




Making a change to reflect something similar to the below, should display
proper behavior (and most likely less computational work):

            while (n < 4 && *rp >= '0' && *rp <= '9')
           {
          if(n < 2)
                   hrs = hrs * 10 + *rp++ - '0';
                else
                   min = min * 10 + *rp++ - '0';
         ++n;
          }

        if (n != 2 && n != 4)
          /* Only two or four digits recognized.  */
          return NULL;

            else if (hrs*100 + min > 2400)
           return NULL;

        tm->tm_gmtoff = hrs*3600 + min*60;
        if (neg)
          tm->tm_gmtoff = -tm->tm_gmtoff;
      }

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16141] strptime %z offset restriction

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16141

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16141] strptime %z offset restriction

[james at loowit dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=16141

James Perkins <james at loowit dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |james at loowit dot net

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16141] strptime %z offset restriction

[james at loowit dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=16141

--- Comment #2 from James Perkins <james at loowit dot net> ---
(In reply to f22raptorf22 from comment #0)
> I propose this restriction be raised to a minimum of 1400, or even allow up
> to an offset of a full day (2400) for future compatibility.

A fix for this is somewhat important, as New Zealand's summer time
(Pacific/Auckland summer time) is +1300, and Samoa's summer time (Pacific/Apia)
is +1400.

I will attach a patch that extends strptime %z to correctly parse values up to
+14:00.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16141] strptime %z offset restriction

[james at loowit dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=16141

--- Comment #3 from James Perkins <james at loowit dot net> ---
Created attachment 7983
  --> https://sourceware.org/bugzilla/attachment.cgi?id=7983&action=edit
Patch to extend valid timezone offset to UTC+14:00

This patch modifies %z to parse timezone offsets from UTC from -1200 to +1400.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16141] strptime %z offset restriction

[james at loowit dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=16141

--- Comment #4 from James Perkins <james at loowit dot net> ---
Created attachment 7985
  --> https://sourceware.org/bugzilla/attachment.cgi?id=7985&action=edit
Patch to fix minutes calculation

The second patch addresses the problem with the portion of the timezone offset
from the minutes being miscalculated as the offset; instead of the decimal time
conversion, just use the minutes portion and multiply by 60.

With these two patches applied, the tst-strptime2 test passes 'all OK'

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16141] strptime %z offset restriction

[james at loowit dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=16141

--- Comment #5 from James Perkins <james at loowit dot net> ---
I reworked the patches slightly and posted them to the libc-alpha list:

https://sourceware.org/ml/libc-alpha/2014-12/msg00066.html
https://sourceware.org/ml/libc-alpha/2014-12/msg00067.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/16141] strptime %z offset restriction

[james at loowit dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=16141

James Perkins <james at loowit dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at sourceware dot org   |james at loowit dot net

--- Comment #6 from James Perkins <james at loowit dot net> ---
Taking this bug. Proposed patchset on glibc-alpha now in review, at revision
V5.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug time/16141] strptime %z offset restriction

[jsm28 at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=16141

Joseph Myers <jsm28 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|libc                        |time

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug time/16141] strptime %z offset restriction

[james at loowit dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=16141

James Perkins <james at loowit dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |2.23

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug time/16141] strptime %z offset restriction

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=16141

--- Comment #7 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  be13f5a4baa40af3611208edc77fd588ff2c0fab (commit)
       via  cccc95f28ace0be4fe97537dbd867c08fbf71a8e (commit)
      from  315267abcd2eeb467820a3fada43874e35bbcd3d (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=be13f5a4baa40af3611208edc77fd588ff2c0fab

commit be13f5a4baa40af3611208edc77fd588ff2c0fab
Author: James Perkins <james@loowit.net>
Date:   Mon Aug 17 13:48:39 2015 -0700

    time/tst-strptime2.c: test full input range +/- 0-9999

    strptime's %z specifier parses a string consisting of a sign ('+'
    or '-'), two hours digits, and optionally two minutes digits, into a
    tm.tm_gmtoff field containing the signed number of seconds the time
    zone is offset from UTC time.

    The time/tst-strptime2.c program passes a short list of strings through
    strptime, validating that either the gmtoff value returned matches an
    expected value, or that strptime returns an expected NULL for invalid
    strings (for example, when the minutes portion of the string is outside
    of the range 00 to 59, or the sign is missing before the hours digits).

    In review of strptime fixes, Carlos O'Donell expressed a wish that
    the test function iterate through the entire range of all possible
    numeric strings (-9999 to +9999) which could be passed to strptime %z,
    and validate the correct response.

    Specifically, the test will look for a NULL response from strptime
    when:

      * sign ('+' or '-') is not present before the first digit (invalid
        format).
      * A sign and no digits are found (invalid format).
      * A sign and one digit are found (invalid format).
      * A sign and three digits are found (invalid format).
      * A sign and four digits (-9999 to +9999) are found but the last
        two digits (minutes) are in the range 60 to 99.

    The test will look for a success response from strptime with
    tm.tm_gmtoff matching the calculated tm_gmtoff value when:

      * A sign and four digits are found (-9999 to +9999), and the last
        two digits (minutes) are in the range 00 to 59.
      * A sign and two digit strings are found (-99 to +99).

    The test's iteration over the possible digit values results in 22223
    test strings prepared, tested, and passed by strptime.

    The test supports a --verbose command line option which will show
    the test results of every test input, and a final summary of all
    tests. Here is some sample output:

      PASS: input "1113472456  1030", expected: invalid, return value NULL
      PASS: input "1113472456 +", expected: invalid, return value NULL
      PASS: input "1113472456 -", expected: invalid, return value NULL
      PASS: input "1113472456 +0", expected: invalid, return value NULL
      PASS: input "1113472456 -0", expected: invalid, return value NULL
      PASS: input "1113472456 +1", expected: invalid, return value NULL
      ...
      PASS: input "1113472456 +9", expected: invalid, return value NULL
      PASS: input "1113472456 -9", expected: invalid, return value NULL
      PASS: input "1113472456 +00", expected: valid, tm.tm_gmtoff 0
      PASS: input "1113472456 -00", expected: valid, tm.tm_gmtoff 0
      PASS: input "1113472456 +01", expected: valid, tm.tm_gmtoff 3600
      PASS: input "1113472456 -01", expected: valid, tm.tm_gmtoff -3600
      PASS: input "1113472456 +02", expected: valid, tm.tm_gmtoff 7200
      ...
      PASS: input "1113472456 +99", expected: valid, tm.tm_gmtoff 356400
      PASS: input "1113472456 -99", expected: valid, tm.tm_gmtoff -356400
      PASS: input "1113472456 +000", expected: invalid, return value NULL
      PASS: input "1113472456 -000", expected: invalid, return value NULL
      PASS: input "1113472456 +001", expected: invalid, return value NULL
      ...
      PASS: input "1113472456 +999", expected: invalid, return value NULL
      PASS: input "1113472456 -999", expected: invalid, return value NULL
      PASS: input "1113472456 +0000", expected: valid, tm.tm_gmtoff 0
      PASS: input "1113472456 -0000", expected: valid, tm.tm_gmtoff 0
      PASS: input "1113472456 +0001", expected: valid, tm.tm_gmtoff 60
      PASS: input "1113472456 -0001", expected: valid, tm.tm_gmtoff -60
      ...
      PASS: input "1113472456 +0059", expected: valid, tm.tm_gmtoff 3540
      PASS: input "1113472456 -0059", expected: valid, tm.tm_gmtoff -3540
      PASS: input "1113472456 +0060", expected: invalid, return value NULL
      PASS: input "1113472456 -0060", expected: invalid, return value NULL
      ...
      PASS: input "1113472456 +0099", expected: invalid, return value NULL
      PASS: input "1113472456 -0099", expected: invalid, return value NULL
      PASS: input "1113472456 +0100", expected: valid, tm.tm_gmtoff 3600
      PASS: input "1113472456 -0100", expected: valid, tm.tm_gmtoff -3600
      PASS: input "1113472456 +0101", expected: valid, tm.tm_gmtoff 3660
      ...
      PASS: input "1113472456 +9999", expected: invalid, return value NULL
      PASS: input "1113472456 -9999", expected: invalid, return value NULL
      PASS: 22223 input strings: 0 fail, 22223 pass

    Any failing test will result in printing the failed line to stdout, and
    will trigger the printing of the summary line at the of all tests. For
    example:

      FAIL: input "1113472456  1030", expected: invalid, return value NULL,
        got: valid, tm.tm_gmtoff 37800
      FAIL: 22223 input strings: 1 fail, 22222 pass

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=cccc95f28ace0be4fe97537dbd867c08fbf71a8e

commit cccc95f28ace0be4fe97537dbd867c08fbf71a8e
Author: James Perkins <james@loowit.net>
Date:   Mon Aug 17 13:48:38 2015 -0700

    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.

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog            |   21 +++++
 NEWS                 |    8 +-
 time/strptime_l.c    |   12 +--
 time/tst-strptime2.c |  214 +++++++++++++++++++++++++++++++++++++++++---------
 4 files changed, 206 insertions(+), 49 deletions(-)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug time/16141] strptime %z offset restriction

[joseph at codesourcery dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16141

--- Comment #8 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
If the checked-in patch fixes this bug (as implied by the addition to 
NEWS), please close it as fixed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug time/16141] strptime %z offset restriction

[james at loowit dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=16141

James Perkins <james at loowit dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from James Perkins <james at loowit dot net> ---
Fixed by commit to master 2015-08-29:

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

Tests improved by commit to master 2015-08-29:

be13f5a4baa40af3611208edc77fd588ff2c0fab time/tst-strptime2.c: test full input
range +/- 0-9999

-- 
You are receiving this mail because:
You are on the CC list for the bug.