← Back to team overview

maria-developers team mailing list archive

MDEV-10425 Assertion `collation.derivation == DERIVATION_IMPLICIT' failed in Item_func_conv_charset::fix_length_and_dec()

 

Hello Sergei,

Please review.

Thanks!
commit 6c49030635a36380606948c198dcfd4a6bf2ffc3
Author: Alexander Barkov <bar@xxxxxxxxxxx>
Date:   Wed Sep 21 12:29:09 2016 +0400

    MDEV-10425 Assertion `collation.derivation == DERIVATION_IMPLICIT' failed in Item_func_conv_charset::fix_length_and_dec()
    MDEV-10850 Wrong result for WHERE .. (f2=TO_BASE64('test') OR f2=TO_BASE64('TEST'))
    
    Problem N1: MDEV-10425
    Item_func_{md5|sha|sha2}::fix_length_and_dec() changed args[0]->collation.
    It's done to treat e.g. MD5('a') and MD5('A') as different values.
    It is wrong for a Item_func_xxx to modify its arguments.
    Item_func_conv_charset did not expect that and crashed on assert.
    
    Problem N2: MDEV-10850
    Item_func_to_base64, Item_func_password, Item_func_hex are also
    hash functions, but they did not compare their arguments as binary
    
    Solution:
    - Removing the code changing args[0]->collation
    - Introducing Item_str_ascii_checksum_func as a common parent
      for Item_func_{md5|sha|sha2|password|hex|to_base64}
      and overriding its eq() method to compare arguments binary.

diff --git a/mysql-test/include/func_str_ascii_checksum.inc b/mysql-test/include/func_str_ascii_checksum.inc
new file mode 100644
index 0000000..8e51c92
--- /dev/null
+++ b/mysql-test/include/func_str_ascii_checksum.inc
@@ -0,0 +1,24 @@
+--echo # Start of func_str_ascii_checksum.inc
+
+--echo #
+--echo # MDEV-10850 Wrong result for WHERE .. (f2=TO_BASE64('test') OR f2=TO_BASE64('TEST'))
+--echo #
+
+--eval CREATE TABLE t1 (f1 VARCHAR(4), f2 VARCHAR(255), UNIQUE KEY k1 (f1,f2))
+--eval INSERT INTO t1 VALUES ('test',$func('test')), ('TEST', $func('TEST'))
+--eval SELECT * FROM t1 IGNORE INDEX(k1) WHERE f1='test' AND (f2= $func("test") OR f2= $func("TEST"))
+--eval SELECT * FROM t1                  WHERE f1='test' AND (f2= $func("test") OR f2= $func("TEST"))
+--eval SELECT * FROM t1                  WHERE f1='test' AND (f2= $func("TEST") OR f2= $func("test"))
+DROP TABLE t1;
+
+
+--echo #
+--echo # MDEV-10425 Assertion `collation.derivation == DERIVATION_IMPLICIT' failed in Item_func_conv_charset::fix_length_and_dec()
+--echo #
+
+--eval PREPARE stmt FROM "SELECT $func(CONVERT('foo' USING latin1))"
+EXECUTE stmt;
+DEALLOCATE PREPARE stmt;
+
+
+--echo # End of func_str_ascii_checksum.inc
diff --git a/mysql-test/r/func_crypt.result b/mysql-test/r/func_crypt.result
index 1eda56a..c3d7bf6 100644
--- a/mysql-test/r/func_crypt.result
+++ b/mysql-test/r/func_crypt.result
@@ -106,3 +106,65 @@ OLD_PASSWORD(c1)	PASSWORD(c1)
 77023ffe214c04ff	*82E58A2C08AAFE72C8EB523069CD8ADB33F78F58
 DROP TABLE t1;
 End of 5.0 tests
