Extend PR_ParseTimeString() to accept some ISO 8601 formats to fix timezone parsing in SyslogParser.

base/third_party/nspr/prtime.cc

 /* Portions are Copyright (C) 2011 Google Inc */
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
  * The contents of this file are subject to the Mozilla Public License Version
  * 1.1 (the "License"); you may not use this file except in compliance with
  * the License. You may obtain a copy of the License at
  * http://www.mozilla.org/MPL/
  *
  * Software distributed under the License is distributed on an "AS IS" basis,
@@ skipping 43 matching lines @@
  * PR_GMTParameters
  * PR_ImplodeTime
  *   This was modified to use the Win32 SYSTEMTIME/FILETIME structures
  *   and the timezone offsets are applied to the FILETIME structure.
  * All types and macros are defined in the base/third_party/prtime.h file.
  * These have been copied from the following nspr files. We have only copied
  * over the types we need.
  * 1. prtime.h
  * 2. prtypes.h
  * 3. prlong.h
+ *
+ * Unit tests are in base/time/pr_time_unittest.cc.
  */
 
 #include "base/logging.h"
 #include "base/third_party/nspr/prtime.h"
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
 #elif defined(OS_MACOSX)
 #include <CoreFoundation/CoreFoundation.h>
@@ skipping 424 matching lines @@
  *   Mon Jan 16 16:12 +0130 1989
  *   6 May 1992 16:41-JST (Wednesday)
  *   22-AUG-1993 10:59:12.82
  *   22-AUG-1993 10:59pm
  *   22-AUG-1993 12:59am
  *   22-AUG-1993 12:59 PM
  *   Friday, August 04, 1995 3:54 PM
  *   06/21/95 04:24:34 PM
  *   20/06/95 21:07
  *   95-06-08 19:32:48 EDT
