Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: operation direction parse error #278

Merged
merged 15 commits into from
Apr 23, 2024
1 change: 1 addition & 0 deletions libs/server/Objects/List/ListObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public enum OperationDirection
/// Right or tail
/// </summary>
Right,
Unknown,
}


Expand Down
8 changes: 6 additions & 2 deletions libs/server/Resp/Objects/ListCommands.cs
Original file line number Diff line number Diff line change
Expand Up @@ -678,8 +678,12 @@ private unsafe bool ListMove<TGarnetApi>(int count, byte* ptr, ref TGarnetApi st
if (!RespReadUtils.ReadPtrWithLengthHeader(ref param2.ptr, ref param2.length, ref ptr, recvBufferPtr + bytesRead))
return false;

var sourceDirection = param1.ReadOnlySpan.SequenceEqual("RIGHT"u8) ? OperationDirection.Right : OperationDirection.Left;
var destinationDirection = param2.ReadOnlySpan.SequenceEqual("RIGHT"u8) ? OperationDirection.Right : OperationDirection.Left;
OperationDirection sourceDirection = GetOperationDirection(param1);
OperationDirection destinationDirection = GetOperationDirection(param2);
if (sourceDirection == OperationDirection.Unknown || destinationDirection == OperationDirection.Unknown)
{
return AbortWithErrorMessage(count, CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR);
Zzhiter marked this conversation as resolved.
Show resolved Hide resolved
}

result = ListMove(count, sourceKey, destinationKey, sourceDirection, destinationDirection, out var node, ref storageApi);
if (node != null)
Expand Down
22 changes: 22 additions & 0 deletions libs/server/Resp/Objects/ObjectStoreUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,27 @@ private bool AbortWithErrorMessage(int count, ReadOnlySpan<byte> errorMessage)

return true;
}

/// <summary>
/// Tries to parse the input as "LEFT" or "RIGHT" and returns the corresponding OperationDirection.
/// If parsing fails, returns OperationDirection.Unknown.
/// </summary>
/// <param name="input">The input to parse.</param>
/// <returns>The parsed OperationDirection, or OperationDirection.Unknown if parsing fails.</returns>
public OperationDirection GetOperationDirection(ArgSlice input)
{
// Optimize for the common case
if (input.ReadOnlySpan.SequenceEqual("LEFT"u8))
return OperationDirection.Left;
if (input.ReadOnlySpan.SequenceEqual("RIGHT"u8))
return OperationDirection.Right;
// Rare case: try making upper case and retry
MakeUpperCase(input.ptr);
if (input.ReadOnlySpan.SequenceEqual("LEFT"u8))
return OperationDirection.Left;
if (input.ReadOnlySpan.SequenceEqual("RIGHT"u8))
return OperationDirection.Right;
return OperationDirection.Unknown;
}
}
}
40 changes: 40 additions & 0 deletions test/Garnet.test/RespListTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,13 @@ public async Task CanUseLMoveGC()
using var db = TestUtils.GetGarnetClient();
db.Connect();

// Test for Operation direction error.
var exception = Assert.ThrowsAsync<Exception>(async () =>
{
await db.ExecuteForStringResultAsync("LMOVE", new string[] { "mylist", "myotherlist", "right", "lef" });
});
Assert.AreEqual("ERR syntax error", exception.Message);

//If source does not exist, the value nil is returned and no operation is performed.
var response = await db.ExecuteForStringResultAsync("LMOVE", new string[] { "mylist", "myotherlist", "RIGHT", "LEFT" });
Assert.AreEqual(null, response);
Expand Down Expand Up @@ -681,6 +688,39 @@ public async Task CanUseLMoveGC()
Assert.AreEqual(expectedResponseArray, responseArray);
}

[Test]
public async Task CanUseLMoveWithCaseInsensitiveDirectionGC()
{
using var db = TestUtils.GetGarnetClient();
db.Connect();

await db.ExecuteForStringResultAsync("RPUSH", new string[] { "mylist", "one" });
await db.ExecuteForStringResultAsync("RPUSH", new string[] { "mylist", "two" });
await db.ExecuteForStringResultAsync("RPUSH", new string[] { "mylist", "three" });

var response = await db.ExecuteForStringResultAsync("LMOVE", new string[] { "mylist", "myotherlist", "right", "left" });
Assert.AreEqual("three", response);

var responseArray = await db.ExecuteForStringArrayResultAsync("LRANGE", new string[] { "mylist", "0", "-1" });
var expectedResponseArray = new string[] { "one", "two" };
Assert.AreEqual(expectedResponseArray, responseArray);

responseArray = await db.ExecuteForStringArrayResultAsync("LRANGE", new string[] { "myotherlist", "0", "-1" });
expectedResponseArray = new string[] { "three" };
Assert.AreEqual(expectedResponseArray, responseArray);

response = await db.ExecuteForStringResultAsync("LMOVE", new string[] { "mylist", "myotherlist", "LeFT", "RIghT" });
Assert.AreEqual("one", response);

responseArray = await db.ExecuteForStringArrayResultAsync("LRANGE", new string[] { "mylist", "0", "-1" });
expectedResponseArray = new string[] { "two" };
Assert.AreEqual(expectedResponseArray, responseArray);

responseArray = await db.ExecuteForStringArrayResultAsync("LRANGE", new string[] { "myotherlist", "0", "-1" });
expectedResponseArray = new string[] { "three", "one" };
Assert.AreEqual(expectedResponseArray, responseArray);
}

[Test]
public async Task CanUseLMoveWithCancellationTokenGC()
{
Expand Down