+#
+# Start of 10.1 tests
+#
+# Start of func_str_ascii_checksum.inc
+#
+# MDEV-10850 Wrong result for WHERE .. (f2=TO_BASE64('test') OR f2=TO_BASE64('TEST'))
+#
+CREATE TABLE t1 (f1 VARCHAR(4), f2 VARCHAR(255), UNIQUE KEY k1 (f1,f2));
+INSERT INTO t1 VALUES ('test',password('test')), ('TEST', password('TEST'));
+SELECT * FROM t1 IGNORE INDEX(k1) WHERE f1='test' AND (f2= password("test") OR f2= password("TEST"));
+f1	f2
+test	*94BDCEBE19083CE2A1F959FD02F964C7AF4CFC29
+TEST	*47A6B0EA08A36FAEBE4305B373FE37E3CF27C357
+SELECT * FROM t1                  WHERE f1='test' AND (f2= password("test") OR f2= password("TEST"));
+f1	f2
+TEST	*47A6B0EA08A36FAEBE4305B373FE37E3CF27C357
+test	*94BDCEBE19083CE2A1F959FD02F964C7AF4CFC29
+SELECT * FROM t1                  WHERE f1='test' AND (f2= password("TEST") OR f2= password("test"));
+f1	f2
+TEST	*47A6B0EA08A36FAEBE4305B373FE37E3CF27C357
+test	*94BDCEBE19083CE2A1F959FD02F964C7AF4CFC29
+DROP TABLE t1;
+#
+# MDEV-10425 Assertion `collation.derivation == DERIVATION_IMPLICIT' failed in Item_func_conv_charset::fix_length_and_dec()
+#
+PREPARE stmt FROM "SELECT password(CONVERT('foo' USING latin1))";
+EXECUTE stmt;
+password(CONVERT('foo' USING latin1))
+*F3A2A51A9B0F2BE2468926B4132313728C250DBF
+DEALLOCATE PREPARE stmt;
+# End of func_str_ascii_checksum.inc
+# Start of func_str_ascii_checksum.inc
+#
+# MDEV-10850 Wrong result for WHERE .. (f2=TO_BASE64('test') OR f2=TO_BASE64('TEST'))
+#
+CREATE TABLE t1 (f1 VARCHAR(4), f2 VARCHAR(255), UNIQUE KEY k1 (f1,f2));
+INSERT INTO t1 VALUES ('test',old_password('test')), ('TEST', old_password('TEST'));
+SELECT * FROM t1 IGNORE INDEX(k1) WHERE f1='test' AND (f2= old_password("test") OR f2= old_password("TEST"));
+f1	f2
+test	378b243e220ca493
+TEST	06df397e084be793
+SELECT * FROM t1                  WHERE f1='test' AND (f2= old_password("test") OR f2= old_password("TEST"));
+f1	f2
+TEST	06df397e084be793
+test	378b243e220ca493
+SELECT * FROM t1                  WHERE f1='test' AND (f2= old_password("TEST") OR f2= old_password("test"));
+f1	f2
+TEST	06df397e084be793
+test	378b243e220ca493
+DROP TABLE t1;
+#
+# MDEV-10425 Assertion `collation.derivation == DERIVATION_IMPLICIT' failed in Item_func_conv_charset::fix_length_and_dec()
+#
+PREPARE stmt FROM "SELECT old_password(CONVERT('foo' USING latin1))";
+EXECUTE stmt;
+old_password(CONVERT('foo' USING latin1))
+7c786c222596437b
+DEALLOCATE PREPARE stmt;
+# End of func_str_ascii_checksum.inc
+#
+# End of 10.1 tests
+#
diff --git a/mysql-test/r/func_digest.result b/mysql-test/r/func_digest.result
index 095b693..6821c84 100644
--- a/mysql-test/r/func_digest.result
+++ b/mysql-test/r/func_digest.result
@@ -1426,3 +1426,35 @@ Catalog	Database	Table	Table_alias	Column	Column_alias	Type	Length	Max length	Is
 def					sha2('1',224)	253	56	56	Y	0	31	8
 sha2('1',224)
 e25388fde8290dc286a6164fa2d97e551b53498dcbf7bc378eb1f178
