commands: the wire version says what the version string says
`buildInfo` has always reported 4.4.0 and the handshake has always reported maxWireVersion 8, which is 4.2. A driver believes the wire version: it refused client-side to send `hint` on an unacknowledged delete or findAndModify (the error is "only supported on MongoDB 4.4+", raised without a round trip), and withheld `comment` from getMore, listCollections and listDatabases (lib/operations/get_more.js:43 and its neighbours). Both are things this engine handles -- the acknowledged hint suites pass, and getMore ignores fields it does not know -- so the effect was purely the number disagreeing with itself. The 8 was not arbitrary. The comment above it tied it to omitting `topologyVersion`, which is what keeps a driver off the streaming hello protocol we do not implement -- a real bug, once visible as Compass reconnecting every heartbeat. Checked before touching it, in the driver rather than from memory: `useStreamingProtocol` (lib/sdam/monitor.js:154) returns false whenever `topologyVersion` is absent and never looks at the wire version at all. The omission is the whole mechanism; the wire version was a second line of defence that never existed. The comment now says so. A test asserts the two agree, so they cannot drift apart again silently, which is the actual defect here -- not the value. Spec suites 173/118/196 -> 189/102/196: 16 cases fixed, none broken. Ten are the unacknowledged-hint cases the previous commit uncovered, six are `comment` forwarding. 166/166 unit tests in ReleaseFast and ReleaseSafe, 82/82 fuzz, e2e 49, e2e2 concurrent 2 + crash pair, e2e3 16, e2e4 17, e2e6 72, e2e7 86, crash-fuzz 60 cycles.
This commit is contained in:
@@ -257,7 +257,13 @@ fn add_server_info(ctx: *Context, reply: *wire.Reply) !void {
|
||||
try reply.put("logicalSessionTimeoutMinutes", .{ .int32 = 30 });
|
||||
try reply.put("connectionId", .{ .int32 = @intCast(ctx.connection_id) });
|
||||
try reply.put("minWireVersion", .{ .int32 = 0 });
|
||||
try reply.put("maxWireVersion", .{ .int32 = 8 });
|
||||
// 9, because this server calls itself 4.4.0 in `buildInfo` and 4.4 is
|
||||
// wire 9. Reporting 8 was reporting 4.2, and a driver believes the wire
|
||||
// version over the string: it refused client-side to send `hint` on an
|
||||
// unacknowledged delete or findAndModify (ten spec cases), and withheld
|
||||
// `comment` from getMore, listCollections and listDatabases. Both are
|
||||
// things this engine handles.
|
||||
try reply.put("maxWireVersion", .{ .int32 = 9 });
|
||||
try reply.put("readOnly", .{ .bool = false });
|
||||
|
||||
// Deliberately no `topologyVersion`. A driver treats its presence as
|
||||
@@ -269,8 +275,13 @@ fn add_server_info(ctx: *Context, reply: *wire.Reply) !void {
|
||||
// fails the heartbeat ("Server ended moreToCome unexpectedly"), drops
|
||||
// the connection and resets its pool — a connect/disconnect loop once
|
||||
// per heartbeat, which is what MongoDB Compass showed. Omitting the
|
||||
// field keeps monitoring on the polling path, which we do implement,
|
||||
// and matches maxWireVersion 8: streaming hello arrived in wire 9.
|
||||
// field keeps monitoring on the polling path, which we do implement.
|
||||
//
|
||||
// This is the whole of the mechanism, and it does not depend on the wire
|
||||
// version: `useStreamingProtocol` (driver lib/sdam/monitor.js:154) polls
|
||||
// whenever `topologyVersion` is absent, whatever else the handshake said.
|
||||
// Checked when maxWireVersion went to 9 above, since the old comment here
|
||||
// leaned on 8 as a second line of defence that never existed.
|
||||
}
|
||||
|
||||
fn cmd_hello(ctx: *Context, _: *wire.Message, reply: *wire.Reply) !void {
|
||||
@@ -2718,7 +2729,38 @@ test "ping and hello replies parse" {
|
||||
try testing.expectEqual(@as(f64, 1.0), ok.double);
|
||||
const primary = bson.get_pair(reply2.pairs.items, "isWritablePrimary").?;
|
||||
try testing.expect(primary.bool);
|
||||
try testing.expectEqual(@as(i32, 8), bson.get_pair(reply2.pairs.items, "maxWireVersion").?.int32);
|
||||
try testing.expectEqual(@as(i32, 9), bson.get_pair(reply2.pairs.items, "maxWireVersion").?.int32);
|
||||
}
|
||||
|
||||
test "the wire version agrees with the version the server calls itself" {
|
||||
// These two are read by different parts of a driver -- the handshake picks
|
||||
// features off the wire version, `runOnRequirements` in the spec suites
|
||||
// reads the string -- and when they disagreed the driver believed the wire
|
||||
// version and withheld commands the version string promised.
|
||||
var threaded: std.Io.Threaded = .init_single_threaded;
|
||||
defer threaded.deinit();
|
||||
const io = threaded.io();
|
||||
var tdb = try TestDb.init(io);
|
||||
defer tdb.deinit();
|
||||
var ctx = tdb.ctx(io);
|
||||
|
||||
var hello = wire.Reply.init(testing.allocator);
|
||||
defer hello.deinit();
|
||||
var hello_msg = try parse_fake_msg("hello", .null, &.{});
|
||||
defer hello_msg.deinit();
|
||||
try dispatch(&ctx, &hello_msg, &hello);
|
||||
|
||||
var info = wire.Reply.init(testing.allocator);
|
||||
defer info.deinit();
|
||||
var info_msg = try parse_fake_msg("buildInfo", .null, &.{});
|
||||
defer info_msg.deinit();
|
||||
try dispatch(&ctx, &info_msg, &info);
|
||||
|
||||
// The mapping is the server's own: 4.2 is wire 8, 4.4 is wire 9.
|
||||
const wire_version = bson.get_pair(hello.pairs.items, "maxWireVersion").?.int32;
|
||||
const version = bson.get_pair(info.pairs.items, "version").?.string;
|
||||
const expected: i32 = if (std.mem.startsWith(u8, version, "4.4.")) 9 else if (std.mem.startsWith(u8, version, "4.2.")) 8 else -1;
|
||||
try testing.expectEqual(expected, wire_version);
|
||||
}
|
||||
|
||||
test "handshake does not advertise the streaming hello protocol" {
|
||||
|
||||
Reference in New Issue
Block a user