search

voxpupuli / puppet-staging

⛔️ Deprecated in favor of puppet-archive
https://forge.puppet.com/puppet/archive
Apache License 2.0
51 stars 109 forks source link

Handle URLs with parameters more sensibly. #15

Closed ojacobson closed 10 years ago

ojacobson commented 10 years ago

Shellquote is the closest thing to array-of-arguments exec calls puppet supports. As far as I've ever been able to test, it correctly handles quoting for the shells Puppet uses for exec{} resources. Using it to construct curl/wget commands means that they correctly download, for example, Jetty:

http://eclipse.org/downloads/download.php?file=/jetty/stable-8/dist/jetty-distribution-8.1.14.v20131031.tar.gz&r=1

Without shellquote, the exec resources see the embedded & near the end and generate two shell commands (the second of which is r=1). With shellquote, the URL is kept as a single argument.

I had a look at retrofitting the command lines in the extract.pp manifest, but it's not so worth it there, I think.

nanliu commented 10 years ago

I haven't tried, but could we just shell quote the source so there's no backward breakage?

$shell_esc_source = shellquote($source)
$http_get        = "curl ${curl_option} -f -L -o ${name} ${shell_esc_source}"
ojacobson commented 10 years ago

Yeah, that's an option. I habitually use shellquote for the entire command, because I usually want to reason about argv as an array, not as a sequence of shell tokens, but breaking compat just for that might be selfish. I'll have a look.

ojacobson commented 10 years ago

Better? This feels a lot more likely to surprise users (imagine a shell-unfriendly target filename, or username) but less likely to break existing manifests.