diff --git a/libs/server/Lua/LuaRunner.Functions.cs b/libs/server/Lua/LuaRunner.Functions.cs index 23a6295df57..aca94100551 100644 --- a/libs/server/Lua/LuaRunner.Functions.cs +++ b/libs/server/Lua/LuaRunner.Functions.cs @@ -852,7 +852,7 @@ internal int LoadString(nint luaStatePtr) return LuaWrappedError(1, constStrs.InsufficientLuaStackSpace); } - var res = state.LoadString(buff); + var res = state.LoadTextBuffer(buff); if (res != LuaStatus.OK) { state.ClearStack(); diff --git a/libs/server/Lua/LuaRunner.Loader.cs b/libs/server/Lua/LuaRunner.Loader.cs index a9a95ef9e53..a50143935f4 100644 --- a/libs/server/Lua/LuaRunner.Loader.cs +++ b/libs/server/Lua/LuaRunner.Loader.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using System.Text; using KeraLua; @@ -315,7 +314,7 @@ function reset_keys_and_argv(fromKey, fromArgv) recursively_readonly_table(sandbox_env) -- responsible for sandboxing user provided code function load_sandboxed(source) - local rawFunc, err = load(source, nil, nil, sandbox_env) + local rawFunc, err = load(source, nil, 't', sandbox_env) return err, rawFunc end @@ -448,7 +447,7 @@ internal static ReadOnlyMemory PrepareLoaderBlockBytes(HashSet all compilingState.Remove(1); - if (compilingState.LoadString(Encoding.UTF8.GetBytes(finalLoaderBlock)) != LuaStatus.OK) + if (compilingState.LoadTextBuffer(Encoding.UTF8.GetBytes(finalLoaderBlock)) != LuaStatus.OK) { throw new InvalidOperationException("Compiling function should not fail"); } @@ -473,46 +472,5 @@ internal static ReadOnlyMemory PrepareLoaderBlockBytes(HashSet all return newCache.LoaderBlockBytes; } - - /// - /// Take a chunk of Lua code and convert it to binary ops. - /// - /// These ops are faster to load into a runtime than parsing the whole source file again. - /// - internal static byte[] CompileSource(ReadOnlySpan source) - { - // This is equivalent to calling - // - // string.dump(, true) - // - // Which gives us the opcode version of source on the stack - - using var state = new LuaStateWrapper(LuaMemoryManagementMode.Native, null, null); - - state.GetGlobal(LuaType.Table, "string\0"u8); - var pushRes = state.TryPushBuffer("dump"u8); - Debug.Assert(pushRes, "Pushing 'dump' should never fail"); - _ = state.RawGet(LuaType.Function, 1); - - state.Remove(1); - - if (state.LoadString(source) != LuaStatus.OK) - { - // If we're going to fail, just keep the source as is - a future load attempt will fail it too - return source.ToArray(); - } - - state.PushBoolean(true); - - if (state.PCall(2, 1) != LuaStatus.OK) - { - // If we're going to fail, just keep the source as is - a future load attempt will fail it too - return source.ToArray(); - } - - state.KnownStringToBuffer(1, out var ops); - - return ops.ToArray(); - } } } \ No newline at end of file diff --git a/libs/server/Lua/LuaRunner.cs b/libs/server/Lua/LuaRunner.cs index 5b68363b7af..89e26cf7d2f 100644 --- a/libs/server/Lua/LuaRunner.cs +++ b/libs/server/Lua/LuaRunner.cs @@ -328,7 +328,7 @@ public unsafe LuaRunner( throw new GarnetException("Insufficient space in Lua VM for redis version number global"); } - var loadRes = state.LoadBuffer(PrepareLoaderBlockBytes(allowedFunctions, logger).Span); + var loadRes = state.LoadBinaryBuffer(PrepareLoaderBlockBytes(allowedFunctions, logger).Span); if (loadRes != LuaStatus.OK) { if (state.StackTop == 1 && state.Type(1) == LuaType.String) diff --git a/libs/server/Lua/LuaScriptHandle.cs b/libs/server/Lua/LuaScriptHandle.cs index caa9f066dfa..c2793b6e58b 100644 --- a/libs/server/Lua/LuaScriptHandle.cs +++ b/libs/server/Lua/LuaScriptHandle.cs @@ -19,10 +19,14 @@ public sealed class LuaScriptHandle : IDisposable public bool IsDisposed { get; private set; } /// - /// Source (or compiled source) for the associated Lua script. + /// Source for the associated Lua script. /// public ReadOnlyMemory ScriptData { get; } + /// + /// Creates a handle for Lua source. + /// + /// Lua source. public LuaScriptHandle(ReadOnlyMemory scriptData) { ScriptData = scriptData; diff --git a/libs/server/Lua/LuaStateWrapper.cs b/libs/server/Lua/LuaStateWrapper.cs index 48f9fca6203..3148dba374d 100644 --- a/libs/server/Lua/LuaStateWrapper.cs +++ b/libs/server/Lua/LuaStateWrapper.cs @@ -456,20 +456,14 @@ internal bool TrySetGlobal(ReadOnlySpan nullTerminatedGlobalName) } /// - /// This should be used for all LoadBuffers into Lua. - /// - /// Note that this is different from pushing a buffer, as the loaded buffer is compiled and executed. - /// - /// Maintains and to minimize p/invoke calls. + /// Loads Garnet's precompiled loader buffer into Lua. /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal LuaStatus LoadBuffer(ReadOnlySpan buffer) + internal LuaStatus LoadBinaryBuffer(ReadOnlySpan buffer) { AssertLuaStackNotFull(2); - // Note that https://www.lua.org/source/5.4/lauxlib.c.html#luaL_loadbufferx is implemented in terms of - // a PCall, so we don't have to worry about crashes. - var ret = NativeMethods.LoadBuffer(state, buffer); + var ret = NativeMethods.LoadBinaryBuffer(state, buffer); if (ret != LuaStatus.OK) { @@ -486,19 +480,14 @@ internal LuaStatus LoadBuffer(ReadOnlySpan buffer) } /// - /// This should be used for all LoadStrings into Lua. - /// - /// Note that this is different from pushing or loading buffer, as the loaded buffer is compiled but NOT executed. - /// - /// Maintains and to minimize p/invoke calls. + /// Loads an exact-length text buffer into Lua. /// - internal LuaStatus LoadString(ReadOnlySpan buffer) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal LuaStatus LoadTextBuffer(ReadOnlySpan buffer) { AssertLuaStackNotFull(2); - // Note that https://www.lua.org/source/5.4/lauxlib.h.html#luaL_loadbuffer is implemented in terms of - // a PCall, so we don't have to worry about crashes. - var ret = NativeMethods.LoadString(state, buffer); + var ret = NativeMethods.LoadTextBuffer(state, buffer); if (ret != LuaStatus.OK) { diff --git a/libs/server/Lua/NativeMethods.cs b/libs/server/Lua/NativeMethods.cs index 254b6c43e88..7e7e17e17b1 100644 --- a/libs/server/Lua/NativeMethods.cs +++ b/libs/server/Lua/NativeMethods.cs @@ -45,13 +45,6 @@ internal static partial class NativeMethods [UnmanagedCallConv(CallConvs = [typeof(CallConvCdecl)])] private static partial LuaStatus luaL_loadbufferx(lua_State luaState, charptr_t buff, size_t sz, charptr_t name, charptr_t mode); - /// - /// see: https://www.lua.org/manual/5.4/manual.html#luaL_loadstring - /// - [LibraryImport(LuaLibraryName)] - [UnmanagedCallConv(CallConvs = [typeof(CallConvCdecl)])] - private static partial LuaStatus luaL_loadstring(lua_State lua_State, charptr_t buff); - /// /// see: https://www.lua.org/manual/5.4/manual.html#luaL_newstate /// @@ -388,28 +381,32 @@ internal static unsafe void PushBuffer(lua_State luaState, ReadOnlySpan st } /// - /// Push given span to stack, compiles it, and executes it. - /// + /// Loads Garnet's precompiled loader buffer. /// Provided data is copied, and can be reused once this call returns. /// - internal static unsafe LuaStatus LoadBuffer(lua_State luaState, ReadOnlySpan str) + internal static unsafe LuaStatus LoadBinaryBuffer(lua_State luaState, ReadOnlySpan str) { + ReadOnlySpan mode = "b\0"u8; + fixed (byte* ptr = str) + fixed (byte* modePtr = mode) { - return luaL_loadbufferx(luaState, (charptr_t)ptr, (size_t)str.Length, (charptr_t)UIntPtr.Zero, (charptr_t)UIntPtr.Zero); + return luaL_loadbufferx(luaState, (charptr_t)ptr, (size_t)str.Length, (charptr_t)UIntPtr.Zero, (charptr_t)modePtr); } } /// - /// Push given span to stack, and compiles it. - /// + /// Loads an exact-length text buffer. /// Provided data is copied, and can be reused once this call returns. /// - internal static unsafe LuaStatus LoadString(lua_State luaState, ReadOnlySpan str) + internal static unsafe LuaStatus LoadTextBuffer(lua_State luaState, ReadOnlySpan str) { + ReadOnlySpan mode = "t\0"u8; + fixed (byte* ptr = str) + fixed (byte* modePtr = mode) { - return luaL_loadstring(luaState, (charptr_t)ptr); + return luaL_loadbufferx(luaState, (charptr_t)ptr, (size_t)str.Length, (charptr_t)UIntPtr.Zero, (charptr_t)modePtr); } } diff --git a/libs/server/Lua/SessionScriptCache.cs b/libs/server/Lua/SessionScriptCache.cs index 705a0b9deaf..a08208b58e8 100644 --- a/libs/server/Lua/SessionScriptCache.cs +++ b/libs/server/Lua/SessionScriptCache.cs @@ -185,9 +185,9 @@ out ScriptHashKey? digestOnHeap try { - var compiledSource = LuaRunner.CompileSource(source); + var scriptData = luaScriptHandle?.ScriptData ?? source.ToArray(); - runner = new LuaRunner(memoryManagementMode, memoryLimitBytes, logMode, allowedFunctions, compiledSource, storeWrapper.serverOptions.LuaTransactionMode, processor, scratchBufferNetworkSender, storeWrapper.redisProtocolVersion, logger); + runner = new LuaRunner(memoryManagementMode, memoryLimitBytes, logMode, allowedFunctions, scriptData, storeWrapper.serverOptions.LuaTransactionMode, processor, scratchBufferNetworkSender, storeWrapper.redisProtocolVersion, logger); // If compilation fails, an error is written out if (runner.CompileForSession(session)) @@ -203,7 +203,7 @@ out ScriptHashKey? digestOnHeap ScriptHashKey storeKeyDigest = new(into); digestOnHeap = storeKeyDigest; - luaScriptHandle ??= new(compiledSource); + luaScriptHandle ??= new(scriptData); scriptCache.Add(storeKeyDigest, (runner, luaScriptHandle)); // On first script load, register for timeout notifications diff --git a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs index 55d3f728887..e66d85455e8 100644 --- a/test/standalone/Garnet.test.scripting/LuaScriptTests.cs +++ b/test/standalone/Garnet.test.scripting/LuaScriptTests.cs @@ -16,6 +16,7 @@ using System.Threading.Tasks; using Garnet.common; using Garnet.server; +using KeraLua; using NUnit.Framework; using NUnit.Framework.Legacy; using StackExchange.Redis; @@ -1027,6 +1028,64 @@ public void MultiSessionScriptFlush() ClassicAssert.True(exc2.Message.StartsWith("NOSCRIPT ")); } + [Test] + public void ScriptInputsRequireTextSource() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + var binaryChunk = CompileChunk("return 1"u8); + + var evalException = ClassicAssert.Throws(() => db.Execute("EVAL", [binaryChunk, 0])); + StringAssert.Contains("binary chunk", evalException.Message); + + var hash = Convert.ToHexString(SHA1.HashData(binaryChunk)).ToLowerInvariant(); + var loadException = ClassicAssert.Throws(() => db.Execute("SCRIPT", ["LOAD", binaryChunk])); + StringAssert.Contains("binary chunk", loadException.Message); + var exists = (RedisResult[])db.Execute("SCRIPT", ["EXISTS", hash]); + ClassicAssert.AreEqual(0, (int)exists[0]); + } + + private static byte[] CompileChunk(ReadOnlySpan source) + { + using var state = new LuaStateWrapper(LuaMemoryManagementMode.Native, null, null); + + state.GetGlobal(LuaType.Table, "string\0"u8); + ClassicAssert.True(state.TryPushBuffer("dump"u8)); + _ = state.RawGet(LuaType.Function, 1); + state.Remove(1); + ClassicAssert.AreEqual(LuaStatus.OK, state.LoadTextBuffer(source)); + state.PushBoolean(true); + ClassicAssert.AreEqual(LuaStatus.OK, state.PCall(2, 1)); + state.KnownStringToBuffer(1, out var chunk); + + return chunk.ToArray(); + } + + [Test] + public void HostInsertedScriptSource() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + var source = "return 2"u8; + var hash = Convert.ToHexString(SHA1.HashData(source)).ToLowerInvariant(); + var digest = GC.AllocateUninitializedArray(SessionScriptCache.SHA1Len, pinned: true); + _ = Encoding.ASCII.GetBytes(hash, digest); + + ClassicAssert.True(server.Provider.StoreWrapper.storeScriptCache.TryAdd(new ScriptHashKey(digest), new LuaScriptHandle(source.ToArray()))); + ClassicAssert.AreEqual(2, (int)db.Execute("EVALSHA", hash, 0)); + } + + [Test] + public void EvalUsesFullSourceLength() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + var source = Encoding.UTF8.GetBytes("return 1\0return 2"); + + var exc = ClassicAssert.Throws(() => db.Execute("EVAL", [source, 0])); + StringAssert.StartsWith("Compilation error:", exc.Message); + } + [Test] public void CrossSessionEvalScriptCaching() {