fix: handle special characters in postinstall script#2649
Open
aloisklink wants to merge 2 commits intomswjs:mainfrom
Open
fix: handle special characters in postinstall script#2649aloisklink wants to merge 2 commits intomswjs:mainfrom
postinstall script#2649aloisklink wants to merge 2 commits intomswjs:mainfrom
Conversation
`execSync` is dangerous, since if we're in a folder that has spaces in it (e.g. 'My Documents/library/node_modules/msw'), the space will cause this command to fail. Using `execFileSync` instead fixes this issue.
The normal Node.JS function to create a new Node.JS process is [`child_process.fork`][1]. Since it doesn't have a synchronous API, we can't use it, but it does default to using the same Node.JS version as the calling process by using `process.execPath`: > By default, `child_process.fork()` will spawn new Node.js instances > using the [`process.execPath`][2] of the parent process. Considering that our postinstall script in `package.json` uses `node`, this probably won't make a difference, but it's still good practice. [1]: https://nodejs.org/docs/latest-v24.x/api/child_process.html#child_processforkmodulepath-args-options [2]: https://nodejs.org/docs/latest-v24.x/api/process.html#processexecpath
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
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.
Improve handling of shell metacharacters in folder names in the
postinstallscript by replacingexecSyncwithexecFileSync.For example, spaces in folder names are now allowed:
Current behaviour
Installing
mswwith amsw.workerDirectoryset in yourpackage.jsonfile fails if the folder structure has shell special characters (e.g. spaces) in it.Proposed
Installing
mswwith amsw.workerDirectoryset in yourpackage.jsonfile works, even if your folder structure has spaces in it.Future work
The
config/scripts/patch-ts.jsscript is still vulnerable to this issue too, but since that only affects developers of this library, not users, I've ignored it for this PR:msw/config/scripts/patch-ts.js
Lines 91 to 96 in e0a4643