maria-developers team mailing list archive

Thread
Date

Re: Review request: mdev-4635

To: Sergei Golubchik <serg@xxxxxxxxxxx>
From: Alexander Barkov <bar@xxxxxxxxxxx>
Date: Thu, 13 Jun 2013 21:22:27 +0400
Cc: maria-developers@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20130613161606.GA24527@meddwl.fritz.box>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6

Hi, Serg,

On 06/13/2013 08:16 PM, Sergei Golubchik wrote:

Hi, Alexander!

On Jun 13, Alexander Barkov wrote:


please review a fix for MDEV-4635


Is it for 5.1 or 5.5?


For 5.3.

=== modified file 'mysql-test/r/func_time.result'
--- mysql-test/r/func_time.result	2013-03-20 15:13:00 +0000
+++ mysql-test/r/func_time.result	2013-06-13 08:39:15 +0000
@@ -1910,3 +1910,13 @@ SELECT 1 FROM DUAL WHERE MINUTE(TIMEDIFF
  1
  SELECT 1 FROM DUAL WHERE SECOND(TIMEDIFF(NULL, '12:12:12'));
  1
+#
+# MDEV-4635 Crash in UNIX_TIMESTAMP(STR_TO_DATE('2020','%Y'))
+#
+SET TIME_ZONE='+02:00';
+SELECT UNIX_TIMESTAMP(STR_TO_DATE('2020','%Y'));
+UNIX_TIMESTAMP(STR_TO_DATE('2020','%Y'))
+NULL
+Warnings:
+Error	1411	Incorrect datetime value: '2020' for function str_to_date
+SET TIME_ZONE=DEFAULT;

=== modified file 'mysql-test/r/strict.result'
--- mysql-test/r/strict.result	2011-06-05 02:56:06 +0000
+++ mysql-test/r/strict.result	2013-06-13 12:15:43 +0000
@@ -213,11 +213,11 @@ ERROR 22007: Incorrect date value: '2004
  INSERT INTO t1 (col1) VALUES(STR_TO_DATE('0.10.2004 15.30','%d.%m.%Y %H.%i'));
  ERROR 22007: Incorrect date value: '2004-10-00 15:30:00' for column 'col1' at row 1
  INSERT INTO t1 (col1) VALUES(STR_TO_DATE('31.9.2004 15.30','%d.%m.%Y %H.%i'));
-ERROR 22007: Incorrect date value: '2004-09-31 15:30:00' for column 'col1' at row 1
+ERROR HY000: Incorrect datetime value: '31.9.2004 15.30' for function str_to_date


Hmm, I don't know. I liked the old error message more.
Indeed, datetime value '31.9.2004 15.30' is just fine for a function str_to_date().
But '2004-09-31 15:30:00' is not fine for column 'col1' in the strict mode.

Why did that change?


Before the change the warning was generated in
Field_timestamp::store_TIME_with_warning.

Now this warning is generated much earlier,
in Item_func_str_to_date::get_date(),
after check_date() is called.


The problem was that Item_func_str_to_date:
a. checked TIME_NO_ZERO_DATE
b. did not check TIME_NO_ZERO_IN_DATE (the bug)
c. did not check invalid dates


After the patch it:

a. checks TIME_NO_ZERO_DATE
b. checks TIME_NO_ZERO_IN_DATE
c. checks invalid dates

I thought it would be good to check everything,
for consistency. But a side effect is a different warning.


But we can suppress invalid dates detection, so it:

a. checks TIME_NO_ZERO_DATE
b. checks TIME_NO_ZERO_IN_DATE
c. does not check invalid dates

The test for invalid dates can be suppressed
by passing "fuzzy_date | TIME_INVALID_DATES"
to check_date(). See this version of the patch attached.


I agree that the old warning is nicer. But I'm not sure what's better.
Suppressing the test rather slightly looks like hiding more potential
bugs...

=== modified file 'sql/item_timefunc.cc'
--- sql/item_timefunc.cc	2013-03-20 15:13:00 +0000
+++ sql/item_timefunc.cc	2013-06-13 08:20:13 +0000
@@ -1126,7 +1125,7 @@ bool Item_func_unix_timestamp::get_times
    }

    MYSQL_TIME ltime;
-  if (get_arg0_date(&ltime, 0))
+  if (get_arg0_date(&ltime, TIME_NO_ZERO_IN_DATE))
      return 1;


This is correct, of course.

Regards,
Sergei

