NCBC-3852: Touch operations do not return MutationTokens

Motivation:
When GetAndTouch is used on a document of "[12345]", TryReadMutationToken is used and fails

Modifications:
* Use sizeof() in TryReadMutationToken to guard against improper reads.
* Replace magic numbers.
* GetT and Touch operations read Expiry (uint32) from Extras in response, rather than trying to read a mutation token.
* Since Touch and GetT don't inherit from the same base type, use static methods in Touch rather than inheritance to share implementation.

Change-Id: I633d1198b3e3fe3ca99cc76e9676098c60f84594
Reviewed-on: https://review.couchbase.org/c/couchbase-net-client/+/215524
Reviewed-by: Brant Burnett <[email protected]>
Reviewed-by: Jeffry Morris <[email protected]>
Tested-by: Build Bot <[email protected]>

Author: RiPont

src/Couchbase/Core/IO/Operations/GetT.cs

@@ -10,15 +10,22 @@ internal GetT(string bucketName, string key) : base(bucketName, key)
 
         protected override void WriteExtras(OperationBuilder builder)
         {
-            Span<byte> extras = stackalloc byte[4];
-            ByteConverter.FromUInt32(Expires, extras);
-            builder.Write(extras);
+            Touch.WriteExpiry(builder, Expires);
         }
 
         protected override void WriteBody(OperationBuilder builder)
         {
         }
 
+        protected override void ReadExtras(ReadOnlySpan<byte> buffer)
+        {
+            // do not call MutationOperationBase.ReadExtras, as Touch operations do not contain MutationToken
+            if (Touch.TryReadNewExpiry(buffer, Header.ExtrasLength, Header.ExtrasOffset, out var newExpiry))
+            {
+                Expires = newExpiry;
+            }
+        }
+
         public override OpCode OpCode => OpCode.GAT;
     }
 }

src/Couchbase/Core/IO/Operations/OperationBase.cs

@@ -383,10 +383,15 @@ protected void TryReadServerDuration(ReadOnlySpan<byte> buffer)
 
         protected void TryReadMutationToken(ReadOnlySpan<byte> buffer)
         {
-            if (buffer.Length >= 40 && VBucketId.HasValue)
+            var extrasOffset = Header.ExtrasOffset;
+            var extrasLength = Header.ExtrasLength;
+            var bufferMinLength = extrasOffset + extrasLength;
+            if (buffer.Length >= bufferMinLength
+                && VBucketId.HasValue
+                && extrasLength >= (sizeof(Int64)*2))
             {
-                var uuid = ByteConverter.ToInt64(buffer.Slice(Header.ExtrasOffset));
-                var seqno = ByteConverter.ToInt64(buffer.Slice(Header.ExtrasOffset + 8));
+                var uuid = ByteConverter.ToInt64(buffer.Slice(extrasOffset));
+                var seqno = ByteConverter.ToInt64(buffer.Slice(extrasOffset + sizeof(Int64)));
                 MutationToken = new MutationToken(BucketName, VBucketId.Value, uuid, seqno);
             }
         }

src/Couchbase/Core/IO/Operations/Touch.cs

@@ -5,17 +5,44 @@ namespace Couchbase.Core.IO.Operations
 {
     internal class Touch : MutationOperationBase
     {
-        protected override void WriteExtras(OperationBuilder builder)
+        internal static void WriteExpiry(OperationBuilder builder, uint expires)
         {
             Span<byte> extras = stackalloc byte[4];
-            ByteConverter.FromUInt32(Expires, extras);
+            ByteConverter.FromUInt32(expires, extras);
             builder.Write(extras);
         }
 
+        internal static bool TryReadNewExpiry(ReadOnlySpan<byte> buffer, int extrasLength, int extrasOffset, out uint expiry)
+        {
+            if (extrasLength >= sizeof(uint) && buffer.Length >= extrasOffset + extrasLength)
+            {
+                var expiryInExtras = buffer.Slice(extrasOffset);
+                expiry = ByteConverter.ToUInt32(expiryInExtras);
+                return true;
+            }
+
+            expiry = 0;
+            return false;
+        }
+
+        protected override void WriteExtras(OperationBuilder builder)
+        {
+            WriteExpiry(builder, Expires);
+        }
+
         protected override void WriteBody(OperationBuilder builder)
         {
         }
 
+        protected override void ReadExtras(ReadOnlySpan<byte> buffer)
+        {
+            // do not call MutationOperationBase.ReadExtras, as Touch operations do not contain MutationToken
+            if (Touch.TryReadNewExpiry(buffer, Header.ExtrasLength, Header.ExtrasOffset, out var newExpiry))
+            {
+                Expires = newExpiry;
+            }
+        }
+
         public override OpCode OpCode => OpCode.Touch;
     }
 }

