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

base/third_party/nspr/prtime.cc

Index: base/third_party/nspr/prtime.cc
diff --git a/base/third_party/nspr/prtime.cc b/base/third_party/nspr/prtime.cc
index 0c97bc16d8832c793da8269c01bc8d9350e1e404..7446275a053c939cac650c43702a38b5e98a11a8 100644
--- a/base/third_party/nspr/prtime.cc
+++ b/base/third_party/nspr/prtime.cc
@@ -61,6 +61,8 @@
  * 1. prtime.h
  * 2. prtypes.h
  * 3. prlong.h
+ *
+ * Unit tests are in base/time/pr_time_unittest.cc.

[wtc, 2014/05/08 21:04:19]

Maybe this entire comment block should be moved to
base/third_party/nspr/README.chromium?

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

I don't have an opinion about that...

  */
 
 #include "base/logging.h"
@@ -505,6 +507,7 @@ typedef enum
  *   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
@@ -531,6 +534,7 @@ PR_ParseTimeString(
   int hour = -1;
   int min = -1;
   int sec = -1;
+  int usec = -1;
 
   const char *rest = string;
 
@@ -774,6 +778,7 @@ PR_ParseTimeString(
                         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++;
@@ -833,6 +838,31 @@ PR_ParseTimeString(
                                 else
                                   tmp_sec = (rest[0]-'0');
 
+                                /* 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';
+                                    } else if (end - rest == 6) {
+                                      if ('5' <= *end && *end <= '9') tmp_usec++;  /* round to nearest */

[wtc, 2014/05/08 21:04:19]

I suggest we always truncate. The reason is that rounding up may produce
surprising results. For example:

  2014-05-08T23:59:59.9999995Z

would be rounded up to

  2014-05-09T00:00:00.000000Z

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

I wouldn't consider that surprising...  ;-)

Anyways, I'm truncating now.

+                                    }
+                                    end++;
+                                  }
+                                  int ndigits = end - rest;
+                                  while (ndigits++ < 6) tmp_usec *= 10;

[wtc, 2014/05/08 21:04:19]

Nit: put
    tmp_usec *= 10;
on a separate line.

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

Done.

+                                }
+
+                                rest = end;
+                                if (*rest == 'Z') {

[wtc, 2014/05/08 21:04:19]

Should we only allow 'Z' if 'T' was used as a delimiter?

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

I wouldn't do that since according to Wikipedia "it is permitted to omit the 'T'
character by mutual agreement."

+                                  zone = TT_GMT;
+                                  end++;

[wtc, 2014/05/08 21:04:19]

It seems that we should skip the AM/PM processing code if we have seen 'Z'.

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

Done.

+                                }
+
                                 /* If we made it here, we've parsed hour and min,
                                    and possibly sec, so it worked as a unit. */
 
@@ -858,6 +888,7 @@ PR_ParseTimeString(
                                 hour = tmp_hour;
                                 min = tmp_min;
                                 sec = tmp_sec;
+                                usec = tmp_usec;
                                 rest = end;
                                 break;
                           }
@@ -865,8 +896,7 @@ PR_ParseTimeString(
                                          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;
@@ -877,10 +907,18 @@ PR_ParseTimeString(
 
                                 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;
+                                        n1 = n1*10 + (*s++ - '0');
+                                  }
+
                                 if (*s != '/' && *s != '-')                /* slash */
                                   break;
                                 s++;
@@ -911,9 +949,10 @@ PR_ParseTimeString(
                                           n3 = n3*10 + (*s++ - '0');
                                   }
 
-                                if ((*s >= '0' && *s <= '9') ||        /* followed by non-alphanum */
-                                        (*s >= 'A' && *s <= 'Z') ||
-                                        (*s >= 'a' && *s <= 'z'))
+                                if (((*s >= '0' && *s <= '9') ||        /* followed by non-alphanum */
+                                     (*s >= 'A' && *s <= 'Z') ||
+                                     (*s >= 'a' && *s <= 'z')) &&
+                                    !(*s == 'T' && '0' <= s[1] && s[1] <= '9'))  /* allow ISO 8601 T delimiter */

[wtc, 2014/05/08 21:04:19]

I think we should skip over 'T' right here, so that we don't need to check for
'T' in the while loops on lines 1069 and 1078 below.

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

We could skip over the 'T' here, but we cannot eliminate the logic from the
"skip to the end of this token" while() loop (line 1069) as it is required to
prevent the loop from eating the rest of the token.

I've updated the CL, trying to make it more legible.  If you still think
skipping over 'T' here is better, I can do that, but I can't think of an easy
way of removing the logic from line 1069 (except by introducing a new variable
like dont_skip_this_token which I don't consider particularly elegant, either).

                                   break;
 
                                 /* Ok, we parsed three 1-2 digit numbers, with / or -

[wtc, 2014/05/08 21:04:19]

This comment needs to be updated because "three 1-2 digit numbers" is no longer
true.

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

Done.

@@ -921,7 +960,7 @@ PR_ParseTimeString(
                                    (DD/MM/YY or MM/DD/YY or YY/MM/DD.)

[wtc, 2014/05/08 21:04:19]

The last item should be YY[YY]/MM/DD or [YY]YY/MM/DD. See the next comment.

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

Done.

                                  */
 
-                                if (n1 > 31 || n1 == 0)  /* must be YY/MM/DD */
+                                if (n1 > 31 || n1 == 0)  /* must be YY[YY]/MM/DD */

[wtc, 2014/05/08 21:04:19]

Nit: [YY]YY/MM/DD seems more accurate. Not very sure about this though :-)

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

Absolutely!

                                   {
                                         if (n2 > 12) break;
                                         if (n3 > 31) break;
@@ -1026,14 +1065,19 @@ PR_ParseTimeString(
                          *rest != ',' && *rest != ';' &&
                          *rest != '-' && *rest != '+' &&
                          *rest != '/' &&
-                         *rest != '(' && *rest != ')' && *rest != '[' && *rest != ']')
+                         *rest != '(' && *rest != ')' && *rest != '[' && *rest != ']' &&
+                         !(*rest == 'T' && '0' <= rest[1] && rest[1] <= '9')  /* T precedes time in ISO 8601 */
+                )
                 rest++;
           /* skip over uninteresting chars. */
         SKIP_MORE:
           while (*rest &&
                          (*rest == ' ' || *rest == '\t' ||
                           *rest == ',' || *rest == ';' || *rest == '/' ||
-                          *rest == '(' || *rest == ')' || *rest == '[' || *rest == ']'))
+                          *rest == '(' || *rest == ')' || *rest == '[' || *rest == ']' ||
+                          (*rest == 'T' && '0' <= rest[1] && rest[1] <= '9')  /* T precedes time in ISO 8601 */
+                         )
+                )
                 rest++;
 
           /* "-" is ignored at the beginning of a token if we have not yet
@@ -1047,7 +1091,7 @@ PR_ParseTimeString(
                   goto SKIP_MORE;
                 }
 
-        }
+        }  /* while */
 
   if (zone != TT_UNKNOWN && zone_offset == -1)
         {
@@ -1082,6 +1126,8 @@ PR_ParseTimeString(
       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)