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

Fix certain arguments not being correctly escaped or causing batch syntax error #83

Merged
merged 1 commit into from
Jan 23, 2018

Conversation

satazor
Copy link
Contributor

@satazor satazor commented Nov 11, 2017

More specifically:

This was resolved by using ^ to escape all meta chars.
Additionally, double escaping was necessary for cmd-shim files located in node_modules./bin/.

Also, this commit was a major overhaul:

  • Upgrade tooling
  • Upgrate project to es6 (node v4)
  • Fix commands as posix unix relatixe paths not working correctly
  • Fix options argument being mutated
  • Improve compliance with node's ENOENT errors
  • Improve detection of node's shell option support
  • Migrate project to moxystudio

BREAKING CHANGE: remove support for older nodejs versions, only node >= 4 is supported

@satazor
Copy link
Contributor Author

satazor commented Nov 15, 2017

This PR is blocked until a bug in npm is solved regarding executables in node_modules/.bin.

@cspotcode
Copy link

@satazor: I'm curious, what's the NPM bug? Do you have a link to their issue tracker?

@satazor
Copy link
Contributor Author

satazor commented Dec 20, 2017

I can't disclose the bug because it's also a security vulnerability.

I can email you though.

@cspotcode
Copy link

Sure, I'm [email protected]. Thanks.

@andrew-c-tran
Copy link

Any updates to this PR? I've traced a webpack bundling warning in my electron app back to this library attempting to require spawn-sync. This PR by removing node v0.10 support, that should correct the build warning.

@satazor satazor changed the title Major overhaul. Fix certain arguments not being correctly escaped or causing batch syntax error Jan 22, 2018
@satazor
Copy link
Contributor Author

satazor commented Jan 22, 2018

I've updated the PR. I will merge this once it becomes green and release 6.0.0.

@codecov
Copy link

codecov bot commented Jan 22, 2018

Codecov Report

Merging #83 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master     #83   +/-   ##
======================================
  Coverage    98.8%   98.8%           
======================================
  Files           7       7           
  Lines         167     167           
  Branches       35      35           
======================================
  Hits          165     165           
  Misses          2       2
Impacted Files Coverage Δ
lib/util/escape.js 100% <100%> (ø) ⬆️
lib/util/readShebang.js 100% <100%> (ø) ⬆️
test/util/run.js 100% <100%> (ø) ⬆️
lib/util/resolveCommand.js 100% <100%> (ø) ⬆️
index.js 100% <100%> (ø) ⬆️
lib/parse.js 96.15% <100%> (ø) ⬆️
lib/enoent.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d050aef...900cf10. Read the comment docs.

@satazor satazor force-pushed the fix-arg-escape branch 3 times, most recently from f575053 to 2381dfe Compare January 22, 2018 23:26
…ch syntax error

More specifically:
- Fix a bug that made it impossible to escape an argument that contained quotes followed by `>` or other special chars, e.g.: `"foo|bar"`, fixes #82
- Fix a bug were a command containing `%x%` would be replaced with the contents of the `x` environment variable, fixes #51

This was resolved by using `^` to escape all meta chars.
Additionally, double escaping was necessary for cmd-shim files located in `node_modules./bin/`.

Also, this commit was a major overhaul:
- Upgrade tooling
- Upgrate project to es6 (node v4)
- Fix commands as posix unix relatixe paths not working correctly
- Fix `options` argument being mutated
- Improve compliance with node's ENOENT errors
- Improve detection of node's shell option support
- Migrate project to moxystudio

BREAKING CHANGE: remove support for older nodejs versions, only node >= 4 is supported

Fixes #82, #51
@satazor
Copy link
Contributor Author

satazor commented Jan 22, 2018

@cspotcode I've fixed the issue regarding the npm cmd-shim by double-escaping the windows shell meta chars. I've tested this exhaustively and it's working correctly!

@satazor satazor merged commit e77b8f2 into master Jan 23, 2018
@satazor satazor deleted the fix-arg-escape branch January 23, 2018 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad escaping when an argument has quotes followed by a pipe
3 participants