Skip to content

Commit

Permalink
Fix tunnel parsing exception handling.
Browse files Browse the repository at this point in the history
Rather than using STFATAL and exiting, bubble up tunnel parsing
exception to main and display the responsible option arg and print help
menu so the user need not dig into /tmp/etclient-* to determine why et
aborted.

Fixes #491
  • Loading branch information
jshort committed Dec 7, 2022
1 parent 91099f6 commit dbca326
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 14 deletions.
1 change: 1 addition & 0 deletions codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ ignore:
- "src/terminal/TelemetryService*"
- "src/terminal/PsuedoTerminalConsole.hpp"
- "src/terminal/PsuedoUserTerminal.hpp"
- "src/terminal/*Main.cpp"
- "src/base/TcpSocketHandler*"
- "src/base/SubprocessToString*"
coverage:
Expand Down
18 changes: 12 additions & 6 deletions src/terminal/TerminalClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ vector<PortForwardSourceRequest> parseRangesToRequests(const string& input) {
auto j = split(input, ',');
for (auto& pair : j) {
vector<string> sourceDestination = split(pair, ':');
if (sourceDestination.size() < 2) {
throw TunnelParseException(
"Tunnel argument must have source and destination between a ':'");
}
try {
if (sourceDestination[0].find_first_not_of("0123456789-") !=
string::npos &&
Expand All @@ -29,8 +33,8 @@ vector<PortForwardSourceRequest> parseRangesToRequests(const string& input) {

if (sourcePortEnd - sourcePortStart !=
destinationPortEnd - destinationPortStart) {
STFATAL << "source/destination port range mismatch";
exit(1);
throw TunnelParseException(
"source/destination port range must have same length");
} else {
int portRangeLength = sourcePortEnd - sourcePortStart + 1;
for (int i = 0; i < portRangeLength; ++i) {
Expand All @@ -42,17 +46,19 @@ vector<PortForwardSourceRequest> parseRangesToRequests(const string& input) {
}
} else if (sourceDestination[0].find('-') != string::npos ||
sourceDestination[1].find('-') != string::npos) {
STFATAL << "Invalid port range syntax: if source is range, "
"destination must be range";
throw TunnelParseException(
"Invalid port range syntax: if source is range, "
"destination must be range");
} else {
PortForwardSourceRequest pfsr;
pfsr.mutable_source()->set_port(stoi(sourceDestination[0]));
pfsr.mutable_destination()->set_port(stoi(sourceDestination[1]));
pfsrs.push_back(pfsr);
}
} catch (const TunnelParseException& e) {
throw e;
} catch (const std::logic_error& lr) {
STFATAL << "Logic error: " << lr.what();
exit(1);
throw TunnelParseException("Invalid tunnel argument: " + input);
}
}
return pfsrs;
Expand Down
9 changes: 9 additions & 0 deletions src/terminal/TerminalClient.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,14 @@ class TerminalClient {
int keepaliveDuration;
};

class TunnelParseException : public std::exception {
public:
TunnelParseException(const string& msg) : message(msg) {}
const char* what() const noexcept override { return message.c_str(); }

private:
std::string message = " ";
};

} // namespace et
#endif // __ET_TERMINAL_CLIENT__
26 changes: 18 additions & 8 deletions src/terminal/TerminalClientMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ bool ping(SocketEndpoint socketEndpoint,
return true;
}

void handleParseException(std::exception& e, cxxopts::Options& options) {
CLOG(INFO, "stdout") << "Exception: " << e.what() << "\n" << endl;
CLOG(INFO, "stdout") << options.help({}) << endl;
exit(1);
}

int main(int argc, char** argv) {
WinsockContext context;
string tmpDir = GetTempDirectory();
Expand Down Expand Up @@ -340,17 +346,21 @@ int main(int argc, char** argv) {
}
TelemetryService::get()->logToDatadog("Session Started", el::Level::Info,
__FILE__, __LINE__);
TerminalClient terminalClient(
clientSocket, clientPipeSocket, socketEndpoint, id, passkey, console,
is_jumphost, result.count("t") ? result["t"].as<string>() : "",
result.count("r") ? result["r"].as<string>() : "", forwardAgent,
sshSocket, keepaliveDuration);
string tunnel_arg =
result.count("tunnel") ? result["tunnel"].as<string>() : "";
string r_tunnel_arg = result.count("reversetunnel")
? result["reversetunnel"].as<string>()
: "";
TerminalClient terminalClient(clientSocket, clientPipeSocket,
socketEndpoint, id, passkey, console,
is_jumphost, tunnel_arg, r_tunnel_arg,
forwardAgent, sshSocket, keepaliveDuration);
terminalClient.run(result.count("command") ? result["command"].as<string>()
: "");
} catch (TunnelParseException& tpe) {
handleParseException(tpe, options);
} catch (cxxopts::OptionException& oe) {
CLOG(INFO, "stdout") << "Exception: " << oe.what() << "\n" << endl;
CLOG(INFO, "stdout") << options.help({}) << endl;
exit(1);
handleParseException(oe, options);
}

#ifdef WIN32
Expand Down
16 changes: 16 additions & 0 deletions test/TerminalTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,22 @@ class EndToEndTestFixture {
bool wasShutdown = false;
};

TEST_CASE_METHOD(EndToEndTestFixture, "InvalidTunnelArg",
"[InvalidTunnelArg]") {
REQUIRE_THROWS(new TerminalClient(
clientSocketHandler, clientPipeSocketHandler, serverEndpoint,
"1234567890123456", CRYPTO_KEY, fakeConsole, false, "6010", "", false, "",
MAX_CLIENT_KEEP_ALIVE_DURATION));
REQUIRE_THROWS(new TerminalClient(
clientSocketHandler, clientPipeSocketHandler, serverEndpoint,
"1234567890123456", CRYPTO_KEY, fakeConsole, false, "6010-6012:7000", "",
false, "", MAX_CLIENT_KEEP_ALIVE_DURATION));
REQUIRE_THROWS(new TerminalClient(
clientSocketHandler, clientPipeSocketHandler, serverEndpoint,
"1234567890123456", CRYPTO_KEY, fakeConsole, false, "6010-6012:7000-8000",
"", false, "", MAX_CLIENT_KEEP_ALIVE_DURATION));
}

TEST_CASE_METHOD(EndToEndTestFixture, "EndToEndTest", "[EndToEndTest]") {
readWriteTest("1234567890123456", routerSocketHandler, fakeUserTerminal,
serverEndpoint, clientSocketHandler, clientPipeSocketHandler,
Expand Down

0 comments on commit dbca326

Please sign in to comment.