Skip to content

Commit 0f33319

Browse files
committed
Bug#35732000 wrong error message if existing socket-path is not readable
If the routing.socket already exists and isn't readable for the router, router fails to start with: Failed setting up acceptor on '/tmp/router-OOYarI/sockfile': Invalid argument giving no indication what the issue really is. Root Cause ========== Router ... - tries to bind() to the specified socket-path, fails with EADDRINUSE. - tries to connect() to check if another process is bound to the socket, fails a EPERM ... and doesn't exist. - continues with listen() which returns the EINVAL as the socket was not successfully bound to a socket. Change ====== - if connect() fails with another else than ENOTCONN, report the error the user and fail. - added test Change-Id: I8a69a32cb1f51a329b5412c4a5739984cd1d0934
1 parent 8f6c182 commit 0f33319

File tree

2 files changed

+42
-0
lines changed

2 files changed

+42
-0
lines changed

router/src/routing/src/mysql_routing.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,13 @@ stdx::expected<void, std::error_code> AcceptingEndpointUnixSocket::setup() {
586586
if (!last_res) {
587587
return stdx::make_unexpected(last_res.error());
588588
}
589+
} else {
590+
log_warning(
591+
"Checking if existing socket file %s is bound by another process "
592+
"failed: %s",
593+
socket_name_.c_str(), connect_res.error().message().c_str());
594+
595+
return stdx::make_unexpected(connect_res.error());
589596
}
590597
}
591598

router/tests/component/test_routing.cc

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,6 +1119,41 @@ TEST_F(RouterRoutingTest, named_socket_has_right_permissions) {
11191119
socket_file + "'",
11201120
5s));
11211121
}
1122+
1123+
TEST_F(RouterRoutingTest, named_socket_fails_with_socket_is_not_readable) {
1124+
TempDirectory bootstrap_dir;
1125+
1126+
// launch Router with unix socket
1127+
const std::string socket_file = bootstrap_dir.name() + "/sockfile";
1128+
1129+
// create the file that's not readable to trigger a permission denied check.
1130+
{
1131+
int fd = open(socket_file.c_str(), O_WRONLY | O_CREAT | O_EXCL, 0000);
1132+
ASSERT_NE(fd, -1) << errno;
1133+
close(fd);
1134+
}
1135+
1136+
auto writer = config_writer(bootstrap_dir.name());
1137+
1138+
writer.section("routing:basic", {
1139+
{"socket", socket_file},
1140+
{"mode", "read-write"},
1141+
{"destinations", "127.0.0.1:1234"},
1142+
});
1143+
auto &router = router_spawner()
1144+
.wait_for_sync_point(Spawner::SyncPoint::NONE)
1145+
.expected_exit_code(EXIT_FAILURE)
1146+
.spawn({"-c", writer.write()});
1147+
1148+
ASSERT_NO_THROW(router.wait_for_exit());
1149+
1150+
EXPECT_THAT(router.get_logfile_content(),
1151+
::testing::HasSubstr(
1152+
"is bound by another process failed: Permission denied"));
1153+
1154+
// check if the file still exists and hasn't been deleted
1155+
EXPECT_EQ(access(socket_file.c_str(), F_OK), 0);
1156+
}
11221157
#endif
11231158

11241159
TEST_F(RouterRoutingTest, RoutingMaxConnectErrors) {

0 commit comments

Comments
 (0)