Bug#31227: memory overrun with decimal (6,6) and zerofill and group_concat

Reserve the space for the leading 0 (before the decimal point) in DECIMAL(a,a) ZEROFILL.


mysql-test/r/type_decimal.result:
  show that we allocate a large enough buffer for output of DECIMAL(a,a) [ZEROFILL].
mysql-test/t/type_decimal.test:
  show that we allocate a large enough buffer for output of DECIMAL(a,a) [ZEROFILL].
  without patch for bug#31227, valgrind will complain here; so will a
  debug build.
sql/my_decimal.cc:
  Reserve the space for the leading 0 (before the decimal point) in DECIMAL(a,a) ZEROFILL.
  Doxygenise preamble of my_decimal2string().

mysql-test/r/type_decimal.result

@@ -683,6 +683,7 @@ select * from t1;
 a	b
 123.12345	123.1
 drop table t1;
+End of 4.1 tests
 CREATE TABLE t1
 (EMPNUM   CHAR(3) NOT NULL,
 HOURS    DECIMAL(5));
@@ -799,3 +800,10 @@ SELECT ROUND(qty,3), dps, ROUND(qty,dps) FROM t1;
 ROUND(qty,3)	dps	ROUND(qty,dps)
 1.133	3	1.133
 DROP TABLE t1;
+create table t1 (f1 decimal(6,6),f2 decimal(6,6) zerofill);
+insert into t1 values (-0.123456,0.123456);
+select group_concat(f1),group_concat(f2) from t1;
+group_concat(f1)	group_concat(f2)
+-0.123456	0.123456
+drop table t1;
+End of 5.0 tests

mysql-test/t/type_decimal.test

@@ -278,7 +278,7 @@ update t1 set b=a;
 select * from t1;                                                               
 drop table t1;                                                                  
 
-# End of 4.1 tests
+--echo End of 4.1 tests
 
 #
 # Test for BUG#8397: decimal type in subselects (Item_cache_decimal)
@@ -408,3 +408,14 @@ INSERT INTO t1 VALUES (1.1325,3);
 SELECT ROUND(qty,3), dps, ROUND(qty,dps) FROM t1;
 
 DROP TABLE t1;
+
+#
+# Bug #31227: memory overrun with decimal (6,6) and zerofill and group_concat
+# valgrind will complain about this (the group_concat(f2)) on unpatched mysqld.
+#
+create table t1 (f1 decimal(6,6),f2 decimal(6,6) zerofill);
+insert into t1 values (-0.123456,0.123456);
+select group_concat(f1),group_concat(f2) from t1;
+drop table t1;
+
+--echo End of 5.0 tests

sql/my_decimal.cc

@@ -68,24 +68,43 @@ int decimal_operation_results(int result)
 }
 
 
-/*
-  Converting decimal to string
-
-  SYNOPSIS
-     my_decimal2string()
-
-  return
-    E_DEC_OK
-    E_DEC_TRUNCATED
-    E_DEC_OVERFLOW
-    E_DEC_OOM
+/**
+  @brief Converting decimal to string
+
+  @details Convert given my_decimal to String; allocate buffer as needed.
+
+  @param[in]   mask        what problems to warn on (mask of E_DEC_* values)
+  @param[in]   d           the decimal to print
+  @param[in]   fixed_prec  overall number of digits if ZEROFILL, 0 otherwise
+  @param[in]   fixed_dec   number of decimal places (if fixed_prec != 0)
+  @param[in]   filler      what char to pad with (ZEROFILL et al.)
+  @param[out]  *str        where to store the resulting string
+
+  @return error coce
+    @retval E_DEC_OK
+    @retval E_DEC_TRUNCATED
+    @retval E_DEC_OVERFLOW
+    @retval E_DEC_OOM
 */
 
 int my_decimal2string(uint mask, const my_decimal *d,
                       uint fixed_prec, uint fixed_dec,
                       char filler, String *str)
 {
-  int length= (fixed_prec ? (fixed_prec + 1) : my_decimal_string_length(d));
+  /*
+    Calculate the size of the string: For DECIMAL(a,b), fixed_prec==a
+    holds true iff the type is also ZEROFILL, which in turn implies
+    UNSIGNED. Hence the buffer for a ZEROFILLed value is the length
+    the user requested, plus one for a possible decimal point, plus
+    one if the user only wanted decimal places, but we force a leading
+    zero on them. Because the type is implicitly UNSIGNED, we do not
+    need to reserve a character for the sign. For all other cases,
+    fixed_prec will be 0, and my_decimal_string_length() will be called
+    instead to calculate the required size of the buffer.
+  */
+  int length= (fixed_prec
+               ? (fixed_prec + ((fixed_prec == fixed_dec) ? 1 : 0) + 1)
+               : my_decimal_string_length(d));
   int result;
   if (str->alloc(length))
     return check_result(mask, E_DEC_OOM);