+#
+# Start of 10.1 tests
+#
+#
+# MDEV-10850 Wrong result for WHERE .. (f2=TO_BASE64('test') OR f2=TO_BAS E64('TEST'))
+#
+CREATE TABLE t1 (f1 VARCHAR(4), f2 VARCHAR(64), UNIQUE KEY k1 (f1,f2));
+INSERT INTO t1 VALUES ('test',SHA2('test',224)), ('TEST', SHA2('TEST',224));
+SELECT * FROM t1 IGNORE INDEX(k1) WHERE f1='test' AND (f2= SHA2("test",224) OR f2= SHA2("TEST",224));
+f1	f2
+test	90a3ed9e32b2aaf4c61c410eb925426119e1a9dc53d4286ade99a809
+TEST	917ecca24f3e6ceaf52375d8083381f1f80a21e6e49fbadc40afeb8e
+SELECT * FROM t1                  WHERE f1='test' AND (f2= SHA2("test",224) OR f2= SHA2("TEST",224));
+f1	f2
+test	90a3ed9e32b2aaf4c61c410eb925426119e1a9dc53d4286ade99a809
+TEST	917ecca24f3e6ceaf52375d8083381f1f80a21e6e49fbadc40afeb8e
+SELECT * FROM t1                  WHERE f1='test' AND (f2= SHA2("TEST",224) OR f2= SHA2("test",224));
+f1	f2
+test	90a3ed9e32b2aaf4c61c410eb925426119e1a9dc53d4286ade99a809
+TEST	917ecca24f3e6ceaf52375d8083381f1f80a21e6e49fbadc40afeb8e
+DROP TABLE t1;
+#
+# MDEV-10425 Assertion `collation.derivation == DERIVATION_IMPLICIT' failed in Item_func_conv_charset::fix_length_and_dec()
+#
+PREPARE stmt FROM "SELECT SHA2(CONVERT('foo' USING latin1), 224)";
+EXECUTE stmt;
+SHA2(CONVERT('foo' USING latin1), 224)
+0808f64e60d58979fcb676c96ec938270dea42445aeefcd3a4e6f8db
+DEALLOCATE PREPARE stmt;
+#
+# End of 10.1 tests
+#
diff --git a/mysql-test/r/func_str.result b/mysql-test/r/func_str.result
index f196571..04c6a2b 100644
--- a/mysql-test/r/func_str.result
+++ b/mysql-test/r/func_str.result
@@ -4581,6 +4581,62 @@ SELECT COUNT(*) FROM t1, t1 t2 GROUP BY INSERT('', t2.a, t1.a, @@global.max_binl
 COUNT(*)
 25
 DROP TABLE t1;
+# Start of func_str_ascii_checksum.inc
+#
+# MDEV-10850 Wrong result for WHERE .. (f2=TO_BASE64('test') OR f2=TO_BASE64('TEST'))
+#
+CREATE TABLE t1 (f1 VARCHAR(4), f2 VARCHAR(255), UNIQUE KEY k1 (f1,f2));
+INSERT INTO t1 VALUES ('test',hex('test')), ('TEST', hex('TEST'));
+SELECT * FROM t1 IGNORE INDEX(k1) WHERE f1='test' AND (f2= hex("test") OR f2= hex("TEST"));
+f1	f2
+test	74657374
+TEST	54455354
+SELECT * FROM t1                  WHERE f1='test' AND (f2= hex("test") OR f2= hex("TEST"));
+f1	f2
+TEST	54455354
+test	74657374
+SELECT * FROM t1                  WHERE f1='test' AND (f2= hex("TEST") OR f2= hex("test"));
+f1	f2
+TEST	54455354
+test	74657374
+DROP TABLE t1;
+#
+# MDEV-10425 Assertion `collation.derivation == DERIVATION_IMPLICIT' failed in Item_func_conv_charset::fix_length_and_dec()
+#
+PREPARE stmt FROM "SELECT hex(CONVERT('foo' USING latin1))";
+EXECUTE stmt;
+hex(CONVERT('foo' USING latin1))
+666F6F
+DEALLOCATE PREPARE stmt;
+# End of func_str_ascii_checksum.inc
+# Start of func_str_ascii_checksum.inc
+#
+# MDEV-10850 Wrong result for WHERE .. (f2=TO_BASE64('test') OR f2=TO_BASE64('TEST'))
+#
+CREATE TABLE t1 (f1 VARCHAR(4), f2 VARCHAR(255), UNIQUE KEY k1 (f1,f2));
+INSERT INTO t1 VALUES ('test',to_base64('test')), ('TEST', to_base64('TEST'));
+SELECT * FROM t1 IGNORE INDEX(k1) WHERE f1='test' AND (f2= to_base64("test") OR f2= to_base64("TEST"));
+f1	f2
+test	dGVzdA==
+TEST	VEVTVA==
+SELECT * FROM t1                  WHERE f1='test' AND (f2= to_base64("test") OR f2= to_base64("TEST"));
+f1	f2
+test	dGVzdA==
+TEST	VEVTVA==
+SELECT * FROM t1                  WHERE f1='test' AND (f2= to_base64("TEST") OR f2= to_base64("test"));
+f1	f2
+test	dGVzdA==
+TEST	VEVTVA==
+DROP TABLE t1;
+#
+# MDEV-10425 Assertion `collation.derivation == DERIVATION_IMPLICIT' failed in Item_func_conv_charset::fix_length_and_dec()
+#
+PREPARE stmt FROM "SELECT to_base64(CONVERT('foo' USING latin1))";
+EXECUTE stmt;
+to_base64(CONVERT('foo' USING latin1))
+Zm9v
+DEALLOCATE PREPARE stmt;
+# End of func_str_ascii_checksum.inc
 #
 # End of 10.1 tests
 #
diff --git a/mysql-test/t/func_crypt.test b/mysql-test/t/func_crypt.test
index ca6e712..1504051 100644
--- a/mysql-test/t/func_crypt.test
+++ b/mysql-test/t/func_crypt.test
@@ -70,3 +70,17 @@ SELECT OLD_PASSWORD(c1), PASSWORD(c1) FROM t1;
 DROP TABLE t1;
 
 --echo End of 5.0 tests
+
+
+--echo #
+--echo # Start of 10.1 tests
+--echo #
+
+--let func=password
+--source include/func_str_ascii_checksum.inc
+--let func=old_password
+--source include/func_str_ascii_checksum.inc
+
+--echo #
+--echo # End of 10.1 tests
+--echo #
diff --git a/mysql-test/t/func_digest.test b/mysql-test/t/func_digest.test
index 81f19c7..39edb64 100644
--- a/mysql-test/t/func_digest.test
+++ b/mysql-test/t/func_digest.test
@@ -494,3 +494,29 @@ SET NAMES latin1;
 SELECT sha2('1',224);
 --disable_metadata
 
+--echo #
+--echo # Start of 10.1 tests
+--echo #
+
+--echo #
+--echo # MDEV-10850 Wrong result for WHERE .. (f2=TO_BASE64('test') OR f2=TO_BAS E64('TEST'))
+--echo #
+
+CREATE TABLE t1 (f1 VARCHAR(4), f2 VARCHAR(64), UNIQUE KEY k1 (f1,f2));
+INSERT INTO t1 VALUES ('test',SHA2('test',224)), ('TEST', SHA2('TEST',224));
+SELECT * FROM t1 IGNORE INDEX(k1) WHERE f1='test' AND (f2= SHA2("test",224) OR f2= SHA2("TEST",224));
+SELECT * FROM t1                  WHERE f1='test' AND (f2= SHA2("test",224) OR f2= SHA2("TEST",224));
+SELECT * FROM t1                  WHERE f1='test' AND (f2= SHA2("TEST",224) OR f2= SHA2("test",224));
+DROP TABLE t1;
+
+--echo #
+--echo # MDEV-10425 Assertion `collation.derivation == DERIVATION_IMPLICIT' failed in Item_func_conv_charset::fix_length_and_dec()
+--echo #
+
+PREPARE stmt FROM "SELECT SHA2(CONVERT('foo' USING latin1), 224)";
+EXECUTE stmt;
+DEALLOCATE PREPARE stmt;
+
+--echo #
+--echo # End of 10.1 tests
+--echo #
diff --git a/mysql-test/t/func_str.test b/mysql-test/t/func_str.test
index 2645417..98af370 100644
--- a/mysql-test/t/func_str.test
+++ b/mysql-test/t/func_str.test
@@ -1783,6 +1783,12 @@ INSERT INTO t1 VALUES (0),(0),(1),(0),(0);
 SELECT COUNT(*) FROM t1, t1 t2 GROUP BY INSERT('', t2.a, t1.a, @@global.max_binlog_size);
 DROP TABLE t1;
 
+--let func=hex
+--source include/func_str_ascii_checksum.inc
+
+--let func=to_base64
+--source include/func_str_ascii_checksum.inc
+
 --echo #
 --echo # End of 10.1 tests
 --echo #
diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc
index 75b034c..43770a8 100644
--- a/sql/item_strfunc.cc
+++ b/sql/item_strfunc.cc
@@ -165,31 +165,6 @@ String *Item_func_md5::val_str_ascii(String *str)
 }
 
 
