5216 Dmitry Lenev 2012-12-11 [merge]
Merged fix for Bug #15954872 "MAKE MDL SUBSYSTEM AND TABLE
DEFINITION CACHE ROBUST AGAINST BUGS IN CALLERS" into trunk.
modified:
sql/mdl.h
sql/sql_base.cc
sql/sql_cache.cc
sql/sql_cache.h
sql/table.h
unittest/gunit/mdl-t.cc
5215 Gleb Shchepa 2012-12-11 [merge]
Auto up-merge (15892875)
modified:
mysql-test/r/innodb_explain_json_non_select_all.result
mysql-test/r/innodb_explain_json_non_select_none.result
mysql-test/r/innodb_explain_non_select_all.result
mysql-test/r/innodb_explain_non_select_none.result
mysql-test/r/myisam_explain_json_non_select_all.result
mysql-test/r/myisam_explain_json_non_select_none.result
mysql-test/r/myisam_explain_non_select_all.result
mysql-test/r/myisam_explain_non_select_none.result
mysql-test/r/partition_explicit_prune.result
mysql-test/r/partition_locking.result
sql/opt_explain.cc
=== modified file 'sql/mdl.h'
--- a/sql/mdl.h 2012-12-05 17:06:53 +0000
+++ b/sql/mdl.h 2012-12-11 18:12:13 +0000
@@ -335,8 +335,16 @@ public:
const char *db, const char *name)
{
m_ptr[0]= (char) mdl_namespace;
- m_db_name_length= (uint16) (strmov(m_ptr + 1, db) - m_ptr - 1);
- m_length= (uint16) (strmov(m_ptr + m_db_name_length + 2, name) - m_ptr + 1);
+ /*
+ It is responsibility of caller to ensure that db and object names
+ are not longer than NAME_LEN. Still we play safe and try to avoid
+ buffer overruns.
+ */
+ DBUG_ASSERT(strlen(db) <= NAME_LEN && strlen(name) <= NAME_LEN);
+ m_db_name_length= static_cast<uint16>(strmake(m_ptr + 1, db, NAME_LEN) -
+ m_ptr - 1);
+ m_length= static_cast<uint16>(strmake(m_ptr + m_db_name_length + 2, name,
+ NAME_LEN) - m_ptr + 1);
}
void mdl_key_init(const MDL_key *rhs)
{
=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc 2012-12-11 11:58:31 +0000
+++ b/sql/sql_base.cc 2012-12-11 18:12:13 +0000
@@ -271,8 +271,16 @@ static uint create_table_def_key(THD *th
const char *db_name, const char *table_name,
bool tmp_table)
{
- uint key_length= (uint) (strmov(strmov(key, db_name) + 1, table_name) -
- key) + 1;
+ /*
+ In theory caller should ensure that both db and table_name are
+ not longer than NAME_LEN bytes. In practice we play safe to avoid
+ buffer overruns.
+ */
+ DBUG_ASSERT(strlen(db_name) <= NAME_LEN && strlen(table_name) <= NAME_LEN);
+ uint key_length= static_cast<uint>(strmake(strmake(key, db_name, NAME_LEN) +
+ 1, table_name, NAME_LEN) - key +
+ 1);
+
if (tmp_table)
{
int4store(key + key_length, thd->server_id);
@@ -310,8 +318,10 @@ uint get_table_def_key(const TABLE_LIST
is properly initialized, so table definition cache can be produced
from key used by MDL subsystem.
*/
- DBUG_ASSERT(!strcmp(table_list->db, table_list->mdl_request.key.db_name()) &&
- !strcmp(table_list->table_name, table_list->mdl_request.key.name()));
+ DBUG_ASSERT(!strcmp(table_list->get_db_name(),
+ table_list->mdl_request.key.db_name()) &&
+ !strcmp(table_list->get_table_name(),
+ table_list->mdl_request.key.name()));
*key= (const char*)table_list->mdl_request.key.ptr() + 1;
return table_list->mdl_request.key.length() - 1;
@@ -3090,7 +3100,8 @@ err_unlock:
TABLE *find_locked_table(TABLE *list, const char *db, const char *table_name)
{
char key[MAX_DBKEY_LENGTH];
- uint key_length=(uint) (strmov(strmov(key,db)+1,table_name)-key)+1;
+ uint key_length= create_table_def_key((THD*)NULL, key, db, table_name,
+ false);
for (TABLE *table= list; table ; table=table->next)
{
@@ -9337,7 +9348,7 @@ void tdc_remove_table(THD *thd, enum_tdc
thd->mdl_context.is_lock_owner(MDL_key::TABLE, db, table_name,
MDL_EXCLUSIVE));
- key_length=(uint) (strmov(strmov(key,db)+1,table_name)-key)+1;
+ key_length= create_table_def_key(thd, key, db, table_name, false);
if ((share= (TABLE_SHARE*) my_hash_search(&table_def_cache,(uchar*) key,
key_length)))
=== modified file 'sql/sql_cache.cc'
--- a/sql/sql_cache.cc 2012-11-21 12:44:48 +0000
+++ b/sql/sql_cache.cc 2012-12-11 18:12:13 +0000
@@ -2824,11 +2824,9 @@ void Query_cache::invalidate_table(THD *
invalidate_table(thd, table_list->table); // Table is open
else
{
- char key[MAX_DBKEY_LENGTH];
+ const char *key;
uint key_length;
-
- key_length=(uint) (strmov(strmov(key,table_list->db)+1,
- table_list->table_name) -key)+ 1;
+ key_length= get_table_def_key(table_list, &key);
// We don't store temporary tables => no key_length+=4 ...
invalidate_table(thd, (uchar *)key, key_length);
@@ -2940,13 +2938,12 @@ Query_cache::register_tables_from_list(T
block_table->n= n;
if (tables_used->view)
{
- char key[MAX_DBKEY_LENGTH];
+ const char *key;
uint key_length;
DBUG_PRINT("qcache", ("view: %s db: %s",
tables_used->view_name.str,
tables_used->view_db.str));
- key_length= (uint) (strmov(strmov(key, tables_used->view_db.str) + 1,
- tables_used->view_name.str) - key) + 1;
+ key_length= get_table_def_key(tables_used, &key);
/*
There are not callback function for for VIEWs
*/
@@ -3056,7 +3053,7 @@ my_bool Query_cache::register_all_tables
*/
my_bool
-Query_cache::insert_table(uint key_len, char *key,
+Query_cache::insert_table(uint key_len, const char *key,
Query_cache_block_table *node,
uint32 db_length, uint8 cache_type,
qc_engine_callback callback,
@@ -4205,8 +4202,10 @@ uint Query_cache::filename_2_table_key (
*db_length= (filename - dbname) - 1;
DBUG_PRINT("qcache", ("table '%-.*s.%s'", *db_length, dbname, filename));
- DBUG_RETURN((uint) (strmov(strmake(key, dbname, *db_length) + 1,
- filename) -key) + 1);
+ DBUG_RETURN(static_cast<uint>(strmake(strmake(key, dbname,
+ min<uint32>(*db_length,
+ NAME_LEN)) + 1,
+ filename, NAME_LEN) - key) + 1);
}
/****************************************************************************
=== modified file 'sql/sql_cache.h'
--- a/sql/sql_cache.h 2012-05-31 23:23:37 +0000
+++ b/sql/sql_cache.h 2012-12-11 18:07:34 +0000
@@ -370,7 +370,7 @@ protected:
my_bool register_all_tables(Query_cache_block *block,
TABLE_LIST *tables_used,
TABLE_COUNTER_TYPE tables);
- my_bool insert_table(uint key_len, char *key,
+ my_bool insert_table(uint key_len, const char *key,
Query_cache_block_table *node,
uint32 db_length, uint8 cache_type,
qc_engine_callback callback,
=== modified file 'sql/table.h'
--- a/sql/table.h 2012-12-11 11:58:31 +0000
+++ b/sql/table.h 2012-12-11 18:12:13 +0000
@@ -1992,7 +1992,7 @@ public:
@brief Returns the name of the database that the referenced table belongs
to.
*/
- char *get_db_name() { return view != NULL ? view_db.str : db; }
+ char *get_db_name() const { return view != NULL ? view_db.str : db; }
/**
@brief Returns the name of the table that this TABLE_LIST represents.
@@ -2000,7 +2000,7 @@ public:
@details The unqualified table name or view name for a table or view,
respectively.
*/
- char *get_table_name() { return view != NULL ? view_name.str : table_name; }
+ char *get_table_name() const { return view != NULL ? view_name.str : table_name; }
int fetch_number_of_rows();
bool update_derived_keys(Field*, Item**, uint);
bool generate_keys();
=== modified file 'unittest/gunit/mdl-t.cc'
--- a/unittest/gunit/mdl-t.cc 2012-12-04 12:57:18 +0000
+++ b/unittest/gunit/mdl-t.cc 2012-12-11 18:12:13 +0000
@@ -1612,4 +1612,83 @@ TEST_F(MDLTest, HogLockTest5)
max_write_lock_count= org_max_write_lock_count;
}
+
+/** Test class for MDL_key class testing. Doesn't require MDL initialization. */
+
+class MDLKeyTest : public ::testing::Test
+{
+protected:
+ MDLKeyTest()
+ { }
+private:
+ GTEST_DISALLOW_COPY_AND_ASSIGN_(MDLKeyTest);
+};
+
+
+// Google Test recommends DeathTest suffix for classes use in death tests.
+typedef MDLKeyTest MDLKeyDeathTest;
+
+
+/*
+ Verifies that debug build dies with a DBUG_ASSERT if we try to construct
+ MDL_key with too long database or object names.
+*/
+
+#if GTEST_HAS_DEATH_TEST && !defined(DBUG_OFF)
+TEST_F(MDLKeyDeathTest, DieWhenNamesAreTooLong)
+{
+ ::testing::FLAGS_gtest_death_test_style = "threadsafe";
+
+ /* We need a name which is longer than NAME_LEN = 64*3 = 192.*/
+ const char *too_long_name=
+ "0123456789012345678901234567890123456789012345678901234567890123"
+ "0123456789012345678901234567890123456789012345678901234567890123"
+ "0123456789012345678901234567890123456789012345678901234567890123"
+ "0123456789";
+
+ EXPECT_DEATH(MDL_key key0(MDL_key::TABLE, too_long_name, ""),
+ ".*Assertion.*strlen.*failed.*");
+ EXPECT_DEATH(MDL_key key1(MDL_key::TABLE, "", too_long_name),
+ ".*Assertion.*strlen.*failed.*");
+
+ MDL_key key2;
+
+ EXPECT_DEATH(key2.mdl_key_init(MDL_key::TABLE, too_long_name, ""),
+ ".*Assertion.*strlen.*failed.*");
+ EXPECT_DEATH(key2.mdl_key_init(MDL_key::TABLE, "", too_long_name),
+ ".*Assertion.*strlen.*failed.*");
+
+}
+#endif // GTEST_HAS_DEATH_TEST && !defined(DBUG_OFF)
+
+
+/*
+ Verifies that for production build we allow construction of
+ MDL_key with too long database or object names, but they are
+ truncated.
+*/
+
+#if defined(DBUG_OFF)
+TEST_F(MDLKeyTest, TruncateTooLongNames)
+{
+ /* We need a name which is longer than NAME_LEN = 64*3 = 192.*/
+ const char *too_long_name=
+ "0123456789012345678901234567890123456789012345678901234567890123"
+ "0123456789012345678901234567890123456789012345678901234567890123"
+ "0123456789012345678901234567890123456789012345678901234567890123"
+ "0123456789";
+
+ MDL_key key(MDL_key::TABLE, too_long_name, too_long_name);
+
+ const char *db_name= key.db_name();
+ const char *name= key.name();
+
+ EXPECT_LE(strlen(db_name), (uint)NAME_LEN);
+ EXPECT_TRUE(strncmp(db_name, too_long_name, NAME_LEN) == 0);
+ EXPECT_LE(strlen(name), (uint)NAME_LEN);
+ EXPECT_TRUE(strncmp(name, too_long_name, NAME_LEN) == 0);
+}
+#endif // defined(DBUG_OFF)
+
+
} // namespace
No bundle (reason: useless for push emails).
| Thread |
|---|
| • bzr push into mysql-trunk branch (Dmitry.Lenev:5215 to 5216) Bug#15954872 | Dmitry Lenev | 12 Dec |
