Skip to content

Commit

Permalink
Better handle bad db_metadata value
Browse files Browse the repository at this point in the history
Summary:
There is implicit require for db_metadata: it should be a json object , such as "{"shard": "<shard_name>", "rs": "<replicaset_id>"}"

If The db_metadata value is a json array, such as "[{"shard": "<shard_name>", "rs": "<replicaset_id>"}]", rapidjson will SIGSEGV when treating a json array as a json object and accessing members in the "object".

add checks to verify the db_metadata is a json object.

Reviewed By: abhinav04sharma, zhichengzhu

Differential Revision: D16569064

fbshipit-source-id: 61c3761
  • Loading branch information
luqun authored and facebook-github-bot committed Aug 3, 2019
1 parent 85c6ed5 commit 1452928
Show file tree
Hide file tree
Showing 8 changed files with 159 additions and 7 deletions.
50 changes: 47 additions & 3 deletions mysql-test/r/db_metadata.result
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ drop database if exists test7;
drop database if exists test8;
drop database if exists test9;
drop database if exists test10;
drop database if exists test11;
drop database if exists test12;
drop database if exists test13;
drop database if exists test14;
create database test2;
show create database test2;
Database Create Database
Expand Down Expand Up @@ -157,7 +161,7 @@ def test7 {"shard":"test7_shard"}
def test8
def test9 {"shard":"test9_shard"}
create database test10 db_metadata = "invalid_json";
ERROR HY000: Invalid JSON for DB_METADATA attribute: invalid_json.
ERROR HY000: Invalid JSON object for DB_METADATA attribute: invalid_json.
select catalog_name, schema_name, db_metadata from information_schema.schemata_ext;
catalog_name schema_name db_metadata
def information_schema
Expand All @@ -174,7 +178,7 @@ def test7 {"shard":"test7_shard"}
def test8
def test9 {"shard":"test9_shard"}
create database test11 db_metadata = "{\'shard\':\'test11_shard\'}";
ERROR HY000: Invalid JSON for DB_METADATA attribute: {'shard':'test11_shard'}.
ERROR HY000: Invalid JSON object for DB_METADATA attribute: {'shard':'test11_shard'}.
select catalog_name, schema_name, db_metadata from information_schema.schemata_ext;
catalog_name schema_name db_metadata
def information_schema
Expand Down Expand Up @@ -216,6 +220,25 @@ CREATE DATABASE `test13` /*!40100 DEFAULT CHARACTER SET latin1 DB_METADATA '{"sh
show create database test13;
Database Create Database
test13 CREATE DATABASE `test13` /*!40100 DEFAULT CHARACTER SET latin1 DB_METADATA '{"sha''\\"rd":"test13''_sh\\"ard"}' */
create database test14 db_metadata = "[{\"shard\":\"test14_shard\", \"data\":\"1234\"}]";
ERROR HY000: Invalid JSON object for DB_METADATA attribute: [{"shard":"test14_shard", "data":"1234"}].
select catalog_name, schema_name, db_metadata from information_schema.schemata_ext;
catalog_name schema_name db_metadata
def information_schema
def mtr
def mysql
def performance_schema
def test
def test12 {"sha'rd":"test12\"_shard"}
def test13 {"sha'\"rd":"test13'_sh\"ard"}
def test2
def test3
def test4
def test5 {"shard":"test5_shard"}
def test6 {"shard":"test6_shard"}
def test7 {"shard":"test7_shard"}
def test8
def test9 {"shard":"test9_shard"}
alter database test3 character set ascii;
show create database test3;
Database Create Database
Expand Down Expand Up @@ -543,7 +566,7 @@ def test7 {"shard":"test7_shard_altered"}
def test8
def test9 {"shard":"test9_shard_altered"}
alter database test9 db_metadata = "invalid_json";
ERROR HY000: Invalid JSON for DB_METADATA attribute: invalid_json.
ERROR HY000: Invalid JSON object for DB_METADATA attribute: invalid_json.
select catalog_name, schema_name, db_metadata from information_schema.schemata_ext;
catalog_name schema_name db_metadata
def information_schema
Expand Down Expand Up @@ -611,6 +634,26 @@ default-character-set=latin1
default-collation=latin1_swedish_ci
db-read-only=0
db-metadata={"shard":"test10_shard_altered"}
alter database test10 db_metadata = "[{\"shard\":\"test10_shard\", \"data\":\"1234\"}]";
ERROR HY000: Invalid JSON object for DB_METADATA attribute: [{"shard":"test10_shard", "data":"1234"}].
select catalog_name, schema_name, db_metadata from information_schema.schemata_ext;
catalog_name schema_name db_metadata
def information_schema
def mtr
def mysql
def performance_schema
def test
def test10 {"shard":"test10_shard_altered"}
def test12 {"sha'rd":"test12\"_shard"}
def test13 {"sha'\"rd":"test13'_sh\"ard"}
def test2
def test3 {"shard":"test3_shard_altered"}
def test4 {"shard":"test4_shard_altered"}
def test5 {"shard":"Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Really long shard name. Fin"}
def test6 {"shard":"test6_shard_altered"}
def test7 {"shard":"test7_shard_altered"}
def test8
def test9 {"shard":"test9_shard_altered"}
drop database if exists test2;
drop database if exists test3;
drop database if exists test4;
Expand All @@ -623,3 +666,4 @@ drop database if exists test10;
drop database if exists test11;
drop database if exists test12;
drop database if exists test13;
drop database if exists test14;
43 changes: 43 additions & 0 deletions mysql-test/r/db_metadata_reset.result
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,51 @@ test4 CREATE DATABASE `test4` /*!40100 DEFAULT CHARACTER SET latin1 */
show create database test6;
Database Create Database
test6 CREATE DATABASE `test6` /*!40100 DEFAULT CHARACTER SET latin1 */
create database test7 db_metadata = "{\"shard\":\"test7_shard\"}";
show create database test7;
Database Create Database
test7 CREATE DATABASE `test7` /*!40100 DEFAULT CHARACTER SET latin1 DB_METADATA '{"shard":"test7_shard"}' */
set @start_binlog_trx_meta_data= @@global.binlog_trx_meta_data;
set @start_log_warnings = @@global.log_warnings;
set @@global.binlog_trx_meta_data= 1;
set @@global.log_warnings= 2;
select catalog_name, schema_name, default_character_set_name, default_collation_name, sql_path from information_schema.schemata;
catalog_name schema_name default_character_set_name default_collation_name sql_path
def information_schema utf8 utf8_general_ci NULL
def mtr latin1 latin1_swedish_ci NULL
def mysql latin1 latin1_swedish_ci NULL
def performance_schema utf8 utf8_general_ci NULL
def test latin1 latin1_swedish_ci NULL
def test2 latin1 latin1_swedish_ci NULL
def test3 utf8 utf8_general_ci NULL
def test4 latin1 latin1_swedish_ci NULL
def test5 utf16 utf16_general_ci NULL
def test6 latin1 latin1_swedish_ci NULL
def test7 latin1 latin1_swedish_ci NULL
select catalog_name, schema_name, db_metadata from information_schema.schemata_ext;
catalog_name schema_name db_metadata
def information_schema
def mtr
def mysql
def performance_schema
def test
def test2
def test3 {"shard":"test3_shard"}
def test4
def test5 {"shard":"test5_shard"}
def test6
def test7 [{"shard": "test3_shard", "data2": "date2_shard"}]
show create database test7;
Database Create Database
test7 CREATE DATABASE `test7` /*!40100 DEFAULT CHARACTER SET latin1 DB_METADATA '[{"shard": "test3_shard", "data2": "date2_shard"}]' */
use test7;
create table t7 (a int, b int, primary key(a,b));
drop table t7;
set @@global.binlog_trx_meta_data = @start_binlog_trx_meta_data;
set @@global.log_warnings = @start_log_warnings;
drop database if exists test2;
drop database if exists test3;
drop database if exists test4;
drop database if exists test5;
drop database if exists test6;
drop database if exists test7;
15 changes: 15 additions & 0 deletions mysql-test/t/db_metadata.test
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ drop database if exists test7;
drop database if exists test8;
drop database if exists test9;
drop database if exists test10;
drop database if exists test11;
drop database if exists test12;
drop database if exists test13;
drop database if exists test14;
--enable_warnings