tests/Couchbase.UnitTests/Core/IO/Operations/OperationBaseTests.cs

@@ -1,5 +1,8 @@
+using System;
 using System.Threading.Tasks;
 using Couchbase.Core.IO.Operations;
+using Couchbase.UnitTests.Helpers;
+using Couchbase.Utils;
 using Xunit;
 
 namespace Couchbase.UnitTests.Core.IO.Operations
@@ -50,6 +53,36 @@ public async Task StopOperation_CalledTwice_KeepsOriginalTime()
             Assert.Equal(elapsed, operation.Elapsed);
         }
 
+        [Fact]
+        public void OperationBase_ReadMutationTokenDoesNotThrowOnShortExtras()
+        {
+            // NCBC-3852
+            var responseBytes = new byte[47];
+            var fakeOp = new FakeMutationOperation()
+            {
+                VBucketId = 0x1,
+                Header = new OperationHeader()
+                {
+                    ExtrasLength = 4,
+                    FramingExtrasLength = 3,
+                    BodyLength = 23,
+                }
+            };
+
+            fakeOp.ForceTryReadMutationToken(responseBytes.AsSpan());
+            Assert.Null(fakeOp.MutationToken);
+        }
+
+        private class FakeMutationOperation : MutationOperationBase
+        {
+            public override OpCode OpCode => OpCode.Delete;
+
+            public void ForceTryReadMutationToken(ReadOnlySpan<byte> buffer)
+            {
+                TryReadMutationToken(buffer);
+            }
+        }
+
         private class FakeOperation : OperationBase
         {
             public override OpCode OpCode => OpCode.Get;

tests/Couchbase.UnitTests/KeyValue/GetResultTests.cs

@@ -10,6 +10,7 @@
 using Couchbase.Core.Sharding;
 using Couchbase.KeyValue;
 using Couchbase.UnitTests.Helpers;
+using Couchbase.Utils;
 using Microsoft.Extensions.Logging;
 using Moq;
 using Xunit;
@@ -241,5 +242,33 @@ public void Test_OperationBase_Throws_InvalidArgumentException_When_Key_Is_Null(
                 Assert.IsType<InvalidArgumentException>(e);
             }
         }
+
+        [Fact]
+        public void Test_GetAndTouch_ReadsExpiryNotMutationToken()
+        {
+            // NCBC-3852
+            byte[] responseBytes =
+            [
+                0x18, 0x1d, 0x03, 0x00,
+                0x04, 0x01, 0x00, 0x00,
+                0x00, 0x00, 0x00, 0x17,
+                0x1c, 0x00, 0x00, 0x00,
+                0x17, 0xf1, 0xc7, 0xcb,
+                0xf7, 0x9f, 0x00, 0x00,
+                0x02, 0x00, 0x0d, 0x02,
+                0x00, 0x00, 0x01, 0x5b,
+                0x22, 0x41, 0x42, 0x43,
+                0x44, 0x45, 0x31, 0x32,
+                0x33, 0x34, 0x35, 0x36,
+                0x37, 0x22, 0x5d
+            ];
+
+            var getAndTouch = new GetT<byte[]>("default", "Test123");
+            getAndTouch.Expires = 1234567;
+            SlicedMemoryOwner<byte> responsePacket =  new(new FakeMemoryOwner<byte>(new Memory<byte>(responseBytes)));
+            getAndTouch.Read(responsePacket);
+            Assert.NotEqual((uint)1234567, getAndTouch.Expires);
+            Assert.Null(getAndTouch.MutationToken);
+        }
     }
 }