From 766b4a8c7ff930d8d1b3a9b361c40984b5469b06 Mon Sep 17 00:00:00 2001 From: "evgen@moonbone.local" <> Date: Sun, 28 May 2006 22:01:38 +0400 Subject: [PATCH 1/3] Fixed bug#15351: Wrong collation used for comparison of md5() and sha() argument can lead to a wrong result. md5() and sha() functions treat their arguments as case sensitive strings. But when they are compared their arguments were compared as a case insensitive strings which leads to two functions with different arguments and thus different results to being identical. This can lead to a wrong decision made in the range optimizer and thus lead to a wrong result set. Item_func_md5::fix_length_and_dec() and Item_func_sha::fix_length_and_dec() functions now set binary collation on their arguments. --- mysql-test/r/func_str.result | 15 +++++++++++++++ mysql-test/t/func_str.test | 12 ++++++++++++ sql/item_strfunc.cc | 20 ++++++++++++++++++-- 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/mysql-test/r/func_str.result b/mysql-test/r/func_str.result index 0609624af18..24e6bb6f38a 100644 --- a/mysql-test/r/func_str.result +++ b/mysql-test/r/func_str.result @@ -1006,4 +1006,19 @@ NULL select ifnull(load_file("lkjlkj"),"it's null"); ifnull(load_file("lkjlkj"),"it's null") it's null +create table t1 (f1 varchar(4), f2 varchar(64), unique key k1 (f1,f2)); +insert into t1 values ( 'test',md5('test')), ('test', sha('test')); +select * from t1 where f1='test' and (f2= md5("test") or f2= md5("TEST")); +f1 f2 +test 098f6bcd4621d373cade4e832627b4f6 +select * from t1 where f1='test' and (f2= md5("TEST") or f2= md5("test")); +f1 f2 +test 098f6bcd4621d373cade4e832627b4f6 +select * from t1 where f1='test' and (f2= sha("test") or f2= sha("TEST")); +f1 f2 +test a94a8fe5ccb19ba61c4c0873d391e987982fbbd3 +select * from t1 where f1='test' and (f2= sha("TEST") or f2= sha("test")); +f1 f2 +test a94a8fe5ccb19ba61c4c0873d391e987982fbbd3 +drop table t1; End of 4.1 tests diff --git a/mysql-test/t/func_str.test b/mysql-test/t/func_str.test index c2f76dbac43..c36e15a08b9 100644 --- a/mysql-test/t/func_str.test +++ b/mysql-test/t/func_str.test @@ -669,4 +669,16 @@ drop table t1; select load_file("lkjlkj"); select ifnull(load_file("lkjlkj"),"it's null"); +# +# Bug#15351: Wrong collation used for comparison of md5() and sha() +# parameter can lead to a wrong result. +# +create table t1 (f1 varchar(4), f2 varchar(64), unique key k1 (f1,f2)); +insert into t1 values ( 'test',md5('test')), ('test', sha('test')); +select * from t1 where f1='test' and (f2= md5("test") or f2= md5("TEST")); +select * from t1 where f1='test' and (f2= md5("TEST") or f2= md5("test")); +select * from t1 where f1='test' and (f2= sha("test") or f2= sha("TEST")); +select * from t1 where f1='test' and (f2= sha("TEST") or f2= sha("test")); +drop table t1; + --echo End of 4.1 tests diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc index e74d0100b55..e817edac6c0 100644 --- a/sql/item_strfunc.cc +++ b/sql/item_strfunc.cc @@ -118,7 +118,15 @@ String *Item_func_md5::val_str(String *str) void Item_func_md5::fix_length_and_dec() { - max_length=32; + max_length=32; + /* + The MD5() function treats its parameter as being a case sensitive. Thus + we set binary collation on it so different instances of MD5() will be + compared properly. + */ + args[0]->collation.set( + get_charset_by_csname(args[0]->collation.collation->csname, + MY_CS_BINSORT,MYF(0)), DERIVATION_COERCIBLE); } @@ -159,7 +167,15 @@ String *Item_func_sha::val_str(String *str) void Item_func_sha::fix_length_and_dec() { - max_length=SHA1_HASH_SIZE*2; // size of hex representation of hash + max_length=SHA1_HASH_SIZE*2; // size of hex representation of hash + /* + The SHA() function treats its parameter as being a case sensitive. Thus + we set binary collation on it so different instances of MD5() will be + compared properly. + */ + args[0]->collation.set( + get_charset_by_csname(args[0]->collation.collation->csname, + MY_CS_BINSORT,MYF(0)), DERIVATION_COERCIBLE); } From 2877b5ef64ed40ccfdaed3f743753d4348a7e814 Mon Sep 17 00:00:00 2001 From: "evgen@moonbone.local" <> Date: Thu, 15 Jun 2006 16:39:18 +0400 Subject: [PATCH 2/3] item_cmpfunc.h, cast.result: Post fix for bug#16377 --- mysql-test/r/cast.result | 2 +- sql/item_cmpfunc.h | 15 +++++---------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/mysql-test/r/cast.result b/mysql-test/r/cast.result index 2538fbfd61d..68687670e17 100644 --- a/mysql-test/r/cast.result +++ b/mysql-test/r/cast.result @@ -192,7 +192,7 @@ cast("2001-1-1" as datetime) = "2001-01-01 00:00:00" 1 select cast("1:2:3" as TIME) = "1:02:03"; cast("1:2:3" as TIME) = "1:02:03" -1 +0 select cast(NULL as DATE); cast(NULL as DATE) NULL diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 08d9de6ffd6..68852b5a5f6 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -43,11 +43,8 @@ public: int set_compare_func(Item_bool_func2 *owner, Item_result type); inline int set_compare_func(Item_bool_func2 *owner_arg) { - Item_result ar= (*a)->result_as_longlong() && (*b)->const_item() ? - INT_RESULT : (*a)->result_type(); - Item_result br= (*b)->result_as_longlong() && (*a)->const_item() ? - INT_RESULT : (*b)->result_type(); - return set_compare_func(owner_arg, item_cmp_type(ar, br)); + return set_compare_func(owner_arg, item_cmp_type((*a)->result_type(), + (*b)->result_type())); } inline int set_cmp_func(Item_bool_func2 *owner_arg, Item **a1, Item **a2, @@ -60,11 +57,9 @@ public: inline int set_cmp_func(Item_bool_func2 *owner_arg, Item **a1, Item **a2) { - Item_result ar= (*a1)->result_as_longlong() && (*a2)->const_item() ? - INT_RESULT : (*a1)->result_type(); - Item_result br= (*a2)->result_as_longlong() && (*a1)->const_item() ? - INT_RESULT : (*a2)->result_type(); - return set_cmp_func(owner_arg, a1, a2, item_cmp_type(ar, br)); + return set_cmp_func(owner_arg, a1, a2, + item_cmp_type((*a1)->result_type(), + (*a2)->result_type())); } inline int compare() { return (this->*func)(); } From b8b9738e0baabc5c299eb0363b9f1d61d2985021 Mon Sep 17 00:00:00 2001 From: "evgen@moonbone.local" <> Date: Fri, 16 Jun 2006 23:46:37 +0400 Subject: [PATCH 3/3] item_strfunc.cc: Fix for bug#16716 for --ps-protocol mode. item_cmpfunc.cc: Fix for a memory allocation/freeing problem in agg_cmp_type() after fix for bug#16377. Few language corrections. --- sql/item_cmpfunc.cc | 32 +++++++++++--------------------- sql/item_strfunc.cc | 3 ++- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index e92e1d30ca4..126037a24d8 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -83,21 +83,21 @@ static void agg_result_type(Item_result *type, Item **items, uint nitems) items will be used for aggregation. If there are DATE/TIME fields/functions in the list and no string fields/functions in the list then: - The INT_RESULT type will be used for aggregation instead of orginal + The INT_RESULT type will be used for aggregation instead of original result type of any DATE/TIME field/function in the list All constant items in the list will be converted to a DATE/TIME using found field or result field of found function. Implementation notes: - The code is equvalent to: - 1. Check the list for presense of a STRING field/function. + The code is equivalent to: + 1. Check the list for presence of a STRING field/function. Collect the is_const flag. 2. Get a Field* object to use for type coercion 3. Perform type conversion. 1 and 2 are implemented in 2 loops. The first searches for a DATE/TIME - field/function and checks presense of a STRING field/function. + field/function and checks presence of a STRING field/function. The second loop works only if a DATE/TIME field/function is found. - It checks presense of a STRING field/function in the rest of the list. + It checks presence of a STRING field/function in the rest of the list. TODO 1) The current implementation can produce false comparison results for @@ -120,8 +120,9 @@ static void agg_result_type(Item_result *type, Item **items, uint nitems) static void agg_cmp_type(THD *thd, Item_result *type, Item **items, uint nitems) { uint i; - Item::Type res; - char *buff= NULL; + Item::Type res= (Item::Type)0; + /* Used only for date/time fields, max_length = 19 */ + char buff[20]; uchar null_byte; Field *field= NULL; @@ -147,28 +148,20 @@ static void agg_cmp_type(THD *thd, Item_result *type, Item **items, uint nitems) { field= items[i]->tmp_table_field_from_field_type(0); if (field) - buff= alloc_root(thd->mem_root, field->max_length()); - if (!buff || !field) - { - if (field) - delete field; - if (buff) - my_free(buff, MYF(MY_WME)); - field= 0; - } - else field->move_field(buff, &null_byte, 0); break; } } if (field) { - /* Check the rest of the list for presense of a string field/function. */ + /* Check the rest of the list for presence of a string field/function. */ for (i++ ; i < nitems; i++) { if (!items[i]->const_item() && items[i]->result_type() == STRING_RESULT && !items[i]->result_as_longlong()) { + if (res == Item::FUNC_ITEM) + delete field; field= 0; break; } @@ -205,10 +198,7 @@ static void agg_cmp_type(THD *thd, Item_result *type, Item **items, uint nitems) } if (res == Item::FUNC_ITEM && field) - { delete field; - my_free(buff, MYF(MY_WME)); - } } static void my_coll_agg_error(DTCollation &c1, DTCollation &c2, diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc index deb3542f4a5..ee585649f8c 100644 --- a/sql/item_strfunc.cc +++ b/sql/item_strfunc.cc @@ -274,7 +274,8 @@ String *Item_func_concat::val_str(String *str) if (!(res=args[0]->val_str(str))) goto null; use_as_buff= &tmp_value; - is_const= args[0]->const_item(); + /* Item_subselect in --ps-protocol mode will state it as a non-const */ + is_const= args[0]->const_item() || !args[0]->used_tables(); for (i=1 ; i < arg_count ; i++) { if (res->length() == 0)