fix(IPC-39): make the CommandTooLarge pre-check allow for the sequence stamp
The pre-check measures the command envelope while Sequence is still 0, and an unset protobuf field costs nothing to encode -- so the measured size is a lower bound on the frame WriteLoopAsync emits after GWC-28 stamps the sequence. Being a lower bound is the right kind of conservative for the knownSize question the comment answers, and the wrong kind for the pre-check itself: it admits an envelope in the last eleven bytes of the range that the writer then rejects with a session-fatal MessageTooLarge. Compare against MaxMessageBytes - MaxSequenceEncodingBytes (1 tag byte + a 10-byte varint, the widest 64-bit field encoding) and say in the comment that the pre-check is a lower bound relying on that allowance. Not reachable under the shipped validator (caller-controlled commands arrive through gRPC capped at least 64 KiB below the pipe max, gateway-authored commands are tiny), so this closes the argument rather than a live defect. The existing CommandTooLarge test still passes unchanged.
This commit is contained in:
@@ -237,15 +237,22 @@ public sealed class WorkerClient : IWorkerClient
|
|||||||
// CalculateSize. WriteLoopAsync stamps envelope.Sequence immediately before the write
|
// CalculateSize. WriteLoopAsync stamps envelope.Sequence immediately before the write
|
||||||
// (GWC-28), and a non-zero varint field grows the encoding — so the number computed here
|
// (GWC-28), and a non-zero varint field grows the encoding — so the number computed here
|
||||||
// is a lower bound on the frame the writer actually emits, never the frame length. Passing
|
// is a lower bound on the frame the writer actually emits, never the frame length. Passing
|
||||||
// it as a knownSize would under-length the prefix and desync the worker's framing. The
|
// it as a knownSize would under-length the prefix and desync the worker's framing.
|
||||||
// pre-check stays a pre-check: it is conservative in the right direction.
|
//
|
||||||
|
// IPC-39: being a lower bound is conservative for the knownSize question but the wrong
|
||||||
|
// direction for this pre-check, which must not admit an envelope the writer will then
|
||||||
|
// reject with a session-fatal MessageTooLarge. Compare against the frame maximum less the
|
||||||
|
// MaxSequenceEncodingBytes the stamp can add, so every envelope this check accepts still
|
||||||
|
// fits once the sequence is written.
|
||||||
int envelopeSize = commandEnvelope.CalculateSize();
|
int envelopeSize = commandEnvelope.CalculateSize();
|
||||||
if (envelopeSize > _connection.FrameOptions.MaxMessageBytes)
|
int preCheckLimit = _connection.FrameOptions.MaxMessageBytes - MaxSequenceEncodingBytes;
|
||||||
|
if (envelopeSize > preCheckLimit)
|
||||||
{
|
{
|
||||||
throw new WorkerClientException(
|
throw new WorkerClientException(
|
||||||
WorkerClientErrorCode.CommandTooLarge,
|
WorkerClientErrorCode.CommandTooLarge,
|
||||||
$"Worker command {method} serializes to {envelopeSize} bytes, exceeding the negotiated "
|
$"Worker command {method} serializes to {envelopeSize} bytes, exceeding the negotiated "
|
||||||
+ $"worker-frame maximum of {_connection.FrameOptions.MaxMessageBytes} bytes.");
|
+ $"worker-frame maximum of {_connection.FrameOptions.MaxMessageBytes} bytes less the "
|
||||||
|
+ $"{MaxSequenceEncodingBytes}-byte sequence-stamp allowance.");
|
||||||
}
|
}
|
||||||
|
|
||||||
await EnqueueAsync(commandEnvelope, cancellationToken).ConfigureAwait(false);
|
await EnqueueAsync(commandEnvelope, cancellationToken).ConfigureAwait(false);
|
||||||
@@ -455,6 +462,18 @@ public sealed class WorkerClient : IWorkerClient
|
|||||||
_stopCts.Dispose();
|
_stopCts.Dispose();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Worst-case number of bytes <see cref="WriteLoopAsync"/>'s sequence stamp adds to an
|
||||||
|
/// envelope that was sized before it: one tag byte plus a ten-byte varint, the maximum
|
||||||
|
/// encoding of a 64-bit protobuf field. The <c>CommandTooLarge</c> pre-check subtracts this
|
||||||
|
/// from the negotiated frame maximum (IPC-39) because it measures the envelope while
|
||||||
|
/// <c>Sequence</c> is still 0 — an unset field costs nothing to encode, so the measured size
|
||||||
|
/// is a lower bound on the frame the writer emits, and without the allowance an envelope in
|
||||||
|
/// the last eleven bytes of the range would pass the pre-check and then fault the session on
|
||||||
|
/// the write.
|
||||||
|
/// </summary>
|
||||||
|
private const int MaxSequenceEncodingBytes = 11;
|
||||||
|
|
||||||
/// <summary>Manages writing envelopes to the worker pipe.</summary>
|
/// <summary>Manages writing envelopes to the worker pipe.</summary>
|
||||||
private async Task WriteLoopAsync()
|
private async Task WriteLoopAsync()
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user