-/*
-  The MD5()/SHA() functions treat their parameter as being a case sensitive.
-  Thus we set binary collation on it so different instances of MD5() will be
-  compared properly.
-*/
-static CHARSET_INFO *get_checksum_charset(const char *csname)
-{
-  CHARSET_INFO *cs= get_charset_by_csname(csname, MY_CS_BINSORT, MYF(0));
-  if (!cs)
-  {
-    // Charset has no binary collation: use my_charset_bin.
-    cs= &my_charset_bin;
-  }
-  return cs;
-}
-
-
-void Item_func_md5::fix_length_and_dec()
-{
-  CHARSET_INFO *cs= get_checksum_charset(args[0]->collation.collation->csname);
-  args[0]->collation.set(cs, DERIVATION_COERCIBLE);
-  fix_length_and_charset(32, default_charset());
-}
-
-
 String *Item_func_sha::val_str_ascii(String *str)
 {
   DBUG_ASSERT(fixed == 1);
@@ -215,8 +190,6 @@ String *Item_func_sha::val_str_ascii(String *str)
 
 void Item_func_sha::fix_length_and_dec()
 {
-  CHARSET_INFO *cs= get_checksum_charset(args[0]->collation.collation->csname);
-  args[0]->collation.set(cs, DERIVATION_COERCIBLE);
   // size of hex representation of hash
   fix_length_and_charset(SHA1_HASH_SIZE * 2, default_charset());
 }
@@ -346,9 +319,6 @@ void Item_func_sha2::fix_length_and_dec()
                         "sha2");
   }
 
