android: fix base::Time::FromLocalExploded() crash.

base/time/time_posix.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/time/time.h"
 
 #include <stdint.h>
 #include <sys/time.h>
 #include <time.h>
 #if defined(OS_ANDROID)
@@ skipping 190 matching lines @@
 Time Time::FromExploded(bool is_local, const Exploded& exploded) {
   struct tm timestruct;
   timestruct.tm_sec    = exploded.second;
   timestruct.tm_min    = exploded.minute;
   timestruct.tm_hour   = exploded.hour;
   timestruct.tm_mday   = exploded.day_of_month;
   timestruct.tm_mon    = exploded.month - 1;
   timestruct.tm_year   = exploded.year - 1900;
   timestruct.tm_wday   = exploded.day_of_week;  // mktime/timegm ignore this
   timestruct.tm_yday   = 0;     // mktime/timegm ignore this
   timestruct.tm_isdst  = -1;    // attempt to figure it out

[jar (doing other things), 2013/10/16 21:36:34]

Did you consider using a definite isdst setting of 0 or 1?

For some time periods, you should fail, and this will happen at times other than
just after midnight.

The problem is that some exploded times have *no* corresponding
seconds-since-epoch, since they are "skipped."  

If you have a specific dst setting, then I *think* you won't fail... 

[digit1, 2013/10/17 14:00:27]

That's an excellent idea. I've performed some more extensive testing, and on
Android, mktime() can also return -1 depending on the timezone.

I've solved by doing something like:

  mktime() as usual.
  if (result == -1):
      s0 = mktime() with tm_isdst == 0
      s1 = mktime() with tm_isdst == 1
      return the smallest of s0 and s1 that is not -1

This seems to take care of the problem in a more generic way.

 #if !defined(OS_NACL) && !defined(OS_SOLARIS)
   timestruct.tm_gmtoff = 0;     // not a POSIX field, so mktime/timegm ignore
   timestruct.tm_zone   = NULL;  // not a POSIX field, so mktime/timegm ignore
 #endif
 
-  SysTime seconds = SysTimeFromTimeStruct(&timestruct, is_local);
 
   int64 milliseconds;
+  SysTime seconds;
+
+// On older releases of Android, mktime() will sometimes return -1 when given
+// any second after midnight in certain timezones. For more details, see:
+//   http://b.android.com/61137
+#if defined(OS_ANDROID)
+  // SysTimeFromTimeStruct() modifies the input structure, save current value.
+  struct tm timestruct0 = timestruct;
+
+  seconds = SysTimeFromTimeStruct(&timestruct, is_local);
+  if (seconds == -1 && exploded.hour == 0) {
+    // Work-around this by trying to add one hour.
+    timestruct0.tm_hour = 1;
+    seconds = SysTimeFromTimeStruct(&timestruct0, is_local);
+    if (seconds != -1)
+      seconds -= 60 * 60;
+  }
+#else
+  seconds = SysTimeFromTimeStruct(&timestruct, is_local);
+#endif
+
   // Handle overflow.  Clamping the range to what mktime and timegm might
   // return is the best that can be done here.  It's not ideal, but it's better
   // than failing here or ignoring the overflow case and treating each time
   // overflow as one second prior to the epoch.
   if (seconds == -1 &&
       (exploded.year < 1969 || exploded.year > 1970)) {
     // If exploded.year is 1969 or 1970, take -1 as correct, with the
     // time indicating 1 second prior to the epoch.  (1970 is allowed to handle
     // time zone and DST offsets.)  Otherwise, return the most future or past
     // time representable.  Assumes the time_t epoch is 1970-01-01 00:00:00 UTC.
     //
     // The minimum and maximum representible times that mktime and timegm could
     // return are used here instead of values outside that range to allow for
     // proper round-tripping between exploded and counter-type time
     // representations in the presence of possible truncation to time_t by
     // division and use with other functions that accept time_t.
     //
     // When representing the most distant time in the future, add in an extra
     // 999ms to avoid the time being less than any other possible value that
     // this function can return.
+
+    // On Android, SysTime is int64, special care must be taken to avoid
+    // overflows.
+    const int64 min_seconds = (sizeof(SysTime) < sizeof(int64))
+                                  ? std::numeric_limits<SysTime>::min()
+                                  : std::numeric_limits<int32_t>::min();

[digit1, 2013/10/17 14:00:27]

I'm still not sure that using INT32_MAX here is sufficient, but apparently this
code is very cautious because it assumes that the client may perform many
computations with the result, so directly returning INT64_MIN/MAX here might not
be a good idea. What do you think about this?

+    const int64 max_seconds = (sizeof(SysTime) < sizeof(int64))
+                                  ? std::numeric_limits<SysTime>::max()
+                                  : std::numeric_limits<int32_t>::max();
     if (exploded.year < 1969) {
-      CHECK(sizeof(SysTime) < sizeof(int64)) << "integer overflow";
-      milliseconds = std::numeric_limits<SysTime>::min();
-      milliseconds *= kMillisecondsPerSecond;
+      milliseconds = min_seconds * kMillisecondsPerSecond;
     } else {
-      CHECK(sizeof(SysTime) < sizeof(int64)) << "integer overflow";
-      milliseconds = std::numeric_limits<SysTime>::max();
-      milliseconds *= kMillisecondsPerSecond;
+      milliseconds = max_seconds * kMillisecondsPerSecond;
       milliseconds += (kMillisecondsPerSecond - 1);
     }
   } else {
     milliseconds = seconds * kMillisecondsPerSecond + exploded.millisecond;
   }
 
   // Adjust from Unix (1970) to Windows (1601) epoch.
   return Time((milliseconds * kMicrosecondsPerMillisecond) +
       kWindowsEpochDeltaMicroseconds);
 }
@@ skipping 85 matching lines @@
     result.tv_usec = static_cast<suseconds_t>(Time::kMicrosecondsPerSecond) - 1;
     return result;
   }
   int64 us = us_ - kTimeTToMicrosecondsOffset;
   result.tv_sec = us / Time::kMicrosecondsPerSecond;
   result.tv_usec = us % Time::kMicrosecondsPerSecond;
   return result;
 }
 
 }  // namespace base