+ *   1995-06-17T23:11:25.342156Z
  *
  * If the input string doesn't contain a description of the timezone,
  * we consult the `default_to_gmt' to decide whether the string should
  * be interpreted relative to the local time zone (PR_FALSE) or GMT (PR_TRUE).
  * The correct value for this argument depends on what standard specified
  * the time string which you are parsing.
  */
 
 PRStatus
 PR_ParseTimeString(
         const char *string,
         PRBool default_to_gmt,
         PRTime *result_imploded)
 {
   PRExplodedTime tm;
   PRExplodedTime *result = &tm;
   TIME_TOKEN dotw = TT_UNKNOWN;
   TIME_TOKEN month = TT_UNKNOWN;
   TIME_TOKEN zone = TT_UNKNOWN;
   int zone_offset = -1;
   int dst_offset = 0;
   int date = -1;
   PRInt32 year = -1;
   int hour = -1;
   int min = -1;
   int sec = -1;
+  int usec = -1;
 
   const char *rest = string;
 
   int iterations = 0;
 
   PR_ASSERT(string && result);
   if (!string || !result) return PR_FAILURE;
 
   while (*rest)
         {
@@ skipping 223 matching lines @@
                         zone = TT_GMT;
                         break;
                   }
 
                 case '0': case '1': case '2': case '3': case '4':
                 case '5': case '6': case '7': case '8': case '9':
                   {
                         int tmp_hour = -1;
                         int tmp_min = -1;
                         int tmp_sec = -1;
+                        int tmp_usec = -1;
                         const char *end = rest + 1;
                         while (*end >= '0' && *end <= '9')
                           end++;
 
                         /* end is now the first character after a range of digits. */
 
                         if (*end == ':')
                           {
                                 if (hour >= 0 && min >= 0) /* already got it */
                                   break;
@@ skipping 39 matching lines @@
                                   ;
                                 else if ((end - rest) > 2)
                                   /* it is [0-9][0-9][0-9]+: */
                                   break;
                                 else if ((end - rest) == 2)
                                   tmp_sec = ((rest[0]-'0')*10 +
                                                          (rest[1]-'0'));
                                 else
                                   tmp_sec = (rest[0]-'0');
 
-                                /* If we made it here, we've parsed hour and min,
-                                   and possibly sec, so it worked as a unit. */
+                                /* fractional second */
+                                rest = end;
+                                if (*rest == '.') {
+                                  rest++;
+                                  end++;
+                                  tmp_usec = 0;
+                                  /* use up to 6 digits, skip over the rest */
+                                  while (*end >= '0' && *end <= '9') {
+                                    if (end - rest < 6) {
+                                      tmp_usec = tmp_usec * 10 + *end - '0';
+                                    }
+                                    end++;
+                                  }
+                                  int ndigits = end - rest;
+                                  while (ndigits++ < 6)
+                                    tmp_usec *= 10;
+                                }
 
-                                /* skip over whitespace and see if there's an AM or PM
-                                   directly following the time.
-                                 */
-                                if (tmp_hour <= 12)
+                                if (*end == 'Z') {
+                                  zone = TT_GMT;

[wtc, 2014/05/12 19:47:15]

Should we do

    end++;

here? Otherwise 'Z' may be inspected again.

[Thiemo Nagel, 2014/05/13 14:08:19]

It's not strictly necessary because the break will skip through to the while
loop in line 1062 which skips the rest of the token.  Nevertheless I've added it
(and replaced end by rest) in the hope of making the code more readable.

+                                }

[wtc, 2014/05/12 19:47:15]

Nit: Lines 843-861 probably should follow the braces style of this file.
    if (*rest == '.')
      {
        rest++;
        end++;
        ...
      }
    if (*end == 'Z')
      {
        zone = TT_GMT;
      }
    else if (tmp_hour <= 12)

[Thiemo Nagel, 2014/05/13 14:08:19]

Thanks, I fully agree.  Done.

+                                else if (tmp_hour <= 12)
                                   {
+                                    /* If we made it here, we've parsed hour and min,
+                                       and possibly sec, so it worked as a unit. */

[wtc, 2014/05/12 19:47:15]

By "it worked as a unit", did you mean we have fully parsed the time and are
ready for the next token?

[Thiemo Nagel, 2014/05/13 14:08:19]

That's how I understand the original author.  (I've only moved the pre-existing
comment a bit.)  I'm updating the comment now to make it clearer.

+
+                                    /* skip over whitespace and see if there's an AM or PM
+                                       directly following the time.
+                                    */
                                         const char *s = end;
                                         while (*s && (*s == ' ' || *s == '\t'))
                                           s++;
                                         if ((s[0] == 'p' || s[0] == 'P') &&
                                                 (s[1] == 'm' || s[1] == 'M'))
                                           /* 10:05pm == 22:05, and 12:05pm == 12:05 */
                                           tmp_hour = (tmp_hour == 12 ? 12 : tmp_hour + 12);
                                         else if (tmp_hour == 12 &&
                                                          (s[0] == 'a' || s[0] == 'A') &&
                                                          (s[1] == 'm' || s[1] == 'M'))
                                           /* 12:05am == 00:05 */
                                           tmp_hour = 0;
                                   }
 
                                 hour = tmp_hour;
                                 min = tmp_min;
                                 sec = tmp_sec;
+                                usec = tmp_usec;
                                 rest = end;
                                 break;
                           }
                         else if ((*end == '/' || *end == '-') &&
                                          end[1] >= '0' && end[1] <= '9')
                           {
                                 /* Perhaps this is 6/16/95, 16/6/95, 6-16-95, or 16-6-95
-                                   or even 95-06-05...
-                                   #### But it doesn't handle 1995-06-22.
+                                   or even 95-06-05 or 1995-06-22.
                                  */
                                 int n1, n2, n3;
                                 const char *s;
 
                                 if (month != TT_UNKNOWN)
                                   /* if we saw a month name, this can't be. */
                                   break;
 
                                 s = rest;
 
-                                n1 = (*s++ - '0');                                /* first 1 or 2 digits */
+                                n1 = (*s++ - '0');                                /* first 1, 2 or 4 digits */
                                 if (*s >= '0' && *s <= '9')
                                   n1 = n1*10 + (*s++ - '0');
 
+                                if (*s >= '0' && *s <= '9')            /* optional digits 3 and 4 */
+                                  {
+                                        n1 = n1*10 + (*s++ - '0');
+                                        if (*s < '0' || *s > '9')
+                                          break;

[wtc, 2014/05/12 19:47:15]

This break statement means you want to disallow a three-digit year. I am fine
with that (line 942 also has a break statement). Just wanted to confirm this.

[Thiemo Nagel, 2014/05/13 14:08:19]

Yes, that's the intent.

+                                        n1 = n1*10 + (*s++ - '0');
+                                  }

[wtc, 2014/05/12 19:47:15]

Nit: it seems that this new if block (lines 910-916) should be inside the
previous if statement, i.e., we only allow digits 3 and 4 if there is digit 2:

  n1 = (*s++ - '0'); /* first 1, 2 or 4 digits */
  if (*s >= '0' && *s <= '9')
    {
      n1 = n1*10 + (*s++ - '0');
      if (*s >= '0' && *s <= '9') /* option al digits 3 and 4 */
        {
          n1 = n1*10 + (*s++ - '0');
          if (*s < '0' || *s > '9')
            break;
          n1 = n1*10 + (*s++ - '0');
        }
    }

Your current code is fine because it matches the code on lines 934-946. Note
that that code seems to allow a five-digit year. I forgot why.

[Thiemo Nagel, 2014/05/13 14:08:19]

Yes, good point, that's more readable.
I certainly would be interested to know.  Anyways, ISO 8601 does specify only
four digits, that's why I'm not supporting a fifth one.

+
                                 if (*s != '/' && *s != '-')                /* slash */
                                   break;
                                 s++;
 
                                 if (*s < '0' || *s > '9')                /* second 1 or 2 digits */
                                   break;
                                 n2 = (*s++ - '0');
                                 if (*s >= '0' && *s <= '9')
                                   n2 = n2*10 + (*s++ - '0');
 
@@ skipping 10 matching lines @@
                                 if (*s >= '0' && *s <= '9')            /* optional digits 3, 4, and 5 */
                                   {
                                         n3 = n3*10 + (*s++ - '0');
                                         if (*s < '0' || *s > '9')
                                           break;
                                         n3 = n3*10 + (*s++ - '0');
                                         if (*s >= '0' && *s <= '9')
                                           n3 = n3*10 + (*s++ - '0');
                                   }
 
-                                if ((*s >= '0' && *s <= '9') ||        /* followed by non-alphanum */
-                                        (*s >= 'A' && *s <= 'Z') ||
-                                        (*s >= 'a' && *s <= 'z'))
+                                if (*s == 'T' && s[1] >= '0' && s[1] <= '9')
+                                  /* followed by ISO 8601 T delimiter and number is ok */
+                                  ;
+                                else if ((*s >= '0' && *s <= '9') ||
+                                         (*s >= 'A' && *s <= 'Z') ||
+                                         (*s >= 'a' && *s <= 'z'))
+                                  /* but other alphanumerics are not ok */
                                   break;
 
-                                /* Ok, we parsed three 1-2 digit numbers, with / or -
+                                /* Ok, we parsed three multi-digit numbers, with / or -
                                    between them.  Now decide what the hell they are
-                                   (DD/MM/YY or MM/DD/YY or YY/MM/DD.)
+                                   (DD/MM/YY or MM/DD/YY or [YY]YY/MM/DD.)
                                  */
 
-                                if (n1 > 31 || n1 == 0)  /* must be YY/MM/DD */
+                                if (n1 > 31 || n1 == 0)  /* must be [YY]YY/MM/DD */
                                   {
                                         if (n2 > 12) break;
                                         if (n3 > 31) break;
                                         year = n1;
                                         if (year < 70)
                                             year += 2000;
                                         else if (year < 100)
                                             year += 1900;
                                         month = (TIME_TOKEN)(n2 + ((int)TT_JAN) - 1);
                                         date = n3;
@@ skipping 84 matching lines @@
 
           /* Skip to the end of this token, whether we parsed it or not.
                  Tokens are delimited by whitespace, or ,;-/
                  But explicitly not :+-.
            */
           while (*rest &&
                          *rest != ' ' && *rest != '\t' &&
                          *rest != ',' && *rest != ';' &&
                          *rest != '-' && *rest != '+' &&
                          *rest != '/' &&
-                         *rest != '(' && *rest != ')' && *rest != '[' && *rest != ']')
+                         *rest != '(' && *rest != ')' && *rest != '[' && *rest != ']' &&
+                         !(*rest == 'T' && rest[1] >= '0' && rest[1] <= '9')  /* T precedes time in ISO 8601 */

[wtc, 2014/05/12 19:47:15]

Nit: maybe this comment should be moved to the comment block at line 1060.

[Thiemo Nagel, 2014/05/13 14:08:19]

Done.  At that occasion, I've updated the pre-existing comment which was out of
sync with the code.

+                )
                 rest++;
           /* skip over uninteresting chars. */
         SKIP_MORE:
-          while (*rest &&

[Thiemo Nagel, 2014/05/09 16:19:00]

*rest is redundant here.

-                         (*rest == ' ' || *rest == '\t' ||
-                          *rest == ',' || *rest == ';' || *rest == '/' ||
-                          *rest == '(' || *rest == ')' || *rest == '[' || *rest == ']'))
+          while (*rest == ' ' || *rest == '\t' ||
+                 *rest == ',' || *rest == ';' || *rest == '/' ||
+                 *rest == '(' || *rest == ')' || *rest == '[' || *rest == ']')
                 rest++;
 
           /* "-" is ignored at the beginning of a token if we have not yet
                  parsed a year (e.g., the second "-" in "30-AUG-1966"), or if
                  the character after the dash is not a digit. */         
           if (*rest == '-' && ((rest > string &&
               isalpha((unsigned char)rest[-1]) && year < 0) ||
               rest[1] < '0' || rest[1] > '9'))
                 {
                   rest++;
                   goto SKIP_MORE;
                 }
 
-        }
+          /* Skip T that may precede ISO 8601 time. */
+          if (*rest == 'T' && rest[1] >= '0' && rest[1] <= '9')
+            rest++;
+

[wtc, 2014/05/12 19:47:15]

Nit: delete this blank line.

[Thiemo Nagel, 2014/05/13 14:08:19]

Done.

+        }  /* while */
 
   if (zone != TT_UNKNOWN && zone_offset == -1)
         {
           switch (zone)
                 {
                 case TT_PST: zone_offset = -8 * 60; break;
                 case TT_PDT: zone_offset = -8 * 60; dst_offset = 1 * 60; break;
                 case TT_MST: zone_offset = -7 * 60; break;
                 case TT_MDT: zone_offset = -7 * 60; dst_offset = 1 * 60; break;
                 case TT_CST: zone_offset = -6 * 60; break;
@@ skipping 14 matching lines @@
         }
 
   /* If we didn't find a year, month, or day-of-the-month, we can't
          possibly parse this, and in fact, mktime() will do something random
          (I'm seeing it return "Tue Feb  5 06:28:16 2036", which is no doubt
          a numerologically significant date... */
   if (month == TT_UNKNOWN || date == -1 || year == -1 || year > PR_INT16_MAX)
       return PR_FAILURE;
 
   memset(result, 0, sizeof(*result));
+  if (usec != -1)
+        result->tm_usec = usec; 
   if (sec != -1)
         result->tm_sec = sec;
   if (min != -1)
         result->tm_min = min;
   if (hour != -1)
         result->tm_hour = hour;
   if (date != -1)
         result->tm_mday = date;
   if (month != TT_UNKNOWN)
         result->tm_month = (((int)month) - ((int)TT_JAN));
@@ skipping 102 matching lines @@
                               + 60 * localTime.tm_hour
                               + 1440 * (localTime.tm_mday - 2);
         }
 
   result->tm_params.tp_gmt_offset = zone_offset * 60;
   result->tm_params.tp_dst_offset = dst_offset * 60;
 
   *result_imploded = PR_ImplodeTime(result);
   return PR_SUCCESS;
 }