Wrap Installation in Function to Prevent Executing Malformed Code

[qw3rtman]

Installing npm via curl -L https://www.npmjs.com/install.sh | sh could be potentially dangerous if the script does not fully download.

For example (and perhaps this case doesn't exactly match the npm install script), suppose the script contains a line that copies npm to the user's $PATH: cp npm /usr/local/bin/npm. If the script is not fully downloaded in its current layout, it could execute `cp npm /usr', effectively overwriting the entire operating system.

Wrapping the entire installation in a function and then calling the function at the end of the script ensures that the entire installation script is downloaded before executing anything and has no drawbacks nor does it affect the current installation method.

[othiym23]

This seems like a good idea on its face, but back when he was writing this script, @isaacs tested it under every version of POSIX-like-ish sh that he knew of, and there are a lot of quirks that aren't obvious if you've only used bash or dash to do shell script development. @zkat is going to take a look at this and maybe extract from @isaacs any gotchas he encountered around this.

[zkat]

Hey, @qw3rtman!

I talked to @isaacs and spent some time checking the script against known incompatibilities of function definitions. I think it's better to pass on this particular PR because even this benign-looking change is going to cause a pretty serious set of cross-shell incompatibilities. @isaacs put quite a bit of effort originally into making sure that was all set, and there's already some examples from http://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Shell-Functions.html#Shell-Functions where this would get violated. Even if we edited the script to work around those specific issues, I'm afraid that it would make it even harder in the future to make sure this is all compatible.

That said, I agree with the purpose of this! The usual curl -L .... | sh incantation that npm uses (along with many other projects!) has some tricky issues around it as far as incomplete downloads go -- the best recommendation there is likely to stop recommending that users pipe things straight to sh.

Let me know if you have any other thoughts. I'm going to close this now. Real, genuine thanks for the contribution, and I'm sorry it didn't seem like something we could actually land. I would've loved to have this extra safety net. :)