Skip to content

Commit b269b97

Browse files
committed
Fix tunnel parsing exception handling.
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
1 parent ff2249e commit b269b97

File tree

4 files changed

+56
-16
lines changed

4 files changed

+56
-16
lines changed

src/terminal/TerminalClient.cpp

+12-7
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ vector<PortForwardSourceRequest> parseRangesToRequests(const string& input) {
88
auto j = split(input, ',');
99
for (auto& pair : j) {
1010
vector<string> sourceDestination = split(pair, ':');
11+
if (sourceDestination.size() < 2) {
12+
throw TunnelParseException(
13+
"Tunnel argument must have source and destination between a ':'");
14+
}
1115
try {
1216
if (sourceDestination[0].find_first_not_of("0123456789-") !=
1317
string::npos &&
@@ -29,8 +33,7 @@ vector<PortForwardSourceRequest> parseRangesToRequests(const string& input) {
2933

3034
if (sourcePortEnd - sourcePortStart !=
3135
destinationPortEnd - destinationPortStart) {
32-
STFATAL << "source/destination port range mismatch";
33-
exit(1);
36+
throw TunnelParseException("source/destination port range must have same length");
3437
} else {
3538
int portRangeLength = sourcePortEnd - sourcePortStart + 1;
3639
for (int i = 0; i < portRangeLength; ++i) {
@@ -42,17 +45,19 @@ vector<PortForwardSourceRequest> parseRangesToRequests(const string& input) {
4245
}
4346
} else if (sourceDestination[0].find('-') != string::npos ||
4447
sourceDestination[1].find('-') != string::npos) {
45-
STFATAL << "Invalid port range syntax: if source is range, "
46-
"destination must be range";
48+
throw TunnelParseException("Invalid port range syntax: if source is range, "
49+
"destination must be range");
4750
} else {
4851
PortForwardSourceRequest pfsr;
4952
pfsr.mutable_source()->set_port(stoi(sourceDestination[0]));
5053
pfsr.mutable_destination()->set_port(stoi(sourceDestination[1]));
5154
pfsrs.push_back(pfsr);
5255
}
53-
} catch (const std::logic_error& lr) {
54-
STFATAL << "Logic error: " << lr.what();
55-
exit(1);
56+
} catch (const TunnelParseException& e) {
57+
throw e;
58+
} catch (const std::exception& e) {
59+
string msg = "Invalid tunnel argument: " + input;
60+
throw TunnelParseException(msg.c_str());
5661
}
5762
}
5863
return pfsrs;

src/terminal/TerminalClient.hpp

+11
Original file line numberDiff line numberDiff line change
@@ -41,5 +41,16 @@ class TerminalClient {
4141
int keepaliveDuration;
4242
};
4343

44+
class TunnelParseException : public std::exception {
45+
public:
46+
TunnelParseException(const char* msg) : message(msg) {}
47+
const char* what() const noexcept override
48+
{
49+
return message.c_str();
50+
}
51+
private:
52+
std::string message = " ";
53+
};
54+
4455
} // namespace et
4556
#endif // __ET_TERMINAL_CLIENT__

src/terminal/TerminalClientMain.cpp

+18-9
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ bool ping(SocketEndpoint socketEndpoint,
2222
return true;
2323
}
2424

25+
void handleParseException(std::exception& e, cxxopts::Options& options) {
26+
CLOG(INFO, "stdout") << "Exception: " << e.what() << "\n" << endl;
27+
CLOG(INFO, "stdout") << options.help({}) << endl;
28+
exit(1);
29+
}
30+
2531
int main(int argc, char** argv) {
2632
WinsockContext context;
2733
string tmpDir = GetTempDirectory();
@@ -43,7 +49,7 @@ int main(int argc, char** argv) {
4349
options.custom_help("[OPTION...] [user@]host[:port]\n\n"
4450
" Note that 'host' can be a hostname or ipv4 address with or without a port\n"
4551
" or an ipv6 address. If the ipv6 address is abbreviated with :: then it must\n"
46-
" be specfied without a port (use -p,--port)."
52+
" be specified without a port (use -p,--port)."
4753
);
4854

4955
options.add_options()
@@ -334,17 +340,20 @@ int main(int argc, char** argv) {
334340
}
335341
TelemetryService::get()->logToDatadog("Session Started", el::Level::Info,
336342
__FILE__, __LINE__);
343+
string tunnel_arg =
344+
result.count("tunnel") ? result["tunnel"].as<string>() : "";
345+
string r_tunnel_arg =
346+
result.count("reversetunnel") ? result["reversetunnel"].as<string>() : "";
337347
TerminalClient terminalClient(
338348
clientSocket, clientPipeSocket, socketEndpoint, id, passkey, console,
339-
is_jumphost, result.count("t") ? result["t"].as<string>() : "",
340-
result.count("r") ? result["r"].as<string>() : "", forwardAgent,
341-
sshSocket, keepaliveDuration);
342-
terminalClient.run(result.count("command") ? result["command"].as<string>()
343-
: "");
349+
is_jumphost, tunnel_arg, r_tunnel_arg,
350+
forwardAgent, sshSocket, keepaliveDuration);
351+
terminalClient.run(
352+
result.count("command") ? result["command"].as<string>() : "");
353+
} catch (TunnelParseException& tpe) {
354+
handleParseException(tpe, options);
344355
} catch (cxxopts::OptionException& oe) {
345-
CLOG(INFO, "stdout") << "Exception: " << oe.what() << "\n" << endl;
346-
CLOG(INFO, "stdout") << options.help({}) << endl;
347-
exit(1);
356+
handleParseException(oe, options);
348357
}
349358

350359
#ifdef WIN32

test/TerminalTest.cpp

+15
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,21 @@ class EndToEndTestFixture {
255255
bool wasShutdown = false;
256256
};
257257

258+
TEST_CASE_METHOD(EndToEndTestFixture, "InvalidTunnelArg", "[InvalidTunnelArg]") {
259+
REQUIRE_THROWS(new TerminalClient(
260+
clientSocketHandler, clientPipeSocketHandler, serverEndpoint, "1234567890123456",
261+
CRYPTO_KEY, fakeConsole, false, "6010", "", false, "",
262+
MAX_CLIENT_KEEP_ALIVE_DURATION));
263+
REQUIRE_THROWS(new TerminalClient(
264+
clientSocketHandler, clientPipeSocketHandler, serverEndpoint, "1234567890123456",
265+
CRYPTO_KEY, fakeConsole, false, "6010-6012:7000", "", false, "",
266+
MAX_CLIENT_KEEP_ALIVE_DURATION));
267+
REQUIRE_THROWS(new TerminalClient(
268+
clientSocketHandler, clientPipeSocketHandler, serverEndpoint, "1234567890123456",
269+
CRYPTO_KEY, fakeConsole, false, "6010-6012:7000-8000", "", false, "",
270+
MAX_CLIENT_KEEP_ALIVE_DURATION));
271+
}
272+
258273
TEST_CASE_METHOD(EndToEndTestFixture, "EndToEndTest", "[EndToEndTest]") {
259274
readWriteTest("1234567890123456", routerSocketHandler, fakeUserTerminal,
260275
serverEndpoint, clientSocketHandler, clientPipeSocketHandler,

0 commit comments

Comments
 (0)