Executing remote command using expect from C++

Question

I wrote some code to execute remote commands over ssh, using expect, from c++. The code works but is a bit of a mess of c / c++ since there doesn't seem to be an idiomatic way to achieve much of what I wanted in pure c++. I appreciate that there are nominally some security concerns with the way I'm using passwords, but these aren't a concern for my use cases.

remote_commmand.h

#pragma once

#include <stdexcept>

struct RemoteException : public std::runtime_error {
    using std::runtime_error::runtime_error;
};

void remote_command(const std::string &host, const std::string &user, const std::string &password, const std::string &cmd);

remote_command.cpp

#include "remote_command.h"
#include <cstdio>
#include <cstring>
#include <unistd.h>
#include <fstream>
#include <vector>

using std::string;

namespace {

const char* expect_template =
"set timeout 60\n"
"spawn ssh %s@%s %s\n"
"expect {\n"
"   \"*yes/no*\"\n"
"   {\n"
"       send \"yes\\r\"\n"
"       exp_continue\n"
"   }\n"
"   \"*?assword:*\"\n"
"   {\n"
"       send -- \"%s\\r\"\n"
"       send -- \"\\r\"\n"
"       expect eof\n"
"   }\n"
"}\n";

class TempFile {

public:
    explicit TempFile(const char* contents){
        file_name = std::tmpnam(nullptr);
        std::ofstream f(file_name.c_str(), std::ios::out);
        if(!f.write(contents, std::strlen(contents))){
            throw std::runtime_error("ofstream");
        }
    }

    ~TempFile(){
        std::remove(file_name.c_str());
    }

    TempFile(TempFile &other) = delete;
    TempFile& operator=(TempFile &other) = delete;

    std::string file_name;
};

}


void remote_command(const string &host, const string &user, const string &password, const string &cmd){
    std::vector<char> formatted_cmd;
    const std::size_t formatted_size = std::strlen(expect_template) + host.size() + user.size() + password.size() + cmd.size();
    formatted_cmd.resize(formatted_size);

    const std::size_t res = std::snprintf(&formatted_cmd[0], formatted_size, expect_template, user.c_str(), host.c_str(), cmd.c_str(), password.c_str());

    if (res <= 0 || res >= formatted_size ){
        throw RemoteException("infsufficient size for snprintf");
    }

    TempFile tempfile(&formatted_cmd[0]);

    int r = execl("/usr/bin/expect", "-f", tempfile.file_name.c_str(), (char *)0);
    if (r == -1){
        throw RemoteException(string("execl") + std::strerror(errno) );
    }
}

example usage

remote_command("my-host", "user", "pw", "/path/to/my/script.sh");

Answer 1

Consider using raw literals

Your expect_template is a prime candidate to be written as a raw string literal:

const char* expect_template =
R"(set timeout 60
spawn ssh %s@%s %s
expect {
   "*yes/no*"
   {
       send "yes\r"
       exp_continue
   }
   "*?assword:*"
   {
       send -- "%s\r"
       send -- "\r"
       expect eof
   }
}
)";

I may have missed something in the editing, but the basic idea is pretty simple: you don't use escapes at all, just insert exactly what you want the string to contain (including new-lines).

Given an R immediately before the quote, the compiler treats the content as a raw literal. The opening delimiter is the quote, other optional "stuff", and an open paren. The closing delimiter is a close paren, the same optional stuff, and the close quote. In your case, the optional "stuff" isn't needed--you'd use it if you might need to include )" as part of your string. In that case, you might have something like R"<^>( at the beginning so the end of the string would only be signaled by )<^>" (the exact content doesn't matter a whole lot--you just have to be sure it's something that can't occur inside the string).

Use of .c_str()

C++98/03 required that you use s.c_str() when passing s to a stream's constructor or open member function. C++11 eliminated that requirement, so you can just pass the string directly.

Passing strings as parameters

Absent a reason to do otherwise, I'd at least consider passing std::string const & as the parameter to (for one example) TempFile::TempFile. This can simplify some of the content a little, and still lets you pass a C-style string when/if you want:

explicit TempFile(std::string const &contents) {
    file_name = std::tmpnam(nullptr);
    std::ofstream f(file_name);
    if(!(f << contents)) {
        throw std::runtime_error("writing ofstream");
    }
}

Use of snprintf

You might want to consider using a stringstream instead of snprintf. This does ease at least a few parts of the job, although it's not exactly perfect either.

std::ostringstream s;

s << 
R("set timeout 60
spawn ssh )" << user << "@" host << " " command << R"(
expect {
   "*yes/no*"
   {
       send "yes\r"
       exp_continue
   }
   "*?assword:*"
   {    
       send -- ")"
<< password << R"(\r"
        send -- "\r"
        expect eof
    }
}
)";

TempFile f(s.str());