Summary
A dialer can never renegotiate the protocol version, because it never recognizes the response that
would tell it to. No version change is pending, so nothing is broken today, but the mechanism that
exists for one does not work and would have to be fixed before it could be used.
Detail
WriteUnknownVersionResponse writes 16 bytes: the magic, a version count, and two versions. A
listener writes exactly that and then closes the connection (classic_listener.go).
ReadV2 reads a full 20-byte message frame before it inspects the magic, so those 16 bytes plus a
close surface as unexpected EOF. The magicUnknownVersion branch is never reached, so
GetRetryVersion never returns a version, and the dialer's retry in CreateWithHeaders redials with
the version that just failed.
Measured against a listener that sends a real response: err type=*errors.errorString,
GetRetryVersion -> 2, false. Padding the response to 20 bytes produces
channel.UnsupportedVersionError and GetRetryVersion -> 2, true, so the rest of the mechanism works
once the response is recognized.
Two further defects sit behind that one, so a recognized response would still parse incorrectly:
- When the version list has not fully arrived, the continuation read fills a
buf declared inside the
if, shadowing the outer one. The bytes read are discarded and only the short leftover is parsed.
- The parse loop runs to the end of the buffer rather than stopping at the declared count, so trailing
bytes are reported as supported versions.
Separately, the count sizes an allocation and arrives from a peer that has not been authenticated, with
no bound on it.
Proposal
Classify the frame once the magic has been read rather than after a whole frame, reading at least the
magic so a frame that has already arrived in full still takes a single read. Parse the version list
through the bytes buffered with the magic followed by the wire, honor the declared count, and bound it.
Summary
A dialer can never renegotiate the protocol version, because it never recognizes the response that
would tell it to. No version change is pending, so nothing is broken today, but the mechanism that
exists for one does not work and would have to be fixed before it could be used.
Detail
WriteUnknownVersionResponsewrites 16 bytes: the magic, a version count, and two versions. Alistener writes exactly that and then closes the connection (
classic_listener.go).ReadV2reads a full 20-byte message frame before it inspects the magic, so those 16 bytes plus aclose surface as
unexpected EOF. ThemagicUnknownVersionbranch is never reached, soGetRetryVersionnever returns a version, and the dialer's retry inCreateWithHeadersredials withthe version that just failed.
Measured against a listener that sends a real response:
err type=*errors.errorString,GetRetryVersion -> 2, false. Padding the response to 20 bytes produceschannel.UnsupportedVersionErrorandGetRetryVersion -> 2, true, so the rest of the mechanism worksonce the response is recognized.
Two further defects sit behind that one, so a recognized response would still parse incorrectly:
bufdeclared inside theif, shadowing the outer one. The bytes read are discarded and only the short leftover is parsed.bytes are reported as supported versions.
Separately, the count sizes an allocation and arrives from a peer that has not been authenticated, with
no bound on it.
Proposal
Classify the frame once the magic has been read rather than after a whole frame, reading at least the
magic so a frame that has already arrived in full still takes a single read. Parse the version list
through the bytes buffered with the magic followed by the wire, honor the declared count, and bound it.