-  CHARSET_INFO *cs= get_checksum_charset(args[0]->collation.collation->csname);
-  args[0]->collation.set(cs, DERIVATION_COERCIBLE);
-
 #else
   THD *thd= current_thd;
   push_warning_printf(thd,
diff --git a/sql/item_strfunc.h b/sql/item_strfunc.h
index 65c2868..db0509e 100644
--- a/sql/item_strfunc.h
+++ b/sql/item_strfunc.h
@@ -96,40 +96,63 @@ class Item_str_ascii_func :public Item_str_func
 };
 
 
-class Item_func_md5 :public Item_str_ascii_func
+/**
+  Functions returning a checksum or a hash of the argument.
+*/
+class Item_str_ascii_checksum_func: public Item_str_ascii_func
+{
+public:
+  Item_str_ascii_checksum_func(THD *thd, Item *a)
+   :Item_str_ascii_func(thd, a) { }
+  Item_str_ascii_checksum_func(THD *thd, Item *a, Item *b)
+   :Item_str_ascii_func(thd, a, b) { }
+  bool eq(const Item *item, bool binary_cmp) const
+  {
+    // Always use binary argument comparison: MD5('x') != MD5('X')
+    return Item_func::eq(item, true);
+  }
+};
+
+
+class Item_func_md5 :public Item_str_ascii_checksum_func
 {
   String tmp_value;
 public:
-  Item_func_md5(THD *thd, Item *a): Item_str_ascii_func(thd, a) {}
+  Item_func_md5(THD *thd, Item *a): Item_str_ascii_checksum_func(thd, a) {}
   String *val_str_ascii(String *);
-  void fix_length_and_dec();
+  void fix_length_and_dec()
+  {
+    fix_length_and_charset(32, default_charset());
+  }
   const char *func_name() const { return "md5"; }
 };
 
 
