Skip to content

Zero ServerInfo fields missing from a server Hello - #581

Open
iliaal wants to merge 1 commit into
ClickHouse:masterfrom
iliaal:fix/serverinfo-init
Open

iliaal wants to merge 1 commit into
ClickHouse:masterfrom
iliaal:fix/serverinfo-init

Conversation

@iliaal

@iliaal iliaal commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

ServerInfo in client.h has no default member initializers, and Client::Impl::ReceiveHello reads timezone, display_name and version_patch only when the server's revision carries them. A server below revision 54401 leaves version_patch indeterminate, so GetServerInfo() returns stale heap bytes. Reconnecting to an older server also keeps the previous server's values.

Fix: default-initialize the numeric members, and reset server_info_ before parsing each Hello.

Reproducer: a Hello packet that announces revision 54400 (or anything below 54401), after a connection to a newer server.

Tests: ClientProtocol.ReconnectToOldRevisionServerClearsPreviousServerInfo fails on master: it keeps "UTC", "d" and 7. ClientProtocol.OldRevisionServerHelloLeavesMissingFieldsZero is a control. It can pass on master when the fresh heap happens to be zero.

ServerInfo had no default member initializers, and Client::Impl::ReceiveHello
reads timezone, display_name and version_patch only when the server revision
carries them. A server below revision 54401 left version_patch indeterminate,
and reconnecting to an older server kept the previous server's values.
Default-initialize the numeric members and reset server_info_ before parsing
each Hello.
@iliaal
iliaal requested review from mzitnik and slabko as code owners October 18, 2026 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant