Closed michaelhogg closed 9 years ago
Oh wow! This is amazing! Thank you so much!
Could you just remove the file name from the commit messages, so they are more inline with the existing ones?
You're welcome :)
I've just rebased the branch and removed the file name prefixes from the commit messages.
Many thanks @shime for creating this nice little library :)
In this PR, I've made the following improvements:
README.md
: improve documentation of optionsindex.js
: pass newError
to callback if no audio file is specifiedindex.js
: improve file existence checkstderr
for checking file existence or for detecting whether a playback error occurred. Some players (eg:mplayer
) output non-fatal warnings tostderr
(eg: information about missing codecs). A much safer solution for checking file existence is to usefs.statSync()
.index.js
: remove unused variables:util
,self
index.js
: function parameter variables should not be redefinedindex.js
: remove unnecessary check for errorerr
here. In thechild_process.exec()
callback,err
will benull
on success, so we can simply passerr
to the user's callback, and let the user check for errors.index.js
: remove uselessreturn
inexec()
callbackchild_process
module, and any return value is simply discarded (see the source code here).index.js
: fix command injection vulnerabilitychild_process.exec()
causes a command injection vulnerability. For example:player.play('foo.mp3; rm -rf ~');
The solution is to usechild_process.execFile()
, which takes an array of arguments and passes them directly to the player. For more details, see: Avoiding Command Injection in Node.js.tests.js
: update proxiedchild_process
method toexecFile()
tests.js
: updatespy.calledWith()
to matchchild_process.execFile()
tests.js
: mock MP3 file is required when testing for missing playerstests.js
: remove unnecessary call tospy.callsArg()
child_process.execFile()
callback to be called.