--disable_query_log
Expand Down Expand Up @@ -104,6 +108,11 @@ drop database test13;
eval $db_create;
show create database test13;

# create database with JSON array
--error ER_DB_METADATA_INVALID_JSON
create database test14 db_metadata = "[{\"shard\":\"test14_shard\", \"data\":\"1234\"}]";
select catalog_name, schema_name, db_metadata from information_schema.schemata_ext;

# alter database tests

# alter database character set
Expand Down Expand Up @@ -235,6 +244,11 @@ alter database test10 read_only = false;
show create database test10;
--exec cat $MYSQLD_DATADIR/test10/db.opt

# alter database with JSON array
--error ER_DB_METADATA_INVALID_JSON
alter database test10 db_metadata = "[{\"shard\":\"test10_shard\", \"data\":\"1234\"}]";
select catalog_name, schema_name, db_metadata from information_schema.schemata_ext;

# cleanup
--disable_warnings
drop database if exists test2;
Expand All @@ -249,4 +263,5 @@ drop database if exists test10;
drop database if exists test11;
drop database if exists test12;
drop database if exists test13;
drop database if exists test14;
--enable_warnings
34 changes: 34 additions & 0 deletions mysql-test/t/db_metadata_reset.test
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Test per-database database-metadata attribute
--source include/have_innodb.inc
--source include/have_log_bin.inc
connection default;

