Google::Protobuf::Timestamp.decode_json sometimes return incorrect timestamp values in Ruby

[south37]

What version of protobuf and what language are you using?
Version: v3.8.0
Language: Ruby

What operating system (Linux, Windows, ...) and version?
MacOS Mojave 10.14.5

What runtime / compiler are you using (e.g., python version or gcc version)

$ ruby --version
ruby 2.6.3p62 (2019-04-16 revision 67580) [x86_64-darwin18]

What did you do?

1. Use Google::Protobuf::Timestamp.decode_json to decode a string in RFC339-format.

[1] pry(main)> t = Google::Protobuf::Timestamp.decode_json('"1993-01-10T00:00:00.000+00:00"')
=> <Google::Protobuf::Timestamp: seconds: 726537600, nanos: 0>

2. Use a decoded Google::Protobuf::Timestamp object to represent a time.

[2] pry(main)> t.to_json
=> "\"1993-01-09T00:00:00Z\""

[3] pry(main)> Time.at(t.seconds)
=> 1993-01-09 00:00:00 +0000

[4] pry(main)> t.seconds
=> 726537600

What did you expect to see
I decoded 1993-01-10, so I expected to see 1993-01-10 as a decoded object.

What did you see instead?
I saw 1993-01-09. This is the date before the expected date.

Anything else we should know about your project / environment

I tried other dates. Many dates were valid, but some were invalid.
I think some dates in some years (e.g. 1993, 1989, 1967, etc.) are invalid, but I don't know why they are invalid.

[1] pry(main)> t = Google::Protobuf::Timestamp.decode_json('"1993-02-10T00:00:00.000+00:00"'); Time.at(t.seconds)
=> 1993-02-09 00:00:00 +0000

[2] pry(main)> t = Google::Protobuf::Timestamp.decode_json('"1989-01-10T00:00:00.000+00:00"'); Time.at(t.seconds)
=> 1989-01-09 00:00:00 +0000

[3] pry(main)> t = Google::Protobuf::Timestamp.decode_json('"1967-01-10T00:00:00.000+00:00"'); Time.at(t.seconds)
=> 1967-01-11 00:00:00 +0000

[south37]

After investigating the cause of this problem, I found that upb_mktime sometimes returns an invalid epoch time.

protobuf/ruby/ext/google/protobuf_c/upb.c

Lines 10147 to 10160 in 3782317

	static int64_t upb_mktime(const struct tm *tp) {
	int sec = tp->tm_sec;
	int min = tp->tm_min;
	int hour = tp->tm_hour;
	int mday = tp->tm_mday;
	int mon = tp->tm_mon;
	int year = tp->tm_year + TM_YEAR_BASE;
	/* Calculate day of year from year, month, and day of month. */
	int mon_yday = ((__mon_yday[isleap(year)][mon]) - 1);
	int yday = mon_yday + mday;
	return epoch(year, yday, hour, min, sec);
	}

I tested on my Mac(MacOS Mojave 10.14.5). The time zone is Asia/Tokyo.

I tested 1993-02-10, and saw 729216000 as a result. This is "1993-02-09 09:00:00" in unixtime.

$ gcc main.c && ./a.out
729216000  # This is "1993-02-09 09:00:00" in unixtime. I tested 1993-02-10, so this is wrong.

$ sudo systemsetup -gettimezone
Password:
Time Zone: Asia/Tokyo

The test code is shown below.

// main.c

#include "stdio.h"
#include "time.h"
#include "stdbool.h"

#define EPOCH_YEAR 1970
#define TM_YEAR_BASE 1900

static bool isleap(int year) {
  return (year % 4) == 0 && (year % 100 != 0 || (year % 400) == 0);
}

const unsigned short int __mon_yday[2][13] = {
    /* Normal years.  */
    { 0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365 },
    /* Leap years.  */
    { 0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366 }
};

