MDEV-40759: Fix --force-read crash on checksum-failing Format Description event

[prathamesh04]

Fixes MDEV-40759.

Problem

With --force-read, Log_event::read_log_event() returns an
Unknown_log_event (not NULL) when a Format Description event's checksum is
invalid. check_header()'s --start-position prelude blindly down-cast such
an event to Format_description_log_event, which is undefined behaviour (a
vptr error under UBSan, and a crash under -D_GLIBCXX_DEBUG), and in
--base64-output modes other than AUTO also silently swallowed the event.

Fix

• client/mysqlbinlog.cc: in check_header(), report a checksum-failing
  Format Description event through process_event() in both output modes
  (instead of down-casting it), then continue with the default Format
  Description event.
• sql/log_event.cc: only assert "no result on error" when --force-read is
  not in effect, and free the discarded best-effort Unknown_log_event
  (avoids a memory leak).

Test

binlog_mysqlbinlog_force_read simulates the checksum failure with
--debug=d,simulate_checksum_test_failure and checks both AUTO and
--base64-output=NEVER modes. Verified with the debug MTR run:

binlog.binlog_mysqlbinlog_force_read 'mix' [ pass ]

[ParadoxV5]

Thanks for your interest, but we do merge-ups instead of backports here, so please base your PR on 10.11 (where another fix has made this part of your patch obsolete) rather than main.

[ParadoxV5]

This approach is new to me, but I’m not convinced.

A Format Description Event that’s invalid (whether because of a checksum mismatch or some other corruption) is still meant to describe the log’s format.
Both ignoring it and substituting with “a default” FDE would be very wrong (at least for the premise of this check_header() function, in contrast to the dream where the default FDE is already sufficient).

The only case this would not matter is when two FDEs appear back-to-back – this can only come from a handcrafted log, AFAIK.

[ParadoxV5]

In fact, you can smell how wrong it gets from how mariadb-binlog fails if you have some content after start-position:

Suggested change

	INSERT INTO t1 VALUES (1);
	--let $MYSQL_DATADIR= `SELECT @@datadir`
	--let $binlog_file= query_get_value(SHOW BINLOG STATUS, File, 1)
	--let $binlog_start= query_get_value(SHOW BINLOG STATUS, Position, 1)
	--let $MYSQL_DATADIR= `SELECT @@datadir`
	--let $binlog_file= query_get_value(SHOW BINLOG STATUS, File, 1)
	--let $binlog_start= query_get_value(SHOW BINLOG STATUS, Position, 1)
	INSERT INTO t1 VALUES (1);

In the final test, you can use the DROP below as the “content after start-position” rather than this INSERT.

[ParadoxV5]

Hm, what’s this? It’s not used in either 10.11 or the base main… 🧐🤔

[ParadoxV5]

It came from before the hard-forking and is never used…

Personal opinion: It’s good that it’s not used, because it works by scanning the entire mariadb-binlog --help…
(Unlike have_debug.inc from long ago; though suite.pm still scans the entire mariadbd --help, from which have_debug.inc is now derived.)
There ought to be more efficient ways.

[ParadoxV5]

FYI: A modern “blank” binlog is already past the FDE: There’s the GTID List, a Binlog Checkpoint for XA, maybe (forgot if there is) a Rotate.

Manually growing the log is also fine, to not depend on those additional header events, especially since we do want to test with some manually controlled content below.