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());

• cloakedlearning: Great, thanks for the raw literals. I looked into using them, but the fact they could be used multiline totally passed me by.
• cloakedlearning: Unless I'm missing something basic_ofstream::write must take a const char* not a std::string so I still need the c_str() at that call site.
• Jerry Coffin: @cloakedlearning: Oops--yes, you do. Sorry 'bout that.
• cloakedlearning: No problem. Seems like an odd omission to me. Hopefully string_view will help clean things up a bit in c++17
• Jerry Coffin: @cloakedlearning: Modified code to do it the obvious way.