Fix LoadModuleCS parsing for module paths containing spaces (#1951)

[badrishc]

Problem

--loadmodulecs (and the LoadModuleCS configuration setting) failed to load modules whose path contains spaces. Each module spec was split on every space (moduleCS.Split(' ')), so a path such as C:\Users\John Doe\Garnet Modules\MyModule.dll was broken into a bogus path plus spurious arguments and failed to load. The module file-path option validator had the same bug (filePathArg.Split(' ')[0]).

Fixes #1951.

Fix

Added a shared parser ModuleUtils.TryParseModuleSpec(spec, out modulePath, out moduleArgs) that delimits the module path from its arguments as follows:

1. If the spec starts with a double quote, the path is the quoted text and the remainder (space-separated) are the arguments.
2. Otherwise, the path is the text up to and including the first space-separated token ending in a module assembly extension (.dll/.exe), preserving spaces within the path; remaining tokens are arguments.
3. If no such token is found (e.g. a directory path), the entire spec is treated as the path.

Both GarnetServer.LoadModules and ModuleFilePathValidationAttribute now use this single parser, so the load path and validation path stay consistent.

This makes the exact reported command work with no extra escaping:

GarnetServer.exe --loadmodulecs "C:\Users\John Doe\Garnet Modules\MyModule.dll"

Backward compatibility is preserved: path arg0 arg1 (no spaces in path) still parses into a path plus arguments.

Tests

• ModuleUtilsTests — 14 parser cases: plain paths, paths with spaces, args, quoted paths, .exe, trimming, directories, and empty/whitespace input.
• RespModuleTests (RespModuleAdditionalTests) — 2 end-to-end tests that load a module from a path containing spaces via --loadmodulecs, with and without arguments.
• Verified no regressions in the full module suite (26 tests) and the config validator tests (ImportExportConfigLocal, ImportExportRedisConfigLocal), and dotnet format --verify-no-changes is clean.

Docs

Documented the module loading syntax (spaces-in-path support and quoting) in website/docs/extensions/module.md.

[Copilot]

Pull request overview

Fixes module-loading via --loadmodulecs / LoadModuleCS when module paths contain spaces by centralizing spec parsing in ModuleUtils.TryParseModuleSpec, and updates both runtime loading and option validation to use it. Adds targeted unit and end-to-end tests plus documentation for the supported syntax.

Changes:

• Add ModuleUtils.TryParseModuleSpec to correctly split <module-path> [args...] while preserving spaces in paths (including quoted paths).
• Use the shared parser in GarnetServer.LoadModules and in ModuleFilePathValidationAttribute to keep load/validate behavior consistent.
• Add parser unit tests + end-to-end tests loading a module from a space-containing path, and document the supported syntax.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
website/docs/extensions/module.md	Documents module loading syntax, including paths-with-spaces and quoting.
test/standalone/Garnet.test.scripting/RespModuleTests.cs	Adds end-to-end coverage for loading modules from paths containing spaces (with/without args).
test/standalone/Garnet.test.scripting/ModuleUtilsTests.cs	Adds unit tests for the new module-spec parser.
libs/server/Module/ModuleUtils.cs	Introduces shared module-spec parsing logic for --loadmodulecs-style strings.
libs/host/GarnetServer.cs	Switches module loading to use TryParseModuleSpec output (path + args).
libs/host/Configuration/OptionsValidators.cs	Switches LoadModuleCS path validation to use TryParseModuleSpec for correct handling of spaces.

[badrishc]

Ran an additional round of automated review (GPT-5.6 and Gemini 3.1 Pro). Addressed the substantive findings in dfcf0fd:

• Robust path/arg delimiting (Gemini): The parser now resolves the module path against the filesystem — the longest prefix of the specification that exists as a file or directory is taken as the path, and the remaining tokens become arguments. This correctly handles a path whose directory name contains a .dll/.exe fragment (e.g. .../plugins.dll cache/Module.dll) and a directory module given a .dll-ending argument, both of which the pure extension heuristic mis-split. The extension heuristic remains as a fallback for paths that don't exist at parse time.
• Case-insensitive assembly extensions (GPT): FileUtils.TryGetFiles now matches .dll/.exe case-insensitively, so an uppercase .DLL/.EXE module path loads instead of resolving to zero assemblies.
• No startup crash on empty assembly list (GPT): LoadModules now guards against an empty loaded-assembly list before indexing [0], logging a clear error and continuing instead of throwing at startup.

New tests: a filesystem-probing parser unit test, plus integration tests loading a module from a .dll-fragment directory name and from an uppercase .DLL path. All module tests (35), custom-command/REGISTERCS tests (36), and config-validation tests pass; dotnet format is clean.

[badrishc]

Follow-up on the earlier "case-insensitive assembly extensions" change (from the GPT review round): reverted in a399ea9.

FileUtils.TryGetFiles is shared by REGISTERCS and module loading, and on a case-sensitive filesystem (Linux) making extension matching case-insensitive changes directory-scan semantics — it could newly collect uppercase-extension files and even fail an otherwise-valid load (e.g. a BadImageFormatException on an unrelated *.DLL in an allowlisted directory). It's also outside this PR's scope.

Uppercase-extension loading was never supported (pre-PR TryGetFiles was already case-sensitive), and the startup crash the change was paired with is independently prevented by the empty-assembly guard in LoadModules. So FileUtils.cs is now back to its main version (no net change), and the parser's extension heuristic is made case-sensitive (StringComparison.Ordinal) for consistency. Users on a case-sensitive filesystem should simply match the extension case (.dll/.exe) in their paths.

The filesystem-probing path resolution (which handles real files per-OS) and the empty-assembly guard are retained.

[badrishc]

Simplified the module-spec parser to a deterministic rule (commit 458589a), addressing the indeterminacy/exploitability concern.

Instead of probing the filesystem / guessing the path-vs-argument boundary, parsing now works like a command line: the first space-separated token is the module path and the rest are arguments, unless the path is wrapped in double quotes (in which case the quoted text is the path and may contain spaces). A module path containing spaces must be quoted, e.g.:

--loadmodulecs "\"/path/to/My Modules/My Module.dll\" arg0 arg1"

This removes the filesystem probing from the parser entirely (net −156 lines) and makes behavior fully deterministic and input-only. Retained: the empty-assembly guard in LoadModules, the invalid-quoted-spec validation, and case-sensitive extension matching. Tests and website/docs/extensions/module.md updated accordingly; parser/module/config tests pass and dotnet format is clean.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.