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

Replacing Newtonsoft.Json dependency with System.Text.Json + JsonPath.Net in GarnetJSON #815

Merged
merged 17 commits into from
Nov 21, 2024
2 changes: 1 addition & 1 deletion Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<PackageVersion Include="BenchmarkDotNet" Version="0.14.0" />
<PackageVersion Include="BenchmarkDotNet.Diagnostics.Windows" Version="0.13.12" />
<PackageVersion Include="CommandLineParser" Version="2.9.1" />
<PackageVersion Include="Newtonsoft.Json" Version="13.0.3" />
<PackageVersion Include="JsonPath.Net" Version="1.1.6" />
badrishc marked this conversation as resolved.
Show resolved Hide resolved
<PackageVersion Include="NLua" Version="1.7.3" />
<PackageVersion Include="NUnit" Version="4.1.0" />
<PackageVersion Include="NUnit3TestAdapter" Version="4.6.0" />
Expand Down
24 changes: 18 additions & 6 deletions libs/common/FileUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.IO;
using System.Linq;
using System.Reflection;
using System.Runtime.Loader;
using System.Security;
using System.Text;
using System.Text.RegularExpressions;
Expand All @@ -22,18 +23,20 @@ public static class FileUtils
/// </summary>
/// <param name="paths">Paths to files or directories</param>
/// <param name="extensions">Extensions to match files (defaults to any)</param>
/// <param name="ignoreFileNames">File names to ignore</param>
/// <param name="searchOption">In case path is a directory, determines whether to search only top directory or all subdirectories</param>
/// <param name="files">Files that match the extensions</param>
/// <param name="errorMessage">Error message, if applicable</param>
/// <returns>True if successfully enumerated all directories</returns>
public static bool TryGetFiles(IEnumerable<string> paths, out IEnumerable<string> files, out string errorMessage, string[] extensions = null, SearchOption searchOption = SearchOption.TopDirectoryOnly)
public static bool TryGetFiles(IEnumerable<string> paths, out string[] files, out string errorMessage, string[] extensions = null, IEnumerable<string> ignoreFileNames = null, SearchOption searchOption = SearchOption.TopDirectoryOnly)
{
var validExtensionPattern = "^\\.[A-Za-z0-9]+$";
var anyExtension = false;

errorMessage = string.Empty;
var sbErrorMessage = new StringBuilder();
files = null;
var ignoreFiles = ignoreFileNames == null ? new HashSet<string>() : [.. ignoreFileNames];

string extensionPattern = null;
if (extensions == null || extensions.Length == 0)
Expand Down Expand Up @@ -63,7 +66,8 @@ public static bool TryGetFiles(IEnumerable<string> paths, out IEnumerable<string
{
if (File.Exists(path))
{
if (anyExtension || Regex.IsMatch(path, extensionPattern))
if ((anyExtension || Regex.IsMatch(path, extensionPattern)) &&
!ignoreFiles.Contains(Path.GetFileName(path)))
{
tmpFiles.Add(path);
}
Expand Down Expand Up @@ -92,7 +96,8 @@ public static bool TryGetFiles(IEnumerable<string> paths, out IEnumerable<string

foreach (var filePath in filePaths)
{
if (anyExtension || Regex.IsMatch(filePath, extensionPattern))
if ((anyExtension || Regex.IsMatch(filePath, extensionPattern)) &&
!ignoreFiles.Contains(Path.GetFileName(path)))
{
tmpFiles.Add(filePath);
}
Expand All @@ -103,7 +108,7 @@ public static bool TryGetFiles(IEnumerable<string> paths, out IEnumerable<string
if (!errorMessage.IsNullOrEmpty())
return false;

files = tmpFiles;
files = [.. tmpFiles];
return true;
}

Expand Down Expand Up @@ -144,18 +149,25 @@ public static bool TryLoadAssemblies(IEnumerable<string> assemblyPaths, out IEnu
loadedAssemblies = null;

var tmpAssemblies = new List<Assembly>();

foreach (var path in assemblyPaths)
{
Assembly assembly;
try
{
var data = File.ReadAllBytes(path);
assembly = Assembly.Load(data);
assembly = AssemblyLoadContext.Default.LoadFromAssemblyPath(path);
}
catch (Exception ex) when (ex is IOException || ex is UnauthorizedAccessException ||
ex is NotSupportedException || ex is BadImageFormatException ||
ex is SecurityException)
{
if (ex is FileLoadException && ex.Message == "Assembly with same name is already loaded")
{
var assemblyName = AssemblyName.GetAssemblyName(path).Name;
tmpAssemblies.Add(AssemblyLoadContext.Default.Assemblies.First(a => a.GetName().Name == assemblyName));
continue;
}

sbErrorMessage.AppendLine($"Unable to load assembly from path: {path}. Error: {ex.Message}");
continue;
}
Expand Down
26 changes: 20 additions & 6 deletions libs/server/Module/ModuleUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,39 @@ namespace Garnet.server
{
public class ModuleUtils
{
/// <summary>
/// Load assemblies from specified binary paths
/// </summary>
/// <param name="binaryPaths">Source paths for assemblies (can be either files or directories)</param>
/// <param name="allowedExtensionPaths">List of allowed paths for loading assemblies from</param>
/// <param name="allowUnsignedAssemblies">True if loading unsigned assemblies is allowed</param>
/// <param name="loadedAssemblies">Loaded assemblies</param>
/// <param name="errorMessage">Error message</param>
/// <param name="ignoreFileNames">File names to ignore (optional)</param>
/// <param name="searchOption">In case path is a directory, determines whether to search only top directory or all subdirectories</param>
/// <param name="ignoreAssemblyLoadErrors">False if method should return an error when at least one assembly was not loaded correctly (false by default)</param>
/// <returns></returns>
public static bool LoadAssemblies(
IEnumerable<string> binaryPaths,
string[] allowedExtensionPaths,
bool allowUnsignedAssemblies,
out IEnumerable<Assembly> loadedAssemblies,
out ReadOnlySpan<byte> errorMessage)
out ReadOnlySpan<byte> errorMessage,
string[] ignoreFileNames = null,
SearchOption searchOption = SearchOption.AllDirectories,
bool ignoreAssemblyLoadErrors = false)
{
loadedAssemblies = null;
errorMessage = default;

// Get all binary file paths from inputs binary paths
if (!FileUtils.TryGetFiles(binaryPaths, out var files, out _, [".dll", ".exe"], SearchOption.AllDirectories))
if (!FileUtils.TryGetFiles(binaryPaths, out var binaryFiles, out _, [".dll", ".exe"], ignoreFileNames, SearchOption.AllDirectories))
{
errorMessage = CmdStrings.RESP_ERR_GENERIC_GETTING_BINARY_FILES;
return false;
}

// Check that all binary files are contained in allowed binary paths
var binaryFiles = files.ToArray();
if (allowedExtensionPaths != null)
{
if (binaryFiles.Any(f =>
Expand All @@ -46,7 +60,7 @@ public static bool LoadAssemblies(
// If necessary, check that all assemblies are digitally signed
if (!allowUnsignedAssemblies)
{
foreach (var filePath in files)
foreach (var filePath in binaryFiles)
{
using var fs = File.OpenRead(filePath);
using var peReader = new PEReader(fs);
Expand All @@ -61,7 +75,7 @@ public static bool LoadAssemblies(
}

var publicKeyBytes = metadataReader.GetBlobBytes(assemblyPublicKeyHandle);
if (publicKeyBytes == null || publicKeyBytes.Length == 0)
if (publicKeyBytes.Length == 0)
{
errorMessage = CmdStrings.RESP_ERR_GENERIC_ASSEMBLY_NOT_SIGNED;
return false;
Expand All @@ -70,7 +84,7 @@ public static bool LoadAssemblies(
}

// Get all assemblies from binary files
if (!FileUtils.TryLoadAssemblies(binaryFiles, out loadedAssemblies, out _))
if (!FileUtils.TryLoadAssemblies(binaryFiles, out loadedAssemblies, out _) && !ignoreAssemblyLoadErrors)
{
errorMessage = CmdStrings.RESP_ERR_GENERIC_LOADING_ASSEMBLIES;
return false;
Expand Down
23 changes: 22 additions & 1 deletion libs/server/Resp/AdminCommands.cs
Original file line number Diff line number Diff line change
Expand Up @@ -484,8 +484,29 @@ private bool NetworkModuleLoad(CustomCommandManager customCommandManager)
for (var i = 0; i < moduleArgs.Length; i++)
moduleArgs[i] = parseState.GetArgSliceByRef(i + 1).ToString();

var errorMsg = ReadOnlySpan<byte>.Empty;

var binPath = Path.GetDirectoryName(modulePath);
var moduleFileName = Path.GetFileName(modulePath);

// Load dependencies from the module path
if (Directory.Exists(binPath) && !ModuleUtils.LoadAssemblies([binPath],
storeWrapper.serverOptions.ExtensionBinPaths,
storeWrapper.serverOptions.ExtensionAllowUnsignedAssemblies, out _, out errorMsg, [moduleFileName],
SearchOption.TopDirectoryOnly, true))
{
if (!errorMsg.IsEmpty)
{
while (!RespWriteUtils.WriteError(errorMsg, ref dcurr, dend))
SendAndReset();
}

return true;
}

// Load the module path
if (ModuleUtils.LoadAssemblies([modulePath], storeWrapper.serverOptions.ExtensionBinPaths,
storeWrapper.serverOptions.ExtensionAllowUnsignedAssemblies, out var loadedAssemblies, out var errorMsg))
storeWrapper.serverOptions.ExtensionAllowUnsignedAssemblies, out var loadedAssemblies, out errorMsg))
{
Debug.Assert(loadedAssemblies != null);
var assembliesList = loadedAssemblies.ToList();
Expand Down
5 changes: 3 additions & 2 deletions playground/GarnetJSON/GarnetJSON.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@
</PropertyGroup>

<PropertyGroup>
<CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Newtonsoft.Json" />
<ItemGroup>
<PackageReference Include="JsonPath.Net" />
</ItemGroup>

<ItemGroup>
Expand Down
3 changes: 2 additions & 1 deletion playground/GarnetJSON/JsonCommands.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ public override bool Reader(ReadOnlyMemory<byte> key, ref ObjectInput input, IGa

var offset = 0;
var path = CustomCommandUtils.GetNextArg(ref input, ref offset);
var strPath = path.IsEmpty ? "$" : Encoding.UTF8.GetString(path);

if (((JsonObject)value).TryGet(Encoding.UTF8.GetString(path), out var result, logger))
if (((JsonObject)value).TryGet(strPath, out var result, logger))
CustomCommandUtils.WriteBulkString(ref output, Encoding.UTF8.GetBytes(result));
else
WriteNullBulkString(ref output);
Expand Down
Loading
Loading