Fix --restrict-to-path with spaces and forward slashes on Windows (#82, #83) - #84
Merged
Conversation
…dows The npm launcher spawned Java with shell:true on Windows, where Node concatenates argv into an unquoted command string. Any argument containing spaces was word-split before Java received it, breaking command parsing. Spawn Java with the argument array and no shell so each argument reaches the JVM verbatim. findJava() already returns a concrete executable, so no shell is needed for resolution.
Scoping a push or pull to a path (for example a single channel or group) with --restrict-to-path selected nothing on Windows when the path was given with forward slashes: the run completed without error, but no files were written. The value is matched against generated file paths with String/startsWith (mirthsync.xml/serialize-node and mirthsync.actions/local-locs). The required prefix was built from the raw restrict-to-path, while the file paths use the platform File/separator, so a forward-slash path never matched the backslash file paths on Windows and the filter excluded everything. Normalize the restrict-to-path separators to File/separator when parsing the option so the prefix matches regardless of which slash style was supplied. Add tests covering the normalization and a forward-slash pull.
Complements the npm main-spawn fix already on this branch: - pkg/npm/bin/mirthsync.js: also spawn the java-version detection without a shell, and guard the result on !error so a missing java on Windows is not misread as present (a shell launched for a missing command still writes to stderr, which the old check treated as "java found"). - pkg/mirthsync.sh / pkg/mirthsync.bat: quote the jar path and "$@" so a space-containing install path or argument survives in the non-npm launchers.
capture-pre-pull-local-files decides whether an API is rooted at the target by stripping a trailing separator from its local-path and comparing to the target. The strip used a hardcoded "/", so on Windows the trailing File/separator (backslash) was not removed, the comparison failed, and root-level APIs (resources, configuration-map, backup) fell into the all-files-seq branch -- capturing every file under the target root (including .git and other user-maintained files) as orphan-delete candidates. With --delete-orphaned those unmanaged files could be deleted. Compare against File/separator instead. No-op on POSIX (separator is already "/"); on Windows, root-level APIs now capture only their managed file. Update the delete-orphaned test mock to return a File/separator-terminated local-path (the hardcoded "/" mock would fail on Windows once the production check uses File/separator).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two related Windows bugs with
--restrict-to-path:mirthsyncon Windows failed to parse commands when--restrict-to-pathcontained spaces (even quoted): the npm wrapper spawned Java withshell:trueon Windows, so Node re-joined argv into one unquoted command string and the shell word-split any spaced argument before Java saw it.-r Channels/<group>/<channel>, defaultcodemode) completed with exit 0 but wrote no file on Windows. Root cause is the same separator mismatch: the forward-slash restrict path never matched the backslash file paths that the push/pull filter compares against viaString/startsWith. (The reporter's-m items+.xmlworkaround was incidental — once separators are normalized, plainstartsWithalready matches<channel>.xml.)Approach
Built on @JosephTelford's Windows-developed-and-tested fixes (commits
d010ef4,6ac4fb7), plus two small, separately-scoped additions.d010ef46ac4fb7-rseparators toFile/separatorso forward-slash restrict paths match on Windows — the #83 root cause; keeps the existingstartsWithmatcher74e5043findJavadetection + guard on!error; quote the jar path and"$@"inmirthsync.sh/mirthsync.batfor non-npm users4265248capture-pre-pull-local-filesto compare againstFile/separatorinstead of a hardcoded"/", so Windows--delete-orphanedno longer captures (and risks deleting) unmanaged files like.gitOn-disk format is unchanged
This deliberately does not touch where files are written. Separator normalization is applied to
-ronly (a pure filter —restrict-to-pathnever constructs a write path),-tis left as-is, and the existingstartsWithmatcher is kept. Pulled trees are byte-for-byte identical to 3.5.2 (verified by diffing full and restricted pulls).Testing
lein testgreen locally: 41 tests, 351 assertions, 0 failures — including the live OIE 4.5.2 integration suite.d010ef4,6ac4fb7) are already green on thewindows-latestCI job.Closes #82
Closes #83