diff --git a/.gitignore b/.gitignore index 055785b5e..f79f65a37 100644 --- a/.gitignore +++ b/.gitignore @@ -72,6 +72,7 @@ Makefile.in /tests/compress /tests/cryptest /tests/epv_unpack +/tests/exrpctest /tests/gxl-383 /tests/jsontest /tests/lzxpress diff --git a/Makefile.am b/Makefile.am index 249b3c92a..d0431e1d2 100644 --- a/Makefile.am +++ b/Makefile.am @@ -258,7 +258,7 @@ mapi_la_LDFLAGS = ${plugin_LDFLAGS} ${PHP_LDFLAGS} mapi_la_LIBADD = libphp_mapi.la EXTRA_mapi_la_DEPENDENCIES = ${default_sym} -noinst_PROGRAMS = dldcheck tests/bdump tests/bodyconv tests/compress tests/cryptest tests/gxl-383 tests/jsontest tests/lzxpress tests/utiltest tests/vcard tests/zendfake tools/tzdump +noinst_PROGRAMS = dldcheck tests/bdump tests/bodyconv tests/compress tests/cryptest tests/exrpctest tests/gxl-383 tests/jsontest tests/lzxpress tests/utiltest tests/vcard tests/zendfake tools/tzdump if HAVE_ESEDB noinst_PROGRAMS += tests/epv_unpack endif @@ -275,6 +275,8 @@ tests_cryptest_SOURCES = tests/cryptest.cpp tests_cryptest_LDADD = libgromox_common.la tests_epv_unpack_SOURCES = tests/epv_unpack.cpp tools/edb_pack.cpp tools/edb_pack.hpp tests_epv_unpack_LDADD = ${libesedb_LIBS} ${libHX_LIBS} libgromox_common.la libgromox_mapi.la +tests_exrpctest_SOURCES = tests/exrpctest.cpp +tests_exrpctest_LDADD = libgromox_common.la libgromox_exrpc.la tests_gxl_383_SOURCES = tests/gxl-383.cpp tests_gxl_383_LDADD = libgromox_common.la libgromox_exrpc.la libgromox_mapi.la tests_jsontest_SOURCES = tests/jsontest.cpp diff --git a/doc/changelog.rst b/doc/changelog.rst index bc836746a..d227078d4 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -1,6 +1,10 @@ -Development 2.21.10 +Development 2.21.13 =================== +Fixes: + +* exmdb_client: discard connections when EOF is detected + Enhancements: * nsp,ab: support name resolution of IDN addresses diff --git a/lib/exmdb_client.cpp b/lib/exmdb_client.cpp index 2040ee65d..a6547fcff 100644 --- a/lib/exmdb_client.cpp +++ b/lib/exmdb_client.cpp @@ -37,7 +37,7 @@ static constexpr unsigned int mdcl_ping_timeout = 2; static_assert(SOCKET_TIMEOUT >= mdcl_ping_timeout); static std::list mdcl_agent_list; static std::list mdcl_server_list; -static std::mutex mdcl_server_lock; +static std::mutex mdcl_server_lock; /* he protecc mdcl_server_list+mdcl_agent_list */ static atomic_bool mdcl_notify_stop; static unsigned int mdcl_conn_max, mdcl_threads_max; static pthread_t mdcl_scan_id; @@ -332,6 +332,7 @@ static int launch_notify_listener(remote_svr &srv) try if (mdcl_event_proc == nullptr) return 0; mdcl_agent_list.emplace_back(); + /* Notification thread creates its own socket. */ auto &ag = mdcl_agent_list.back(); ag.pserver = &srv; ag.sockd = -1; @@ -442,6 +443,16 @@ bool exmdb_client_check_local(const char *prefix, BOOL *pvt) return true; } +static bool sock_ready_for_write(int fd) +{ + struct pollfd pfd = {fd, POLLIN}; + /* + * If there was already data to read (poll returns 1) or EOF was hit + * (poll returns 1), the socket is not ready for write. + */ + return poll(&pfd, 1, 0) == 0; +} + static remote_conn_ref exmdb_client_get_connection(const char *dir) { remote_conn_ref fc; @@ -452,9 +463,12 @@ static remote_conn_ref exmdb_client_get_connection(const char *dir) mlog(LV_ERR, "exmdb_client: cannot find remote server for %s", dir); return fc; } - if (i->conn_list.size() > 0) { - fc.tmplist.splice(fc.tmplist.end(), i->conn_list, i->conn_list.begin()); - return fc; + while (i->conn_list.size() > 0) { + if (sock_ready_for_write(i->conn_list.front().sockd)) { + fc.tmplist.splice(fc.tmplist.end(), i->conn_list, i->conn_list.begin()); + return fc; + } + i->conn_list.pop_front(); } if (i->active_handles >= mdcl_conn_max) { mlog(LV_ERR, "exmdb_client: reached maximum connections (%u) to [%s]:%hu/%s", @@ -496,20 +510,24 @@ BOOL exmdb_client_do_rpc(const exreq *rq, exresp *rsp) if (!exmdb_client_read_socket(conn->sockd, bin, mdcl_rpc_timeout)) return false; conn->last_time = time(nullptr); - conn.reset(); if (bin.pb == nullptr) return false; if (bin.cb == 1) { - fprintf(stderr, "%s(%s): %s\n", __func__, znul(rq->dir), - exmdb_rpc_strerror(static_cast(bin.pb[0]))); exmdb_rpc_free(bin.pb); + /* Connection is still good in principle. */ + conn.reset(); return false; } if (bin.cb < 5) { - /* Malformed packet? */ exmdb_rpc_free(bin.pb); + /* + * Malformed packet? Let connection die + * (~exmdb_connection_ref), lest the next response might pick + * up garbage from the current response. + */ return false; } + conn.reset(); rsp->call_id = rq->call_id; bin.cb -= 5; bin.pb += 5; diff --git a/tests/exrpctest.cpp b/tests/exrpctest.cpp new file mode 100644 index 000000000..271c28837 --- /dev/null +++ b/tests/exrpctest.cpp @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +// SPDX-FileCopyrightText: 2024 grommunio GmbH +// This file is part of Gromox. +#include +#include +#include +#include +#include +#include + +using namespace gromox; +namespace exmdb_client = exmdb_client_remote; + +int main(int argc, char **argv) +{ + exmdb_client_init(1, 0); + if (exmdb_client_run(PKGSYSCONFDIR) != 0) + return EXIT_FAILURE; + + static constexpr STORE_ENTRYID other_store = {0, 0, 0, 0, {}, 0, deconst(""), deconst("")}; + const char *g_storedir = argc == 1 ? "/" : argv[1]; + char *newdir = nullptr; + unsigned int user_id = 0, domain_id = 0; + + printf("req 1\n"); + if (!exmdb_client::store_eid_to_user(g_storedir, &other_store, &newdir, + &user_id, &domain_id)) + mlog(LV_DEBUG, "store_eid_to_user failed as expected"); + else + mlog(LV_ERR, "store_eid_to_user unexpectedly succeeded"); + // Connection should have died by now + printf("req 2\n"); + + static constexpr uint32_t tags[] = {PR_STORE_RECORD_KEY}; + static constexpr PROPTAG_ARRAY ptags[] = {std::size(tags), deconst(tags)}; + TPROPVAL_ARRAY props{}; + exmdb_client::get_store_properties(g_storedir, CP_UTF8, ptags, &props); + return 0; +}