Publisher message length check overflow

[RyanYANG52]

Hi, cloudtoid

in publisher code, message length needs to be small than capacity, if tailoffset is great than long.MaxLength, it will SafeIncrement to a negative value, so this line here should check if TailOffset < HeadOffset

interprocess/src/Interprocess/Queue/Publisher.cs

Line 34 in e2768b0

var capacity = Buffer.Capacity - tailOffset + header.HeadOffset;

great work by the work

[RyanYANG52]

this line here, if a message is split by long.MaxValue perfectly, headerOffset = long.MaxValue - 8, bodyOffset should be 0, but here is long.MinValue.

// this way should be ok
SafeIncrementMessageOffset(messageHeaderOffset, sizeof(MessageHeader));

interprocess/src/Interprocess/Queue/Queue.cs

Line 66 in e2768b0

=> sizeof(MessageHeader) + messageHeaderOffset;

[prezaei]

Hi @RyanYANG52 , can you provide a quick repro? Thanks

[RyanYANG52]

I think this test should pass, right

        [Fact]
        public unsafe void OverflowEnqueue()
        {
            using var p = (CreatePublisher(1024) as Publisher)!;

            // simulate write and read empty header until here
            p.Header->HeadOffset = 8 * (long.MaxValue / 8);
            p.Header->TailOffset = 8 * (long.MaxValue / 8);

            var message = new byte[8];

            p.TryEnqueue(message).Should().BeTrue();
            p.TryEnqueue(message).Should().BeTrue();
        }

[prezaei]

Hey @RyanYANG52 , can you check out the PR below and let me know if this solves the issue for you? Thanks

[RyanYANG52]

        protected long SafeIncrementMessageOffset(long offset, long increment)
            => (offset + increment) % Buffer.Capacity;

If using % Capacity instead of long.MaxValue, messageLength should be less than Buffer.Capacity

                if (tailOffset >= headOffset)
                {
                    if (messageLength >= Buffer.Capacity - (tailOffset - headOffset))
                        return false;
                }
                else
                {
                    if (messageLength >= headOffset - tailOffset)
                        return false;
                }

for example, this test should not pass

            using var p = CreatePublisher(48);
            // p.Buffer.Capacity == 32; (48 - 16)

            p.TryEnqueue(ByteArray3).Should().BeTrue();
            // HeadOffset == 0; TailOffset == 16;
            p.TryEnqueue(ByteArray3).Should().BeTrue();
            // HeadOffset == 0; TailOffset == 0;
            p.TryEnqueue(ByteArray3).Should().BeTrue();
            // HeadOffset == 0; TailOffset == 16;

[RyanYANG52]

            if (increment > long.MaxValue - offset)
                return -long.MaxValue + offset + increment; // Do NOT change the order of additions here

another detail I noticed is in the old approach, why use -long.MaxValue instead of long.MinValue,
8 * (long.MaxValue / 8) increment 8 would be 1, not 0

[prezaei]

@RyanYANG52 , Please check out the latest PR and LMK if that fixes the issue fully. It is a hard problem :)

[RyanYANG52]

yeah, it has lots of corner cases, try this test

            using (var p = CreatePublisher(48))
            {
                p.TryEnqueue(ByteArray50).Should().BeFalse(); // failed here
            }

if (tailOffset != headOffset) 
{
...
} 
else
{ 
// check messageLength is less or equal than Capacity
}

[prezaei]

Nice. @RyanYANG52 ... Try out the latest? Thanks for all the help man.

[RyanYANG52]

you are welcome, and all works well