commit db371c00f63ef31945cf9aaad638b318f426f618 Author: Jeremy Cole Date: Wed Jun 12 10:57:14 2013 -0700 Fix binlog header parsing; Improve mysqlbinlog output There were several instances where a binlog event's header was read incorrectly if events have extra headers. Particularly, the two events with "frozen" headers (Format_description_log_event and Rotate_event) were not using LOG_EVENT_MINIMAL_HEADER_LEN as they should have. This would cause them to potentially over-read header data when normal events have larger headers (say 31 bytes) due to those events actually having only 19-byte headers. This would cause later reads of the post-header portion including the data to be corrupted, and for the server to assert in various places. Additionally, the output of "mysqlbinlog -H" was badly formatted and also had errors related to header size. The bugs have been corrected and the formatting improved for readability (more or less to almost perfectly match "hexdump -C" that it was originally intended to mimick). Change-Id: I0dcfd45034b029c785f00a5166c9ee7f3000fb67 Reviewed-by: Pavel Ivanov diff --git a/mysql-test/r/mysqlbinlog.result b/mysql-test/r/mysqlbinlog.result index e86c237..3d4ddd5 100644 --- a/mysql-test/r/mysqlbinlog.result +++ b/mysql-test/r/mysqlbinlog.result @@ -606,7 +606,7 @@ We expect this value to be 2 (one for the INSERT, one for COMMIT). The bug being tested was that 'Query' lines were not preceded by '#' If the line is in the table, it had to have been preceded by a '#' -SELECT COUNT(*) AS `BUG#28293_expect_2` FROM patch WHERE a LIKE '%Query%'; +SELECT COUNT(*) AS `BUG#28293_expect_2` FROM patch WHERE a LIKE '#%Query%'; BUG#28293_expect_2 2 DROP TABLE patch; diff --git a/mysql-test/t/mysqlbinlog.test b/mysql-test/t/mysqlbinlog.test index 96b18be..4f862dd 100644 --- a/mysql-test/t/mysqlbinlog.test +++ b/mysql-test/t/mysqlbinlog.test @@ -275,17 +275,16 @@ FLUSH LOGS; DROP TABLE t1; -# We create a table, patch, and load the output into it -# By using LINES STARTING BY '#' + SELECT WHERE a LIKE 'Query' -# We can easily see if a 'Query' line is missing the '#' character -# as described in the original bug +# We create a table named "patch", and load the output into it. +# By using LIKE, we can easily see if the output is missing the '#' +# character, as described in the bug. --disable_query_log CREATE TABLE patch (a BLOB); --exec $MYSQL_BINLOG --hexdump --local-load=$MYSQLTEST_VARDIR/tmp/ $MYSQLD_DATADIR/master-bin.000012 > $MYSQLTEST_VARDIR/tmp/mysqlbinlog_tmp.dat ### Starting master-bin.000014 eval LOAD DATA LOCAL INFILE '$MYSQLTEST_VARDIR/tmp/mysqlbinlog_tmp.dat' - INTO TABLE patch FIELDS TERMINATED BY '' LINES STARTING BY '#'; + INTO TABLE patch FIELDS TERMINATED BY ''; --remove_file $MYSQLTEST_VARDIR/tmp/mysqlbinlog_tmp.dat --enable_query_log @@ -293,7 +292,7 @@ eval LOAD DATA LOCAL INFILE '$MYSQLTEST_VARDIR/tmp/mysqlbinlog_tmp.dat' --echo The bug being tested was that 'Query' lines were not preceded by '#' --echo If the line is in the table, it had to have been preceded by a '#' --echo -SELECT COUNT(*) AS `BUG#28293_expect_2` FROM patch WHERE a LIKE '%Query%'; +SELECT COUNT(*) AS `BUG#28293_expect_2` FROM patch WHERE a LIKE '#%Query%'; DROP TABLE patch; # diff --git a/sql/log_event.cc b/sql/log_event.cc index a85fa77..45453a6 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -1697,6 +1697,147 @@ Log_event* Log_event::read_log_event(const char* buf, uint event_len, #ifdef MYSQL_CLIENT +static void hexdump_minimal_header_to_io_cache(IO_CACHE *file, + my_off_t offset, + uchar *ptr) +{ + DBUG_ASSERT(LOG_EVENT_MINIMAL_HEADER_LEN == 19); + + /* + Pretty-print the first LOG_EVENT_MINIMAL_HEADER_LEN (19) bytes of the + common header, which contains the basic information about the log event. + Every event will have at least this much header, but events could contain + more headers (which must be printed by other methods, if desired). + */ + char emit_buf[256]; // Enough for storing one line + my_b_printf(file, + "# " + "|Timestamp " + "|Type " + "|Master ID " + "|Size " + "|Master Pos " + "|Flags\n"); + size_t const emit_buf_written= + my_snprintf(emit_buf, sizeof(emit_buf), + "# %8llx " /* Position */ + "|%02x %02x %02x %02x " /* Timestamp */ + "|%02x " /* Type */ + "|%02x %02x %02x %02x " /* Master ID */ + "|%02x %02x %02x %02x " /* Size */ + "|%02x %02x %02x %02x " /* Master Pos */ + "|%02x %02x\n", /* Flags */ + (ulonglong) offset, /* Position */ + ptr[0], ptr[1], ptr[2], ptr[3], /* Timestamp */ + ptr[4], /* Type */ + ptr[5], ptr[6], ptr[7], ptr[8], /* Master ID */ + ptr[9], ptr[10], ptr[11], ptr[12], /* Size */ + ptr[13], ptr[14], ptr[15], ptr[16], /* Master Pos */ + ptr[17], ptr[18]); /* Flags */ + + DBUG_ASSERT(static_cast(emit_buf_written) < sizeof(emit_buf)); + my_b_write(file, reinterpret_cast(emit_buf), emit_buf_written); + my_b_write(file, "#\n", 2); +} + +/* + The number of bytes to print per line. Should be an even number, + and "hexdump -C" uses 16, so we'll duplicate that here. +*/ +#define HEXDUMP_BYTES_PER_LINE 16 + +static void hexdump_data_to_io_cache(IO_CACHE *file, + my_off_t offset, + uchar *ptr, + my_off_t size) +{ + /* + Each byte prints as two hex digits, plus a space. The hex output will + also have an extra space in the middle, and the \0 at the end. + */ + char *h, hex_string[HEXDUMP_BYTES_PER_LINE * 3 + 2]= {0}; + char *c, char_string[HEXDUMP_BYTES_PER_LINE + 1]= {0}; + my_off_t i; + + if (size == 0) + return; + + /* + Print the rest of the event (without common header) + */ + my_off_t starting_offset = offset; + for (i= 0, c= char_string, h= hex_string; + i < size; + i++, ptr++) + { + my_snprintf(h, 4, "%02x ", *ptr); + h+= 3; + + *c++= my_isprint(&my_charset_bin, *ptr) ? *ptr : '.'; + + /* Print in groups of HEXDUMP_BYTES_PER_LINE characters. */ + if ((i % HEXDUMP_BYTES_PER_LINE) == (HEXDUMP_BYTES_PER_LINE - 1)) + { + /* + Currently, my_b_printf() does not support the full set of printf() + formats, so we have to do it this way. + */ + *c= '\0'; + *h= '\0'; + char emit_buf[256]; + size_t const emit_buf_written= + my_snprintf(emit_buf, sizeof(emit_buf), + "# %8llx %s |%s|\n", + (ulonglong) starting_offset, + hex_string, char_string); + DBUG_ASSERT(static_cast(emit_buf_written) < sizeof(emit_buf)); + my_b_write(file, reinterpret_cast(emit_buf), emit_buf_written); + c= char_string; + h= hex_string; + starting_offset+= HEXDUMP_BYTES_PER_LINE; + } + else if ((i % (HEXDUMP_BYTES_PER_LINE / 2)) + == ((HEXDUMP_BYTES_PER_LINE / 2) - 1)) + { + /* + In the middle of the group of HEXDUMP_BYTES_PER_LINE, emit an extra + space in the hex string, to make two groups. + */ + *h++= ' '; + } + + } + *c= '\0'; + *h= '\0'; + + /* + There is still data left in our buffer, which means that the previous + line was not perfectly HEXDUMP_BYTES_PER_LINE characters, so write an + incomplete line, with spaces to pad out to the same length as a full + line would be, to make things more readable. + */ + if (hex_string[0]) + { + char emit_buf[256]; + for (int j= (HEXDUMP_BYTES_PER_LINE - strlen(char_string)); j > 0; j--) + { + if (j == (HEXDUMP_BYTES_PER_LINE / 2)) *h++= ' '; + *h++= ' '; + *h++= ' '; + *h++= ' '; + } + *h= '\0'; + size_t const emit_buf_written= + my_snprintf(emit_buf, sizeof(emit_buf), + "# %8llx %s |%s|\n", + (ulonglong) starting_offset, + hex_string, char_string); + DBUG_ASSERT(static_cast(emit_buf_written) < sizeof(emit_buf)); + my_b_write(file, reinterpret_cast(emit_buf), emit_buf_written); + } + my_b_write(file, "#\n", 2); +} + /* Log_event::print_header() */ @@ -1731,86 +1872,43 @@ void Log_event::print_header(IO_CACHE* file, { my_b_printf(file, "\n"); uchar *ptr= (uchar*)temp_buf; - my_off_t size= - uint4korr(ptr + EVENT_LEN_OFFSET) - LOG_EVENT_MINIMAL_HEADER_LEN; - my_off_t i; + my_off_t size= uint4korr(ptr + EVENT_LEN_OFFSET); - /* Header len * 4 >= header len * (2 chars + space + extra space) */ - char *h, hex_string[LOG_EVENT_MINIMAL_HEADER_LEN*4]= {0}; - char *c, char_string[16+1]= {0}; + if (has_fixed_minimal_header()) + size-= LOG_EVENT_MINIMAL_HEADER_LEN; + else + size-= print_event_info->common_header_len; - /* Pretty-print event common header if header is exactly 19 bytes */ - if (print_event_info->common_header_len == LOG_EVENT_MINIMAL_HEADER_LEN) - { - char emit_buf[256]; // Enough for storing one line - my_b_printf(file, "# Position Timestamp Type Master ID " - "Size Master Pos Flags \n"); - size_t const bytes_written= - my_snprintf(emit_buf, sizeof(emit_buf), - "# %8.8lx %02x %02x %02x %02x %02x " - "%02x %02x %02x %02x %02x %02x %02x %02x " - "%02x %02x %02x %02x %02x %02x\n", - (unsigned long) hexdump_from, - ptr[0], ptr[1], ptr[2], ptr[3], ptr[4], ptr[5], ptr[6], - ptr[7], ptr[8], ptr[9], ptr[10], ptr[11], ptr[12], ptr[13], - ptr[14], ptr[15], ptr[16], ptr[17], ptr[18]); - DBUG_ASSERT(static_cast(bytes_written) < sizeof(emit_buf)); - my_b_write(file, (uchar*) emit_buf, bytes_written); - ptr += LOG_EVENT_MINIMAL_HEADER_LEN; - hexdump_from += LOG_EVENT_MINIMAL_HEADER_LEN; - } - - /* Rest of event (without common header) */ - for (i= 0, c= char_string, h=hex_string; - i < size; - i++, ptr++) - { - my_snprintf(h, 4, "%02x ", *ptr); - h += 3; - - *c++= my_isalnum(&my_charset_bin, *ptr) ? *ptr : '.'; - - if (i % 16 == 15) - { - /* - my_b_printf() does not support full printf() formats, so we - have to do it this way. + my_b_printf(file, "# Position\n"); - TODO: Rewrite my_b_printf() to support full printf() syntax. - */ - char emit_buf[256]; - size_t const bytes_written= - my_snprintf(emit_buf, sizeof(emit_buf), - "# %8.8lx %-48.48s |%16s|\n", - (unsigned long) (hexdump_from + (i & 0xfffffff0)), - hex_string, char_string); - DBUG_ASSERT(static_cast(bytes_written) < sizeof(emit_buf)); - my_b_write(file, (uchar*) emit_buf, bytes_written); - hex_string[0]= 0; - char_string[0]= 0; - c= char_string; - h= hex_string; - } - else if (i % 8 == 7) *h++ = ' '; - } - *c= '\0'; + /* Write the header, nicely formatted by field. */ + hexdump_minimal_header_to_io_cache(file, hexdump_from, ptr); - if (hex_string[0]) + if (has_fixed_minimal_header()) { - char emit_buf[256]; - size_t const bytes_written= - my_snprintf(emit_buf, sizeof(emit_buf), - "# %8.8lx %-48.48s |%s|\n", - (unsigned long) (hexdump_from + (i & 0xfffffff0)), - hex_string, char_string); - DBUG_ASSERT(static_cast(bytes_written) < sizeof(emit_buf)); - my_b_write(file, (uchar*) emit_buf, bytes_written); + /* The FDE and rotate events have a "frozen" minimal header. */ + ptr+= LOG_EVENT_MINIMAL_HEADER_LEN; + hexdump_from+= LOG_EVENT_MINIMAL_HEADER_LEN; } + else + { + /* + We've only pretty-printed the minimal header; but skip the rest, + if any (if common_header_len > LOG_EVENT_MINIMAL_HEADER_LEN), as + we don't necessarily understand it. + */ + ptr+= print_event_info->common_header_len; + hexdump_from+= print_event_info->common_header_len; + } + + /* Print the rest of the data, mimicking "hexdump -C" output. */ + hexdump_data_to_io_cache(file, hexdump_from, ptr, size); + /* - need a # to prefix the rest of printouts for example those of - Rows_log_event::print_helper(). + Prefix the next line so that the output from print_helper() + will appear as a comment. */ - my_b_write(file, reinterpret_cast("# "), 2); + my_b_write(file, "# Event: ", 9); } DBUG_VOID_RETURN; } @@ -4343,7 +4441,7 @@ Start_log_event_v3::Start_log_event_v3(const char* buf, *description_event) :Log_event(buf, description_event) { - buf+= description_event->common_header_len; + buf+= LOG_EVENT_MINIMAL_HEADER_LEN; binlog_version= uint2korr(buf+ST_BINLOG_VER_OFFSET); memcpy(server_version, buf+ST_SERVER_VER_OFFSET, ST_SERVER_VER_LEN); @@ -5012,9 +5110,9 @@ uint8 get_checksum_alg(const char* buf, ulong len) DBUG_ENTER("get_checksum_alg"); DBUG_ASSERT(buf[EVENT_TYPE_OFFSET] == FORMAT_DESCRIPTION_EVENT); - memcpy(version, buf + - buf[LOG_EVENT_MINIMAL_HEADER_LEN + ST_COMMON_HEADER_LEN_OFFSET] - + ST_SERVER_VER_OFFSET, ST_SERVER_VER_LEN); + memcpy(version, + buf + LOG_EVENT_MINIMAL_HEADER_LEN + ST_SERVER_VER_OFFSET, + ST_SERVER_VER_LEN); version[ST_SERVER_VER_LEN - 1]= 0; do_server_version_split(version, &version_split); @@ -5885,16 +5983,14 @@ Rotate_log_event::Rotate_log_event(const char* buf, uint event_len, { DBUG_ENTER("Rotate_log_event::Rotate_log_event(char*,...)"); // The caller will ensure that event_len is what we have at EVENT_LEN_OFFSET - uint8 header_size= description_event->common_header_len; uint8 post_header_len= description_event->post_header_len[ROTATE_EVENT-1]; uint ident_offset; - if (event_len < header_size) + if (event_len < LOG_EVENT_MINIMAL_HEADER_LEN) DBUG_VOID_RETURN; - buf += header_size; - pos = post_header_len ? uint8korr(buf + R_POS_OFFSET) : 4; - ident_len = (uint)(event_len - - (header_size+post_header_len)); - ident_offset = post_header_len; + buf+= LOG_EVENT_MINIMAL_HEADER_LEN; + pos= post_header_len ? uint8korr(buf + R_POS_OFFSET) : 4; + ident_len= (uint)(event_len - (LOG_EVENT_MINIMAL_HEADER_LEN + post_header_len)); + ident_offset= post_header_len; set_if_smaller(ident_len,FN_REFLEN-1); new_log_ident= my_strndup(buf + ident_offset, (uint) ident_len, MYF(MY_WME)); DBUG_PRINT("debug", ("new_log_ident: '%s'", new_log_ident)); diff --git a/sql/log_event.h b/sql/log_event.h index bfca295..d19c080 100644 --- a/sql/log_event.h +++ b/sql/log_event.h @@ -1225,6 +1225,7 @@ public: #endif virtual Log_event_type get_type_code() = 0; virtual bool is_valid() const = 0; + virtual bool has_fixed_minimal_header() { return FALSE; } void set_artificial_event() { flags |= LOG_EVENT_ARTIFICIAL_F; } void set_relay_log_event() { flags |= LOG_EVENT_RELAY_LOG_F; } bool is_artificial_event() const { return flags & LOG_EVENT_ARTIFICIAL_F; } @@ -2441,6 +2442,7 @@ public: const Format_description_log_event* description_event); ~Start_log_event_v3() {} Log_event_type get_type_code() { return START_EVENT_V3;} + bool has_fixed_minimal_header() { return TRUE; } #ifdef MYSQL_SERVER bool write(IO_CACHE* file); #endif @@ -2937,6 +2939,7 @@ public: my_free((void*) new_log_ident); } Log_event_type get_type_code() { return ROTATE_EVENT;} + bool has_fixed_minimal_header() { return TRUE; } int get_data_size() { return ident_len + ROTATE_HEADER_LEN;} bool is_valid() const { return new_log_ident != 0; } #ifdef MYSQL_SERVER