Skip to content
/ server Public

MDEV-38010: Master/Relay Log Info files ignore trailing garbage in numeric lines #4752

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ayush-jha123 wants to merge 1 commit into
base: main
Choose a base branch
from
+13 −1
Open

MDEV-38010: Master/Relay Log Info files ignore trailing garbage in numeric lines #4752

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion sql/rpl_info_file.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,21 @@ namespace Int_IO_CACHE
specific variants of a safe string-to-integer converter
(e.g., std::from_chars() when all platforms support it).
*/
if (*end == '\n' && value <= std::numeric_limits<I>::max())
if (value <= std::numeric_limits<I>::max())
{
value= static_cast<I>(val);
/*
MDEV-38010: Consume the rest of the line if the buffer didn't reach the newline.
This handles cases where trailing garbage on a numeric line caused the
parser to stop early, leaving the garbage to corrupt the next line.
*/
if (buf[length - 1] != '\n')
{
int c;
do {
c= my_b_get(file);
Copy link
Member

@gkodinov gkodinov Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole line, complete with the newline, is already read from the file into a buffer. Let's say it's "123b c\n". Before your change the thing would have read "123" from the buffer and errored out since the end of the line is not reached into the buffer immediately after.

With your change, after reading 123 from the buffer, the code will start reading from the file until it reaches a new line symbol. This will result in completely skipping the next line instead of parsing it.

To tell you frankly the whole idea behind "ignore unrecognized (e.g., non-numeric) content at the end of the line" is a bit exotic to me. I'd rather error out and return an error. As garbage after the expected value could be an indicator of a garbled file.

But, feel free to ignore the last paragraph and leave it to the final reviewer to decide.

Copy link
Author

@ayush-jha123 ayush-jha123 Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback and for catching that logic flaw! You are completely right—my previous approach of reading the rest of the file would inadvertently skip the next valid configuration line since the current line was already in the buffer.

I also agree with your point about erroring out rather than trying to gracefully ignore corrupted data. If a master.info file has trailing garbage on a numeric field, it is safer to reject it as garbled rather than risk loading incorrect replication parameters.

I have retargeted this PR to the 10.11 branch as requested, and I've updated the patch. The parsing logic (using strtol / sscanf) now checks if there are any non-whitespace characters left after reading the number. If trailing garbage is detected, it instantly returns an error. I have also added a test case to verify this behavior.

Thanks again for the guidance!

} while (c != '\n' && c != my_b_EOF);
}
return false;
}
[[fallthrough]];
Expand Down