kevva / bin-wrapper

Binary wrapper that makes your programs seamlessly available as local dependencies
MIT License
152 stars 65 forks source link

Fix critical error when space in bin path #43

Closed marob closed 7 years ago

marob commented 8 years ago

The bin-version-check module delegates version extraction to the bin-version module: https://github.com/sindresorhus/bin-version-check/blob/master/index.js#L16

The bin-version module extracts the bin version with childProcess.exec(bin + ' --version', ...): https://github.com/sindresorhus/bin-version/blob/master/index.js#L6

childProcess.exec expects its first argument to be "The command to run, with space-separated arguments": https://nodejs.org/api/child_process.html#child_process_child_process_exec_command_options_callback

Consequently, before my fix, if a space is present in the path to the bin, everything after the first space is interpreted as arguments. And the bin correspond to the first part of the path (before the first space), which is NOT ok.

shinnn commented 8 years ago

@marob Can you submit this PR to https://github.com/sindresorhus/bin-version/pulls instead? I think that's more proper way to fix this problem.

cc: @sindresorhus

marob commented 8 years ago

Here is the corresponding PR: https://github.com/sindresorhus/bin-version/pull/6

I'm not convinced it should be fixed in bin-version, but I'll let you and @sindresorhus decide.

shinnn commented 8 years ago

@marob Thanks.

sindresorhus commented 8 years ago

Yeah, should be fixed in bin-version. Never thought of the use-case that someone would supply an absolute path.

marob commented 8 years ago

OK, I've updated my PR on bin-version to try not to break any module while fixing the issue. Meanwhile, I'm keeping this PR (on bin-wrapper) if fixing the issue in bin-version proves to be impossible.

sindresorhus commented 7 years ago

This was fixed in bin-version.