int64_t epoch(int year, int yday, int hour, int min, int sec) {
  int64_t years = year - EPOCH_YEAR;

  int64_t leap_days = years / 4 - years / 100 + years / 400;

  int64_t days = years * 365 + yday + leap_days;
  int64_t hours = days * 24 + hour;
  int64_t mins = hours * 60 + min;
  int64_t secs = mins * 60 + sec;
  return secs;
}

static int64_t upb_mktime(const struct tm *tp) {
  int sec = tp->tm_sec;
  int min = tp->tm_min;
  int hour = tp->tm_hour;
  int mday = tp->tm_mday;
  int mon = tp->tm_mon;
  int year = tp->tm_year + TM_YEAR_BASE;

  /* Calculate day of year from year, month, and day of month. */
  int mon_yday = ((__mon_yday[isleap(year)][mon]) - 1);
  int yday = mon_yday + mday;

  return epoch(year, yday, hour, min, sec);
}

int main(int argc, char** argv) {
  // We try 1993-02-10
  struct tm m = {
    0,  /* tm_sec */
    0,  /* tm_min */
    0,  /* tm_hour */
    10, /* tm_mday */
    1,  /* tm_mon, 2 */
    93  /* tm_year, 1993 */
  };

  int64_t sec = upb_mktime(&m);
  printf("%lld\n", sec);
  // Answer is 729216000.
  // This is "1993-02-09 09:00:00" in unixtime.

  return 0;
}

I also tested other dates. When I tested 1990-02-10, the result was correct.

$ gcc test_19900210.c && ./a.out
634608000  # This is "1990-02-10 09:00:00" in unixtime. I tested 1990-02-10, so this is correct.

The test code is shown below.

// test_19900210.c

#include "stdio.h"
#include "time.h"
#include "stdbool.h"

#define EPOCH_YEAR 1970
#define TM_YEAR_BASE 1900

static bool isleap(int year) {
  return (year % 4) == 0 && (year % 100 != 0 || (year % 400) == 0);
}

const unsigned short int __mon_yday[2][13] = {
    /* Normal years.  */
    { 0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365 },
    /* Leap years.  */
    { 0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366 }
};

int64_t epoch(int year, int yday, int hour, int min, int sec) {
  int64_t years = year - EPOCH_YEAR;

  int64_t leap_days = years / 4 - years / 100 + years / 400;

  int64_t days = years * 365 + yday + leap_days;
  int64_t hours = days * 24 + hour;
  int64_t mins = hours * 60 + min;
  int64_t secs = mins * 60 + sec;
  return secs;
}

static int64_t upb_mktime(const struct tm *tp) {
  int sec = tp->tm_sec;
  int min = tp->tm_min;
  int hour = tp->tm_hour;
  int mday = tp->tm_mday;
  int mon = tp->tm_mon;
  int year = tp->tm_year + TM_YEAR_BASE;

  /* Calculate day of year from year, month, and day of month. */
  int mon_yday = ((__mon_yday[isleap(year)][mon]) - 1);
  int yday = mon_yday + mday;

  return epoch(year, yday, hour, min, sec);
}

int main(int argc, char** argv) {
  // I try 1990-02-10
  struct tm m = {
    0,  /* tm_sec */
    0,  /* tm_min */
    0,  /* tm_hour */
    10, /* tm_mday */
    1,  /* tm_mon, 2 */
    90  /* tm_year, 1990 */
  };

  int64_t sec = upb_mktime(&m);
  printf("%lld\n", sec);
  // Answer is 634608000
  // This is "1990-02-10 09:00:00" in unixtime.

  return 0;
}

$ diff main.c test_19900210.c
47c47
<   // I try 1993-02-10
---
>   // I try 1990-02-10
54c54
<     93  /* tm_year, 1993 */
---
>     90  /* tm_year, 1990 */
59,60c59,60
<   // Answer is 729216000
<   // This is "1993-02-09 09:00:00" in unixtime.
---
>   // Answer is 634608000
>   // This is "1990-02-10 09:00:00" in unixtime.

[south37]

I think this issue was fixed in #6695 , so I close this. 💡
cc. @haberman @TeBoring