=== modified file 'mysql-test/r/func_time.result'
--- mysql-test/r/func_time.result	2013-03-20 15:13:00 +0000
+++ mysql-test/r/func_time.result	2013-06-13 08:39:15 +0000
@@ -1910,3 +1910,13 @@ SELECT 1 FROM DUAL WHERE MINUTE(TIMEDIFF
 1
 SELECT 1 FROM DUAL WHERE SECOND(TIMEDIFF(NULL, '12:12:12'));
 1
+#
+# MDEV-4635 Crash in UNIX_TIMESTAMP(STR_TO_DATE('2020','%Y'))
+#
+SET TIME_ZONE='+02:00';
+SELECT UNIX_TIMESTAMP(STR_TO_DATE('2020','%Y'));
+UNIX_TIMESTAMP(STR_TO_DATE('2020','%Y'))
+NULL
+Warnings:
+Error	1411	Incorrect datetime value: '2020' for function str_to_date
+SET TIME_ZONE=DEFAULT;

=== modified file 'mysql-test/t/func_time.test'
--- mysql-test/t/func_time.test	2013-03-20 15:13:00 +0000
+++ mysql-test/t/func_time.test	2013-06-13 08:29:09 +0000
@@ -1154,3 +1154,11 @@ SELECT 1 FROM DUAL WHERE DAYOFMONTH(TIME
 SELECT 1 FROM DUAL WHERE HOUR(TIMEDIFF(NULL, '12:12:12'));
 SELECT 1 FROM DUAL WHERE MINUTE(TIMEDIFF(NULL, '12:12:12'));
 SELECT 1 FROM DUAL WHERE SECOND(TIMEDIFF(NULL, '12:12:12'));
+
+
+--echo #
+--echo # MDEV-4635 Crash in UNIX_TIMESTAMP(STR_TO_DATE('2020','%Y'))
+--echo #
+SET TIME_ZONE='+02:00';
+SELECT UNIX_TIMESTAMP(STR_TO_DATE('2020','%Y'));
+SET TIME_ZONE=DEFAULT;

=== modified file 'sql/item_timefunc.cc'
--- sql/item_timefunc.cc	2013-03-20 15:13:00 +0000
+++ sql/item_timefunc.cc	2013-06-13 16:50:38 +0000
@@ -396,8 +396,8 @@ static bool extract_date_time(DATE_TIME_
       l_time->minute > 59 || l_time->second > 59)
     goto err;
 
-  if ((fuzzy_date & TIME_NO_ZERO_DATE) &&
-       (l_time->year == 0 || l_time->month == 0 || l_time->day == 0))
+  int was_cut;
+  if (check_date(l_time, fuzzy_date | TIME_INVALID_DATES, &was_cut))
     goto err;
 
   if (val != val_end)
@@ -799,7 +799,6 @@ longlong Item_func_to_days::val_int_endp
   res=(longlong) calc_daynr(ltime.year,ltime.month,ltime.day);
   /* Set to NULL if invalid date, but keep the value */
   null_value= check_date(&ltime,
-                         (ltime.year || ltime.month || ltime.day),
                          (TIME_NO_ZERO_IN_DATE | TIME_NO_ZERO_DATE),
                          &dummy);
   if (null_value)
@@ -1126,7 +1125,7 @@ bool Item_func_unix_timestamp::get_times
   }
 
   MYSQL_TIME ltime;
-  if (get_arg0_date(&ltime, 0))
+  if (get_arg0_date(&ltime, TIME_NO_ZERO_IN_DATE))
     return 1;
 
   uint error_code;
@@ -2309,8 +2308,7 @@ bool Item_date_typecast::get_date(MYSQL_
   ltime->time_type= MYSQL_TIMESTAMP_DATE;
 
   int unused;
-  if (check_date(ltime, ltime->year || ltime->month || ltime->day,
-                 fuzzy_date, &unused))
+  if (check_date(ltime, fuzzy_date, &unused))
   {
     Lazy_string_time str(ltime);
     make_truncated_value_warning(current_thd, MYSQL_ERROR::WARN_LEVEL_WARN,

=== modified file 'sql/mysql_priv.h'
--- sql/mysql_priv.h	2013-02-28 20:48:47 +0000
+++ sql/mysql_priv.h	2013-06-13 08:48:11 +0000
@@ -2558,7 +2558,16 @@ extern bool make_date_time(DATE_TIME_FOR
 int my_time_compare(MYSQL_TIME *a, MYSQL_TIME *b);
 longlong get_datetime_value(THD *thd, Item ***item_arg, Item **cache_arg,
                             Item *warn_item, bool *is_null);
-
+static inline bool
+non_zero_date(const MYSQL_TIME *ltime)
+{
+  return ltime->year || ltime->month || ltime->day;
+}
+static inline bool
+check_date(const MYSQL_TIME *ltime, ulonglong flags, int *was_cut)
+{
+  return check_date(ltime, non_zero_date(ltime), flags, was_cut);
+}
 int test_if_number(char *str,int *res,bool allow_wildcards);
 void change_byte(uchar *,uint,char,char);
 bool init_read_record(READ_RECORD *info, THD *thd, TABLE *reg_form,

References

Review request: mdev-4635
From: Alexander Barkov, 2013-06-13