From e825e8eec4db8fbe6bcc7fe7b45c5499e2ab3454 Mon Sep 17 00:00:00 2001 From: Kory Draughn Date: Mon, 22 Jul 2024 12:19:57 -0400 Subject: [PATCH] [#7155] client_connection.cpp: Use error information in THROW expressions. --- lib/core/src/client_connection.cpp | 18 +++- unit_tests/src/test_client_connection.cpp | 116 ++++++++++++++++++++++ 2 files changed, 130 insertions(+), 4 deletions(-) diff --git a/lib/core/src/client_connection.cpp b/lib/core/src/client_connection.cpp index c342496771..8f928e2783 100644 --- a/lib/core/src/client_connection.cpp +++ b/lib/core/src/client_connection.cpp @@ -156,9 +156,9 @@ namespace irods::experimental { only_connect(_host, _port, _username); - if (clientLogin(conn_.get()) != 0) { + if (const auto ec = clientLogin(conn_.get()); ec < 0) { // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay) - THROW(AUTHENTICATION_ERROR, "Client login error"); + THROW(ec, "Client login error"); } } // connect_and_login @@ -169,9 +169,9 @@ namespace irods::experimental { only_connect(_host, _port, _proxy_username, _username); - if (clientLogin(conn_.get()) != 0) { + if (const auto ec = clientLogin(conn_.get()); ec < 0) { // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay) - THROW(AUTHENTICATION_ERROR, "Client login error"); + THROW(ec, "Client login error"); } } // connect_and_login @@ -183,6 +183,11 @@ namespace irods::experimental conn_.reset( rcConnect(_host.c_str(), _port, _username.name().c_str(), _username.zone().c_str(), NO_RECONN, &error)); + if (error.status < 0) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay) + THROW(error.status, error.msg); + } + if (!conn_) { // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay) THROW(USER_SOCK_CONNECT_ERR, "Connect error"); @@ -205,6 +210,11 @@ namespace irods::experimental 1, NO_RECONN)); + if (error.status < 0) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay) + THROW(error.status, error.msg); + } + if (!conn_) { // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-array-to-pointer-decay) THROW(USER_SOCK_CONNECT_ERR, "Connect error"); diff --git a/unit_tests/src/test_client_connection.cpp b/unit_tests/src/test_client_connection.cpp index e472eb0edb..007f567622 100644 --- a/unit_tests/src/test_client_connection.cpp +++ b/unit_tests/src/test_client_connection.cpp @@ -1,3 +1,7 @@ +// +// IMPORTANT: Some tests in this file require rodsadmin level privileges! +// + #include #include "irods/client_connection.hpp" @@ -7,7 +11,11 @@ #include "irods/irods_at_scope_exit.hpp" #include "irods/rcConnect.h" #include "irods/rodsClient.h" +#include "irods/rodsErrorTable.h" #include "irods/user_administration.hpp" +#include "irods/system_error.hpp" + +#include // For ECONNREFUSED. #include #include @@ -300,3 +308,111 @@ TEST_CASE("#7192: all connections store unique session signatures", "client_conn CHECK(std::all_of(std::begin(pairs), std::end(pairs), has_unique_session_signatures)); } + +// NOLINTNEXTLINE(readability-function-cognitive-complexity) +TEST_CASE("#7155: report underlying error info from server on failed connection") +{ + load_client_api_plugins(); + + rodsEnv env; + _getRodsEnv(env); + + SECTION("invalid host") + { + try { + ix::client_connection{"example.org", env.rodsPort, {env.rodsUserName, env.rodsZone}}; + } + catch (const irods::exception& e) { + CHECK(e.code() == USER_SOCK_CONNECT_TIMEDOUT); + CHECK_THAT(e.client_display_what(), Catch::Matchers::Contains("_rcConnect: connectToRhost failed")); + } + } + + SECTION("invalid port") + { + try { + ix::client_connection{env.rodsHost, 8080, {env.rodsUserName, env.rodsZone}}; + } + catch (const irods::exception& e) { + const auto ec = ix::make_error_code(e.code()); + CHECK(ix::get_irods_error_code(ec) == USER_SOCK_CONNECT_ERR); + CHECK(ix::get_errno(ec) == ECONNREFUSED); + CHECK_THAT(e.client_display_what(), Catch::Matchers::Contains("_rcConnect: connectToRhost failed")); + } + } + + SECTION("invalid username, not proxied") + { + try { + ix::client_connection{env.rodsHost, env.rodsPort, {"invalid", env.rodsZone}}; + } + catch (const irods::exception& e) { + CHECK(e.code() == CAT_INVALID_USER); + CHECK_THAT(e.client_display_what(), Catch::Matchers::Contains("Client login error")); + } + } + + SECTION("invalid user zone, not proxied") + { + try { + ix::client_connection{env.rodsHost, env.rodsPort, {env.rodsUserName, "invalid"}}; + } + catch (const irods::exception& e) { + CHECK(e.code() == CAT_INVALID_USER); + CHECK_THAT(e.client_display_what(), Catch::Matchers::Contains("Client login error")); + } + } + + SECTION("deferred connection with empty zone") + { + try { + ix::client_connection conn{ix::defer_connection}; + conn.connect(env.rodsHost, env.rodsPort, {env.rodsUserName, ""}); + } + catch (const irods::exception& e) { + CHECK(e.code() == SYS_INVALID_INPUT_PARAM); + CHECK_THAT(e.client_display_what(), Catch::Matchers::Contains("Empty zone not allowed")); + } + } + + SECTION("proxy tests") + { + ix::client_connection admin_conn; + + namespace ia = irods::experimental::administration; + + const ia::user other_user{"other_user"}; + REQUIRE_NOTHROW(ia::client::add_user(admin_conn, other_user, ia::user_type::rodsadmin)); + + irods::at_scope_exit remove_proxy_admin{ + [&admin_conn, other_user] { ia::client::remove_user(admin_conn, other_user); }}; + + const auto password = std::to_array("rods"); + const ia::user_password_property pwd_property{password.data()}; + REQUIRE_NOTHROW(ia::client::modify_user(admin_conn, other_user, pwd_property)); + + SECTION("proxied user has empty zone name") + { + try { + ix::client_connection{ + env.rodsHost, env.rodsPort, {env.rodsUserName, env.rodsZone}, {other_user.name, ""}}; + } + catch (const irods::exception& e) { + CHECK(e.code() == SYS_INVALID_INPUT_PARAM); + CHECK_THAT(e.client_display_what(), Catch::Matchers::Contains("Empty zone not allowed")); + } + } + + SECTION("proxy user has empty zone name") + { + try { + ix::client_connection{ + env.rodsHost, env.rodsPort, {env.rodsUserName, ""}, {other_user.name, env.rodsZone}}; + } + catch (const irods::exception& e) { + CHECK(e.code() == SYS_INVALID_INPUT_PARAM); + CHECK_THAT(e.client_display_what(), Catch::Matchers::Contains("Empty zone not allowed")); + } + } + } +}