diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index b01d2c65..1d32f097 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -672,14 +672,20 @@ namespace MobileGL::MG_Remote::Wire { // The caller's tails are held to the layout the PAYLOAD declares, which is the same // arithmetic the decoder will run. A Count that says 4000 while the tail holds 8 bytes // dies here, on the producing side, rather than on a peer that can only say "corrupt". - if (tailCount != layout.TailCount) { + // + // A caller may SUPPLY FEWER TAILS THAN THE LAYOUT HAS, but only while the ones it left + // out are empty - Count == 0 is a legal record for every kVarTail row, and requiring a + // {nullptr, 0} entry for it would be a trap rather than a check. Anything else is a + // disagreement between the counts the payload declares and the bytes the caller holds. + if (tailCount > layout.TailCount) { WireProtocolFatalAt("EncodeRecord.tailCount", tailCount, layout.TailCount); } - for (Uint32 i = 0; i < tailCount; ++i) { - if (tails[i].Size != layout.TailBytes[i]) { - WireProtocolFatalAt("EncodeRecord.tailBytes", tails[i].Size, layout.TailBytes[i]); + for (Uint32 i = 0; i < layout.TailCount; ++i) { + const Uint64 supplied = i < tailCount ? tails[i].Size : 0; + if (supplied != layout.TailBytes[i]) { + WireProtocolFatalAt("EncodeRecord.tailBytes", supplied, layout.TailBytes[i]); } - if (tails[i].Size != 0 && tails[i].Bytes == nullptr) { + if (supplied != 0 && tails[i].Bytes == nullptr) { WireProtocolFatal("EncodeRecord.tail", "non-zero tail length with a null pointer"); } } diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index 13a72a68..d53ac624 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -452,6 +452,27 @@ TEST_F(PipeWireCodecTest, KVarTailRoundTripsWithItsTailIntact) { EXPECT_TRUE(applied); } +TEST_F(PipeWireCodecTest, AnEmptyVarTailIsALegalRecordAndNeedsNoPlaceholderEntry) { + // Count == 0 is legal for every kVarTail row - "bind nothing at this range" - and a caller + // that had to pass a {nullptr, 0} entry for each absent tail would be walking into a trap + // rather than through a check. SetStreamOutputTargets is the sharpest case: its layout has + // TWO tails and both are empty at Count 0. + Wire2 wire; + MGPStreamOutputTargets header{}; + header.Count = 0; + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::SetStreamOutputTargets, &header, sizeof(header)), + kInvalidSeq); + MGPVertexBuffers buffers{}; + buffers.Count = 0; + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::SetVertexBuffers, &buffers, sizeof(buffers)), + kInvalidSeq); + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_FALSE(applied); // no applier for stream output; off the reduced path + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_TRUE(applied); +} + TEST_F(PipeWireCodecTest, KReplySlotMapPersistentIsAConstantDecline) { // R-6 / R-2.4. DECLINED is a real answer, not a failure, and the applier is not called at // all: the record's payload is a bare MGPHandleOnly and carries NEITHER the size NOR the