-class Item_func_sha :public Item_str_ascii_func
+class Item_func_sha :public Item_str_ascii_checksum_func
 {
 public:
-  Item_func_sha(THD *thd, Item *a): Item_str_ascii_func(thd, a) {}
+  Item_func_sha(THD *thd, Item *a): Item_str_ascii_checksum_func(thd, a) {}
   String *val_str_ascii(String *);    
   void fix_length_and_dec();      
   const char *func_name() const { return "sha"; }	
 };
 
-class Item_func_sha2 :public Item_str_ascii_func
+class Item_func_sha2 :public Item_str_ascii_checksum_func
 {
 public:
-  Item_func_sha2(THD *thd, Item *a, Item *b): Item_str_ascii_func(thd, a, b) {}
+  Item_func_sha2(THD *thd, Item *a, Item *b)
+   :Item_str_ascii_checksum_func(thd, a, b) {}
   String *val_str_ascii(String *);
   void fix_length_and_dec();
   const char *func_name() const { return "sha2"; }
 };
 
-class Item_func_to_base64 :public Item_str_ascii_func
+class Item_func_to_base64 :public Item_str_ascii_checksum_func
 {
   String tmp_value;
 public:
-  Item_func_to_base64(THD *thd, Item *a): Item_str_ascii_func(thd, a) {}
+  Item_func_to_base64(THD *thd, Item *a)
+   :Item_str_ascii_checksum_func(thd, a) {}
   String *val_str_ascii(String *);
   void fix_length_and_dec();
   const char *func_name() const { return "to_base64"; }
@@ -431,7 +454,7 @@ class Item_func_rtrim :public Item_func_trim
   authentication procedure works, see comments in password.c.
 */
 
-class Item_func_password :public Item_str_ascii_func
+class Item_func_password :public Item_str_ascii_checksum_func
 {
 public:
   enum PW_Alg {OLD, NEW};
@@ -441,9 +464,9 @@ class Item_func_password :public Item_str_ascii_func
   bool deflt;
 public:
   Item_func_password(THD *thd, Item *a):
-    Item_str_ascii_func(thd, a), alg(NEW), deflt(1) {}
+    Item_str_ascii_checksum_func(thd, a), alg(NEW), deflt(1) {}
   Item_func_password(THD *thd, Item *a, PW_Alg al):
-    Item_str_ascii_func(thd, a), alg(al), deflt(0) {}
+    Item_str_ascii_checksum_func(thd, a), alg(al), deflt(0) {}
   String *val_str_ascii(String *str);
   bool fix_fields(THD *thd, Item **ref);
   void fix_length_and_dec()
@@ -803,12 +826,12 @@ class Item_func_conv :public Item_str_func
 };
 
 
-class Item_func_hex :public Item_str_ascii_func
+class Item_func_hex :public Item_str_ascii_checksum_func
 {
   String tmp_value;
 public:
   Item_func_hex(THD *thd, Item *a):
-    Item_str_ascii_func(thd, a) {}
+    Item_str_ascii_checksum_func(thd, a) {}
   const char *func_name() const { return "hex"; }
   String *val_str_ascii(String *);
   void fix_length_and_dec()

Follow ups