Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Problem with embedded spaces in find command in deploy:finalize_update #300

Closed
niall opened this Issue · 10 comments

6 participants

@niall

A bug appears to have been introduced with 2.13.5 such that a find command generated by deploy:finalize_update which should execute on several directories instead tries to execute on ONE directory whose name is the names of the several directories concatenated with spaces.

With 2.13.4 this is reported by cap:

  • executing "find /some/path/releases/20121108132920/public/images /some/path/releases/20121108132920/public/stylesheets /some/path/releases/20121108132920/public/javascripts -exec touch -t 201211081329.21 {} ';'; true"

    servers: ["my.server.com"]

    [my.server.com] executing command

    command finished in 667ms

    triggering after callbacks for `deploy:finalize_update'

whereas with 2.13.5 this is what's reported:

  • executing "find /some/path/releases/20121108133909/public/images\ /some/path/releases/20121108133909/public/stylesheets\ /some/path/releases/20121108133909/public/javascripts -exec touch -t -- 201211081339.10 {} ';'; true"

    *** [err :: my.server.com] find:

    *** [err :: my.server.com] `/some/path/releases/20121108133909/public/images /some/path/releases/20121108133909/public/stylesheets /some/path/releases/20121108133909/public/javascripts'

    *** [err :: my.server.com] : No such file or directory

As you can see in the command line there are doubly escaped spaces which results in find being passed 1 argument with escaped spaces rather than 3 separate arguments.

@elhoyos

I'm experiencing something similar with the paths resolved by finalize_update.

Every command issued by this task adds two backslashes at the beginning, which leads to a No such file or directory error:

chmod -R -- g+w \\~/webapps/cr_services/releases/20121108183351 && rm -rf -- \\~/webapps/cr_services/releases/20121108183351/public/system && mkdir -p -- \\~/webapps/cr_services/releases/20121108183351/public/ && ln -s -- ~/webapps/cr_services/shared/system \\~/webapps/cr_services/releases/20121108183351/public/system && rm -rf -- \\~/webapps/cr_services/releases/20121108183351/log && ln -s -- ~/webapps/cr_services/shared/log \\~/webapps/cr_services/releases/20121108183351/log && rm -rf -- \\~/webapps/cr_services/releases/20121108183351/tmp/pids && mkdir -p -- \\~/webapps/cr_services/releases/20121108183351/tmp/ && ln -s -- ~/webapps/cr_services/shared/pids \\~/webapps/cr_services/releases/20121108183351/tmp/pids

483d253 looks very promising. By removing the #shellscape calls the paths return to normality.

@carsomyr

@elhoyos Capistrano was never meant to allow interpolation of ~'s. You'll have to provide the full path, sorry. In the future, there might be a rewrite that allows relative paths, but that would be the extent of it.

@niall

A fix for this is to replace the line with find with this:

run("find #{asset_paths.join(" ")} -exec touch -t #{stamp} {} ';'; true", :env => { "TZ" => "UTC" }) if asset_paths.any?

Note two changes - removal of #shellscape and removal of -- after -t - the problem with -- was obscured by the doubly escaped spaces problem.

@carsomyr

@niall Yup. Was testing it just now. Thanks for being so diligent!

@carsomyr carsomyr referenced this issue from a commit
@carsomyr carsomyr Fix issue #300 6b499f5
@carsomyr carsomyr referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@carsomyr carsomyr closed this
@javanthropus

Is there a release coming soon that includes this fix?

@mcary

I encountered this issue in the lastest released capistrano gem (2.13.5) with a fairly standard Rails 3 code base and cap configuration. Having Capistrano broken out-of-the-box is kind of a downer.

I see this issue is addressed in this commit: 6b499f5

I also see that the commit removes a reasonable security precaution.

I'd like to offer this commit that preserves the shell escaping and doesn't break the command when multiple paths are present:
mcary@e76d99c

Furthermore, I'd like to encourage a new release because the latest currently-released gem results in a failure during deploy with a default configuration.

Pull request: #339

@weyus

I just found this - I'm going to pull from master for now, but I too, encourage a new release. It's crazy to have cap breaking out of the box.

@carsomyr

All, I'll try to push out a release this weekend. Thanks for the feedback.

@carsomyr carsomyr reopened this
@carsomyr

Alright, 2.14.0 has been pushed. Expect more patch levels on the horizon.

@carsomyr carsomyr closed this
@mcary

Thanks! I just uninstalled the gem version that I had patched and replaced it with the one you released (well, actually one patch later, 2.14.1) and confirmed that the find command succeeds now. Much better!

@felixbuenemann felixbuenemann referenced this issue from a commit
@ndbroadbent ndbroadbent Group finalize_update shell commands into one command, harden shell c…
…alls with #shellescape, and separate arguments with --
483d253
@leehambley leehambley referenced this issue from a commit
Marcel M. Cary Escape deploy:finalize_update asset_paths separately
Commit 483d253 improved robustness by escaping some arguments in
shell commands.  But it also introduced a bug when there are multiple
public_children configured (the default case), resulting in issue #300.
Commit 6b499f5 resolved this issue by reverting the code to escape
arguments.

Escape the arguments separately, rather than as one shell word, to
improve robustness while also avoiding the bug in ticket #300.
e76d99c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.