--disable_query_log
Expand Down Expand Up @@ -51,11 +52,44 @@ select catalog_name, schema_name, db_metadata from information_schema.schemata_e
show create database test4;
show create database test6;

# create database with db_metadatae
create database test7 db_metadata = "{\"shard\":\"test7_shard\"}";
show create database test7;
# modify database with bad db_metadatae
# db_metadata should be a json object, but now we modify it with json array
--exec echo "db-metadata=[{\"shard\": \"test3_shard\", \"data2\": \"date2_shard\"}]" > $MYSQLD_DATADIR/test7/db.opt

# Restart server to get rid of the dboptions cache
--source include/restart_mysqld.inc
set @start_binlog_trx_meta_data= @@global.binlog_trx_meta_data;
set @start_log_warnings = @@global.log_warnings;

# show db_metadata
set @@global.binlog_trx_meta_data= 1;
set @@global.log_warnings= 2;
select catalog_name, schema_name, default_character_set_name, default_collation_name, sql_path from information_schema.schemata;
select catalog_name, schema_name, db_metadata from information_schema.schemata_ext;
show create database test7;

# suppress error during THD::gen_trx_metadata
--disable_query_log
call mtr.add_suppression("Exception while adding meta data");
call mtr.add_suppression("Exception while reading meta data: \\[\\{\"shard\": \"test3_shard\", \"data2\": \"date2_shard\"\\}\\]");
--enable_query_log

# bad db-metadata won't crash mysqld or trigger assert
use test7;
create table t7 (a int, b int, primary key(a,b));
drop table t7;
set @@global.binlog_trx_meta_data = @start_binlog_trx_meta_data;
set @@global.log_warnings = @start_log_warnings;

# cleanup
--disable_warnings
drop database if exists test2;
drop database if exists test3;
drop database if exists test4;
drop database if exists test5;
drop database if exists test6;
drop database if exists test7;
--enable_warnings
14 changes: 13 additions & 1 deletion sql/binlog.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8718,6 +8718,14 @@ std::string THD::gen_trx_metadata()
std::string json = buf.GetString();
boost::trim(json);

// verify doc json document
if (!doc.IsObject())
{
// NO_LINT_DEBUG
sql_print_error("Bad JSON format after adding meta data: %s",
json.c_str());
DBUG_RETURN("");
}
std::string comment_str= std::string("/*")
.append(TRX_META_DATA_HEADER)
.append(json)
Expand Down Expand Up @@ -8772,7 +8780,11 @@ bool THD::add_db_metadata(rapidjson::Document &meta_data_root)
if (!local_db_metadata.empty())
{
rapidjson::Document db_metadata_root;
if (db_metadata_root.Parse(local_db_metadata.c_str()).HasParseError())
// rapidjson doesn't like calling GetObject() on json non-object value
// The local_db_metadata format should similar to the following example:
// {"shard":"<shard_name>", "replicaset":"<replicaset_id>"}
if (db_metadata_root.Parse(local_db_metadata.c_str()).HasParseError() ||
!db_metadata_root.IsObject())
{
// NO_LINT_DEBUG
sql_print_error("Exception while reading meta data: %s",
Expand Down
2 changes: 1 addition & 1 deletion sql/share/errmsg-utf8.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7336,7 +7336,7 @@ ER_DB_METADATA_TOO_LONG
eng "Metadata for the database is too long. Max length is %d bytes"

ER_DB_METADATA_INVALID_JSON
eng "Invalid JSON for DB_METADATA attribute: %s."
eng "Invalid JSON object for DB_METADATA attribute: %s."

ER_DB_METADATA_READ_ERROR
eng "Error reading db metadata option: '%-.64s'"
Expand Down
5 changes: 4 additions & 1 deletion sql/sql_class.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5794,7 +5794,10 @@ static std::string get_shard_id(const std::string& db_metadata)
try {
#ifdef HAVE_RAPIDJSON
rapidjson::Document db_metadata_root;
if (db_metadata_root.Parse(db_metadata.c_str()).HasParseError()) {
// The local_db_metadata format should be:
// {"shard":"<shard_name>", "replicaset":"<replicaset_id>"}
if (db_metadata_root.Parse(db_metadata.c_str()).HasParseError() ||
!db_metadata_root.IsObject()) {
return {};
}
const auto iter= db_metadata_root.FindMember("shard");
Expand Down
3 changes: 2 additions & 1 deletion sql/sql_yacc.yy
Original file line number Diff line number Diff line change
Expand Up @@ -6586,7 +6586,8 @@ db_metadata_str:
{
#ifdef HAVE_RAPIDJSON
rapidjson::Document db_metadata_root;
if (db_metadata_root.Parse($3.str).HasParseError())
if (db_metadata_root.Parse($3.str).HasParseError() ||
!db_metadata_root.IsObject())
{
my_error(ER_DB_METADATA_INVALID_JSON, MYF(0), $3.str);
MYSQL_YYABORT;
Expand Down

0 comments on commit 1452928

Please sign in to comment.