diff --git a/Containerfile.mdsip b/Containerfile.mdsip index d246061..b3e764f 100644 --- a/Containerfile.mdsip +++ b/Containerfile.mdsip @@ -277,6 +277,10 @@ RUN set -eux; \ `# shared tree state again, silently and only under concurrency.` \ grep -q 'fork,reuseaddr' /usr/local/bin/fdp-mdsip-server; \ grep -q 'nofork' /usr/local/bin/fdp-mdsip-server; \ + `# Both listener forms must carry fork+nofork: the Unix form is what` \ + `# production runs, the TCP form what the tests and the default run.` \ + grep -q 'UNIX-LISTEN:\$FDP_MDSIP_SOCKET,fork,unlink-early,mode=666' /usr/local/bin/fdp-mdsip-server; \ + [ "$(grep -c 'EXEC:/usr/local/bin/fdp-mdsip-connection,nofork' /usr/local/bin/fdp-mdsip-server)" = 2 ]; \ `# The Pelican-path chain resolves relative to the working directory, so` \ `# losing this line breaks every ptdata lookup and nothing else.` \ grep -q '^cd /$' /usr/local/bin/fdp-mdsip-connection; \ diff --git a/README.md b/README.md index af3a8a4..12a38b0 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,12 @@ ofs.osslib ++ /path/to/libXrdOssMdsplus.so prefix=/tdi server=localhost:8000 http.exthandler mdsip /path/to/libXrdHttpMdsip.so prefix=/mdsip,host=localhost,port=8000,auth=xrootd ``` +``` +# Production: the sandbox is rootless, so the relay reaches it over a Unix socket +# rather than a TCP hop through pasta (which stalls replies above ~4 MB): +http.exthandler mdsip /path/to/libXrdHttpMdsip.so prefix=/mdsip,socket=/run/fdp-mdsip/mdsip.sock,auth=xrootd +``` + Note that neither name in the config carries the **`-5` suffix** the files on disk do: XRootD appends the plugin version itself, and referencing the suffixed name makes it search for `-5-5.so` and only succeed via a fallback. The client @@ -294,6 +300,12 @@ Loaded alongside the osslib, again with the **unsuffixed** name: http.exthandler mdsip /path/to/libXrdHttpMdsip.so prefix=/mdsip,host=localhost,port=8000,auth=xrootd ``` +``` +# Production: the sandbox is rootless, so the relay reaches it over a Unix socket +# rather than a TCP hop through pasta (which stalls replies above ~4 MB): +http.exthandler mdsip /path/to/libXrdHttpMdsip.so prefix=/mdsip,socket=/run/fdp-mdsip/mdsip.sock,auth=xrootd +``` + Parms are a single `XrdOucStream::GetWord()` token, so they **cannot contain spaces** — comma is the separator. Add `+notls` before the library path only when running without TLS, as the test harness does. diff --git a/docs/deployment-notes.md b/docs/deployment-notes.md index f7cefd6..eb1449d 100644 --- a/docs/deployment-notes.md +++ b/docs/deployment-notes.md @@ -281,6 +281,26 @@ its production shape: http.exthandler mdsip /plugins/libXrdHttpMdsip.so prefix=/mdsip,host=,port=8000,auth=xrootd,authpath=/fdp-d3d/archives ``` +**Unix socket (production).** `socket=/run/fdp-mdsip/mdsip.sock` replaces +`host=` and `port=`; naming both is refused at load. The `fdp-mdsip` container +listens there when `FDP_MDSIP_SOCKET` is set (`scripts/fdp-mdsip-server`) and +runs with `Network=none`; the directory is a host tmpfs bind-mounted into both +containers. Measured reason: through pasta every reply above ~4 MB waited out a +200 ms retransmission timeout (d3d-origin-admin +`docs/specs/2026-10-13-mdsip-unix-socket-design.md`). + +Three things to know when operating it: + +- The directive's parameters are one comma-separated token, so a socket path + cannot contain a comma or whitespace. `d3d-origin-admin` renders a fixed + value; nothing else should. +- `port=` is strict: `port=80abc`, which `atoi` used to read as 80, is now + refused at load with the text that was given. +- mdsip logs the peer of each connection into `/tmp/mdsip.access` by reading + `getpeername` into a `sockaddr_in`. An `AF_UNIX` peer fills only the family, + so those lines show a meaningless IPv4 address. The hosts-file match + (`* | MAP_TO_LOCAL`) is unaffected; do not chase that address as a client. + Note the **unsuffixed** library name: XRootD appends the plugin version itself. ### Checks worth doing before the change diff --git a/scripts/fdp-mdsip-server b/scripts/fdp-mdsip-server index 1b2bd1c..65ff269 100755 --- a/scripts/fdp-mdsip-server +++ b/scripts/fdp-mdsip-server @@ -10,5 +10,17 @@ # # Losing either flag serves shared tree state again -- silently, and only under # concurrency. Containerfile.mdsip greps this file for both. +# FDP_MDSIP_SOCKET set: listen on that Unix socket instead of TCP. Production +# uses this because the sandbox is rootless and a TCP hop into it crosses +# pasta, which stalls every reply above ~4 MB for a retransmission timeout. +# unlink-early remove a stale socket file from a previous run before bind +# mode=666 any local process may connect -- the same reachability as the +# bridge-address TCP port had; the directory's 0711 keeps it +# unlistable (d3d-origin-admin docs/specs/2026-10-13-mdsip-unix-socket-design.md) +# The path is a socat address: it must contain no comma or colon. +if [ -n "${FDP_MDSIP_SOCKET:-}" ]; then + exec /usr/bin/socat "UNIX-LISTEN:$FDP_MDSIP_SOCKET,fork,unlink-early,mode=666" \ + EXEC:/usr/local/bin/fdp-mdsip-connection,nofork +fi exec /usr/bin/socat TCP-LISTEN:8000,fork,reuseaddr \ EXEC:/usr/local/bin/fdp-mdsip-connection,nofork diff --git a/src/HttpRelay.cc b/src/HttpRelay.cc index 2356e0d..e03b5e1 100644 --- a/src/HttpRelay.cc +++ b/src/HttpRelay.cc @@ -143,7 +143,7 @@ std::string HeaderValue(std::map &headers, class HttpMdsipRelay : public XrdHttpExtHandler { public: HttpMdsipRelay(XrdSysError *log, const std::string &prefix, - const std::string &host, int port, int idle, + const fdp::MdsipTarget &target, int idle, size_t max_sessions, int timeout_ms, XrdSfsFileSystem *sfs, const std::string &authpath, const std::string &point_prefix, @@ -152,7 +152,7 @@ class HttpMdsipRelay : public XrdHttpExtHandler { : log_(log), prefix_(prefix), sfs_(sfs), authpath_(authpath), point_prefix_(point_prefix), point_authpath_(point_authpath), points_(points), - sessions_(host, port, idle, max_sessions, timeout_ms) {} + sessions_(target, idle, max_sessions, timeout_ms) {} bool MatchesPath(const char *verb, const char *path) { if (!path || !verb) return false; @@ -451,20 +451,21 @@ extern "C" XrdHttpExtHandler *XrdHttpGetExtHandler(XrdSysError *eDest, (void)confg; const std::string prefix = ParmValue(parms, "prefix", "/mdsip"); - const std::string host = ParmValue(parms, "host", "localhost"); - const std::string port_str = ParmValue(parms, "port", "8000"); - const int port = std::atoi(port_str.c_str()); + fdp::MdsipTarget target; + std::string target_error; + if (!fdp::ResolveMdsipTarget(ParmValue(parms, "host", ""), + ParmValue(parms, "port", ""), + ParmValue(parms, "socket", ""), + target, target_error)) { + eDest->Emsg("relay", "refusing to load:", target_error.c_str()); + return 0; + } const int idle = std::atoi(ParmValue(parms, "idle", "300").c_str()); const int timeout_ms = std::atoi(ParmValue(parms, "timeout", "60").c_str()) * 1000; const size_t max_sessions = std::strtoul(ParmValue(parms, "maxsessions", "256").c_str(), 0, 10); - if (port <= 0 || port > 65535) { - eDest->Emsg("relay", "invalid mdsip port", port_str.c_str()); - return 0; - } - // An XrdHttpExtHandler runs BEFORE XRootD's authorization. XrdHttpReq.cc // dispatches at reqstate == 0, ahead of the file-access path where // ofs.authorize and the SciTokens plugin live, so nothing this handler @@ -599,10 +600,13 @@ extern "C" XrdHttpExtHandler *XrdHttpGetExtHandler(XrdSysError *eDest, eDest->Say("++++++ XrdHttpMdsip point endpoint: ", pbanner.c_str()); } - const std::string banner = prefix + " -> mdsip " + host + ":" + port_str; + const std::string where = target.socket_path.empty() + ? target.host + ":" + std::to_string(target.port) + : "unix:" + target.socket_path; + const std::string banner = prefix + " -> mdsip " + where; eDest->Say("++++++ XrdHttpMdsip relay: ", banner.c_str()); - return new HttpMdsipRelay(eDest, prefix, host, port, idle, max_sessions, + return new HttpMdsipRelay(eDest, prefix, target, idle, max_sessions, timeout_ms, sfs, authpath, point_prefix, point_authpath, points); } diff --git a/src/MdsipSession.cc b/src/MdsipSession.cc index bc81de5..fa084eb 100644 --- a/src/MdsipSession.cc +++ b/src/MdsipSession.cc @@ -5,10 +5,12 @@ #include #include #include +#include #include #include #include +#include #include #include @@ -71,6 +73,39 @@ size_t MdsipMessageLength(const std::string &hdr) { return len; } +bool ResolveMdsipTarget(const std::string &host_parm, + const std::string &port_parm, + const std::string &socket_parm, + MdsipTarget &out, std::string &error) { + out = MdsipTarget(); + error.clear(); + if (!socket_parm.empty()) { + if (!host_parm.empty() || !port_parm.empty()) { + error = "socket= cannot be combined with host= or port="; + return false; + } + if (socket_parm.size() >= sizeof(sockaddr_un::sun_path)) { + error = "mdsip socket path too long: " + socket_parm; + return false; + } + out.socket_path = socket_parm; + return true; + } + out.host = host_parm.empty() ? std::string("localhost") : host_parm; + if (port_parm.empty()) { + out.port = 8000; + return true; + } + char *end = 0; + const long port = std::strtol(port_parm.c_str(), &end, 10); + if (!end || *end != '\0' || port <= 0 || port > 65535) { + error = "invalid mdsip port " + port_parm; + return false; + } + out.port = static_cast(port); + return true; +} + bool ReadOneMessage(int fd, std::string &out, int timeout_ms) { out.clear(); @@ -94,11 +129,25 @@ bool ReadOneMessage(int fd, std::string &out, int timeout_ms) { return true; } -MdsipSessions::MdsipSessions(std::string host, int port, int idle_seconds, +MdsipSessions::MdsipSessions(const MdsipTarget &target, int idle_seconds, size_t max_sessions, int timeout_ms) - : host_(std::move(host)), port_(port), idle_seconds_(idle_seconds), + : target_(target), idle_seconds_(idle_seconds), max_sessions_(max_sessions), timeout_ms_(timeout_ms) {} +namespace { +MdsipTarget TcpTarget(std::string host, int port) { + MdsipTarget t; + t.host = std::move(host); + t.port = port; + return t; +} +} // namespace + +MdsipSessions::MdsipSessions(std::string host, int port, int idle_seconds, + size_t max_sessions, int timeout_ms) + : MdsipSessions(TcpTarget(std::move(host), port), idle_seconds, + max_sessions, timeout_ms) {} + MdsipSessions::~MdsipSessions() { std::lock_guard lock(mutex_); for (std::map::iterator it = sessions_.begin(); @@ -125,34 +174,59 @@ std::string MdsipSessions::Open(std::string &error) { } } - char portbuf[16]; - std::snprintf(portbuf, sizeof(portbuf), "%d", port_); - - addrinfo hints; - std::memset(&hints, 0, sizeof(hints)); - hints.ai_family = AF_UNSPEC; - hints.ai_socktype = SOCK_STREAM; - - addrinfo *res = 0; - if (::getaddrinfo(host_.c_str(), portbuf, &hints, &res) != 0 || !res) { - error = "cannot resolve mdsip host " + host_; - return std::string(); - } - int fd = -1; - for (addrinfo *ai = res; ai; ai = ai->ai_next) { - fd = ::socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol); - if (fd < 0) continue; - if (::connect(fd, ai->ai_addr, ai->ai_addrlen) == 0) break; - ::close(fd); - fd = -1; + if (!target_.socket_path.empty()) { + // One connect, no name resolution, no Nagle to disable: a Unix socket + // has neither. The path length check is the only way this can be + // misconfigured, and it is caught at load as well as here. + sockaddr_un addr; + std::memset(&addr, 0, sizeof(addr)); + addr.sun_family = AF_UNIX; + if (target_.socket_path.size() >= sizeof(addr.sun_path)) { + error = "mdsip socket path too long: " + target_.socket_path; + return std::string(); + } + std::strncpy(addr.sun_path, target_.socket_path.c_str(), + sizeof(addr.sun_path) - 1); + fd = ::socket(AF_UNIX, SOCK_STREAM, 0); + if (fd >= 0 && ::connect(fd, reinterpret_cast(&addr), + sizeof(addr)) != 0) { + ::close(fd); + fd = -1; + } + if (fd < 0) { + error = "cannot connect to mdsip at " + target_.socket_path; + return std::string(); + } + } else { + char portbuf[16]; + std::snprintf(portbuf, sizeof(portbuf), "%d", target_.port); + + addrinfo hints; + std::memset(&hints, 0, sizeof(hints)); + hints.ai_family = AF_UNSPEC; + hints.ai_socktype = SOCK_STREAM; + + addrinfo *res = 0; + if (::getaddrinfo(target_.host.c_str(), portbuf, &hints, &res) != 0 || !res) { + error = "cannot resolve mdsip host " + target_.host; + return std::string(); + } + for (addrinfo *ai = res; ai; ai = ai->ai_next) { + fd = ::socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol); + if (fd < 0) continue; + if (::connect(fd, ai->ai_addr, ai->ai_addrlen) == 0) break; + ::close(fd); + fd = -1; + } + ::freeaddrinfo(res); + if (fd < 0) { + error = "cannot connect to mdsip at " + target_.host; + return std::string(); + } + const int one = 1; + ::setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &one, sizeof(one)); } - ::freeaddrinfo(res); - - if (fd < 0) { error = "cannot connect to mdsip at " + host_; return std::string(); } - - const int one = 1; - ::setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &one, sizeof(one)); const std::string token = RandomToken(); if (token.empty()) { ::close(fd); error = "cannot generate a session token"; return std::string(); } diff --git a/src/MdsipSession.hh b/src/MdsipSession.hh index 255805d..93b36c0 100644 --- a/src/MdsipSession.hh +++ b/src/MdsipSession.hh @@ -8,7 +8,26 @@ namespace fdp { -// Holds one mdsip TCP connection per client session. +// Where the relay finds mdsip: a TCP host and port, or a Unix socket path. +// The exthandler line names one form or the other. The Unix form exists +// because the production sandbox is a rootless container: a TCP hop into it +// crosses pasta, which stalls every reply above ~4 MB for a full +// retransmission timeout (measured 2026-10-13, d3d-origin-admin +// docs/specs/2026-10-13-mdsip-unix-socket-design.md). +struct MdsipTarget { + std::string host; // TCP form + int port = 0; // TCP form + std::string socket_path; // Unix form; empty means TCP +}; + +// Raw parameter strings as the directive gave them, "" where it did not. +// Fills `out`, or sets `error` and returns false. +bool ResolveMdsipTarget(const std::string &host_parm, + const std::string &port_parm, + const std::string &socket_parm, + MdsipTarget &out, std::string &error); + +// Holds one mdsip connection (TCP or Unix socket) per client session. // // This exists because mdsip is STATEFUL: openTree sets the tree context on the // connection. A relay that opened a fresh mdsip connection per HTTP request @@ -20,6 +39,9 @@ namespace fdp { // times out. class MdsipSessions { public: + MdsipSessions(const MdsipTarget &target, int idle_seconds, + size_t max_sessions, int timeout_ms); + // The TCP form, kept for callers and tests that name a host and port. MdsipSessions(std::string host, int port, int idle_seconds, size_t max_sessions, int timeout_ms); @@ -64,8 +86,7 @@ private: // relayer if one holds the fd. void Retire(const std::map::iterator &it); - std::string host_; - int port_; + MdsipTarget target_; int idle_seconds_; size_t max_sessions_; int timeout_ms_; diff --git a/tests/integration/run_relay_e2e.sh b/tests/integration/run_relay_e2e.sh index 9084af0..a309cf4 100644 --- a/tests/integration/run_relay_e2e.sh +++ b/tests/integration/run_relay_e2e.sh @@ -16,6 +16,10 @@ PORT=${RELAY_PORT:-10950} HTTP_PORT=${RELAY_HTTP_PORT:-10951} MDSIP_PORT=${RELAY_MDSIP_PORT:-8110} SHIM_PORT=${RELAY_SHIM_PORT:-10952} +# RELAY_SOCKET=/short/path runs the relay against a Unix socket as production +# does; the TCP mdsip still starts, because relay_client_check.py compares a +# relayed session against a direct one. +SOCKET=${RELAY_SOCKET:-} TREES=${FDP_TREES:-/tmp/fdp-trees} RELAY="$ROOT/build/libXrdHttpMdsip.so" # unsuffixed on purpose @@ -56,8 +60,11 @@ sed -e "s#@@RELAY@@#$RELAY#" -e "s#@@MDSIP@@#$MDSIP_PORT#" \ -e "s#@@HTTPPORT@@#$HTTP_PORT#" -e "s#@@PORT@@#$PORT#" -e "s#@@WORK@@#$WORK#" \ "$ROOT/tests/integration/xrootd-relay.cfg" > "$CFG" -MDSIP_PID=""; XRD_PID=""; SHIM_PID="" +MDSIP_PID=""; XRD_PID=""; SHIM_PID=""; INETD_PID="" cleanup() { + [ -n "$INETD_PID" ] && kill "$INETD_PID" 2>/dev/null || true + # The stand-in does not unlink its socket on exit, so a run would leave it. + [ -n "$SOCKET" ] && rm -f "$SOCKET" 2>/dev/null || true [ -n "$SHIM_PID" ] && kill "$SHIM_PID" 2>/dev/null || true [ -n "$XRD_PID" ] && kill "$XRD_PID" 2>/dev/null || true [ -n "$MDSIP_PID" ] && kill "$MDSIP_PID" 2>/dev/null || true @@ -66,6 +73,16 @@ trap cleanup EXIT fail() { echo "FAIL: $1"; echo "--- xrootd log ---"; tail -40 "$WORK/xrootd.log" 2>/dev/null; exit 1; } +# The sandbox runs the container on TCP, so pairing it with a socket would +# exercise the host stand-in while looking like the container topology. +if [ -n "$SOCKET" ] && [ -n "${MDSIP_SANDBOX:-}" ]; then + fail "RELAY_SOCKET with MDSIP_SANDBOX would test the stand-in, not the container; give mdsip-sandbox.sh a socket mode first" +fi +if [ -n "$SOCKET" ]; then + sed -i "s#host=127.0.0.1,port=$MDSIP_PORT#socket=$SOCKET#" "$CFG" + grep -q "socket=$SOCKET" "$CFG" || fail "could not render socket= into the config" +fi + # MDSIP_SANDBOX=1 runs the relay against the containerised mdsip instead of a # bare one on the host -- the production topology, where a client's code # execution lands inside the sandbox rather than on the origin. The relay is @@ -87,6 +104,21 @@ else start_mdsip "$MDSIP_PORT" "$TREES" "$WORK" fi +# The Unix-socket mdsip, with the same trees and TDI path start_mdsip gives the +# TCP one. The sandbox path does not write the hosts file, so write it here. +if [ -n "$SOCKET" ]; then + [ -f "$WORK/mdsip.hosts" ] || printf '* | MAP_TO_LOCAL\n' > "$WORK/mdsip.hosts" + MDS_PATH="$CONDA_PREFIX/tdi;$CONDA_PREFIX/tdi/remote" efit01_path="$TREES" \ + python "$ROOT/tests/mdsip_inetd.py" "$SOCKET" "$WORK/mdsip.hosts" \ + > "$WORK/inetd.log" 2>&1 & INETD_PID=$! + for _ in $(seq 1 100); do + kill -0 "$INETD_PID" 2>/dev/null || fail "the Unix-socket mdsip exited: $(cat "$WORK/inetd.log")" + [ -S "$SOCKET" ] && break + sleep 0.1 + done + [ -S "$SOCKET" ] || fail "the Unix-socket mdsip never listened: $(cat "$WORK/inetd.log")" +fi + xrootd -c "$CFG" -l "$WORK/xrootd.log" >/dev/null 2>&1 & XRD_PID=$! for _ in $(seq 1 150); do kill -0 "$XRD_PID" 2>/dev/null || fail "xrootd exited during startup" @@ -101,6 +133,11 @@ done grep -q "XrdHttpMdsip relay" "$WORK/xrootd.log" || fail "the relay handler was never loaded" echo "--- 0. handler loaded ---" grep "XrdHttpMdsip relay" "$WORK/xrootd.log" | tail -1 +if [ -n "$SOCKET" ]; then + grep -q "mdsip unix:$SOCKET" "$WORK/xrootd.log" || fail "banner does not name the Unix socket" +else + grep -q "mdsip 127.0.0.1:$MDSIP_PORT" "$WORK/xrootd.log" || fail "banner does not name the TCP target" +fi python "$ROOT/tests/integration/mdsip_http_client.py" \ "$SHIM_PORT" "127.0.0.1:$HTTP_PORT" /mdsip > "$WORK/shim.log" 2>&1 & SHIM_PID=$! @@ -155,6 +192,21 @@ else python "$ROOT/tests/integration/relay_client_check.py" \ || fail "the C transport did not match the direct connection" + echo "--- 6b. a multi-megabyte reply is not stalled ---" + LD_LIBRARY_PATH="$ROOT/build:${LD_LIBRARY_PATH:-}" FDP_TUNNEL_SCHEME=http \ + python - "$HTTP_PORT" <<'PYEOF' || fail "large replies stall through the relay" +import sys, time +from MDSplus import Connection +c = Connection("fdp://127.0.0.1:%s/mdsip" % sys.argv[1]) +c.get("zero(1000,1.0)") +ms = [] +for _ in range(5): + t = time.time(); c.get("zero(2000000,1.0)"); ms.append(1000 * (time.time() - t)) +ms.sort() +print("8 MB replies, ms:", [round(x) for x in ms]) +sys.exit(0 if ms[2] < 100 else 1) +PYEOF + echo "--- 7. transport lifecycle and error paths ---" # mdsip -m forks a process per connection, so a leaked relay session is a # leaked mdsip process. Counting them is the only check here that would @@ -167,7 +219,15 @@ else # One process per connection, so no -m in the pattern. Matching the old # "mdsip -m -p" here would find nothing at all, and a counter stuck at 0 # reports "no leak" for every possible outcome. - pgrep -c -f "mdsip -p $MDSIP_PORT" 2>/dev/null || echo 0 + # In socket mode the relay's sessions land on the Unix-served mdsip, + # whose -p is the nominal 8000, so count those instead. + if [ -n "$SOCKET" ]; then + n="$(pgrep -c -f "mdsip -p 8000 -h $WORK/mdsip.hosts" 2>/dev/null)" || true + echo "${n:-0}" + else + n="$(pgrep -c -f "mdsip -p $MDSIP_PORT" 2>/dev/null)" || true + echo "${n:-0}" + fi fi } diff --git a/tests/mdsip_inetd.py b/tests/mdsip_inetd.py index 68624b3..cb15244 100644 --- a/tests/mdsip_inetd.py +++ b/tests/mdsip_inetd.py @@ -10,6 +10,11 @@ than no test server, because it makes the difference invisible. python tests/mdsip_inetd.py + python tests/mdsip_inetd.py + +The second form listens on a Unix socket, as the sandbox does in production +(FDP_MDSIP_SOCKET in scripts/fdp-mdsip-server). mdsip serves an AF_UNIX peer on +fd 0 just as it serves a TCP one; its -p port is then only nominal. """ import os @@ -21,16 +26,34 @@ def main(): - port, hostfile = int(sys.argv[1]), sys.argv[2] + target, hostfile = sys.argv[1], sys.argv[2] # Reap children automatically; nothing here waits on them. signal.signal(signal.SIGCHLD, signal.SIG_IGN) - srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - srv.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) - srv.bind(("127.0.0.1", port)) + if target.startswith("/"): + # A stale socket file from an earlier run would make bind fail with + # EADDRINUSE, as socat's unlink-early avoids in the image. + try: + os.unlink(target) + except FileNotFoundError: + pass + srv = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + srv.bind(target) + # Same reachability as the image's mode=666. + os.chmod(target, 0o666) + port = 8000 # nominal: mdsip never binds it here + where = target + else: + if not target.isdigit(): + sys.exit("first argument must be a TCP port or an absolute socket path") + port = int(target) + srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + srv.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + srv.bind(("127.0.0.1", port)) + where = str(port) srv.listen(128) - print("mdsip_inetd listening on %d" % port, flush=True) + print("mdsip_inetd listening on %s" % where, flush=True) while True: conn, _ = srv.accept() diff --git a/tests/test_mdsipsession.cpp b/tests/test_mdsipsession.cpp index 3cc4674..48da853 100644 --- a/tests/test_mdsipsession.cpp +++ b/tests/test_mdsipsession.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -32,6 +33,10 @@ std::string BodyOf(const std::string &msg) { return msg.substr(fdp::kMdsipHeaderBytes); } +std::string TestSocketPath() { + return "/tmp/fdp-relay-test-" + std::to_string(::getpid()) + ".sock"; +} + // Stands in for mdsip: accepts connections and, per connection, echoes each // message back with its body upper-cased so a reply can be told from a request. class FakeMdsip { @@ -59,10 +64,30 @@ class FakeMdsip { acceptor_ = std::thread(&FakeMdsip::Accept, this); } + explicit FakeMdsip(const std::string &unix_path) + : listen_fd_(-1), port_(0), stop_(false), connections_(0), + unix_path_(unix_path) { + ::unlink(unix_path.c_str()); + listen_fd_ = ::socket(AF_UNIX, SOCK_STREAM, 0); + REQUIRE(listen_fd_ >= 0); + + sockaddr_un addr; + std::memset(&addr, 0, sizeof(addr)); + addr.sun_family = AF_UNIX; + REQUIRE(unix_path.size() < sizeof(addr.sun_path)); + std::strncpy(addr.sun_path, unix_path.c_str(), sizeof(addr.sun_path) - 1); + REQUIRE(::bind(listen_fd_, reinterpret_cast(&addr), + sizeof(addr)) == 0); + REQUIRE(::listen(listen_fd_, 16) == 0); + + acceptor_ = std::thread(&FakeMdsip::Accept, this); + } + ~FakeMdsip() { stop_ = true; if (listen_fd_ >= 0) ::shutdown(listen_fd_, SHUT_RDWR); if (listen_fd_ >= 0) ::close(listen_fd_); + if (!unix_path_.empty()) ::unlink(unix_path_.c_str()); if (acceptor_.joinable()) acceptor_.join(); // Wake the per-connection readers rather than letting each burn its @@ -117,6 +142,7 @@ class FakeMdsip { std::vector workers_; std::mutex mutex_; std::vector clients_; + std::string unix_path_; }; } // namespace @@ -321,3 +347,109 @@ TEST_CASE("concurrent calls on one session are refused, not interleaved") { CHECK(succeeded + refused == 8); CHECK(succeeded >= 1); } + +TEST_CASE("the mdsip target defaults to localhost:8000 over TCP") { + fdp::MdsipTarget t; + std::string error; + REQUIRE(fdp::ResolveMdsipTarget("", "", "", t, error)); + CHECK(t.socket_path.empty()); + CHECK(t.host == "localhost"); + CHECK(t.port == 8000); +} + +TEST_CASE("host= and port= name a TCP target") { + fdp::MdsipTarget t; + std::string error; + REQUIRE(fdp::ResolveMdsipTarget("10.88.0.1", "8100", "", t, error)); + CHECK(t.socket_path.empty()); + CHECK(t.host == "10.88.0.1"); + CHECK(t.port == 8100); +} + +TEST_CASE("socket= names a Unix target and leaves host and port unset") { + fdp::MdsipTarget t; + std::string error; + REQUIRE(fdp::ResolveMdsipTarget("", "", "/run/fdp-mdsip/mdsip.sock", t, error)); + CHECK(t.socket_path == "/run/fdp-mdsip/mdsip.sock"); + CHECK(t.host.empty()); + CHECK(t.port == 0); +} + +TEST_CASE("socket= together with host= or port= is refused") { + fdp::MdsipTarget t; + std::string error; + CHECK_FALSE(fdp::ResolveMdsipTarget("localhost", "", "/run/x.sock", t, error)); + CHECK(error.find("socket=") != std::string::npos); + error.clear(); + CHECK_FALSE(fdp::ResolveMdsipTarget("", "8000", "/run/x.sock", t, error)); + CHECK(error.find("socket=") != std::string::npos); +} + +TEST_CASE("a socket= path that cannot fit sun_path is refused at resolve time") { + fdp::MdsipTarget t; + std::string error; + CHECK_FALSE(fdp::ResolveMdsipTarget("", "", "/" + std::string(200, 'x'), t, error)); + CHECK(error.find("mdsip socket path too long") != std::string::npos); +} + +TEST_CASE("an unusable port is refused with the text that was given") { + fdp::MdsipTarget t; + std::string error; + CHECK_FALSE(fdp::ResolveMdsipTarget("", "70000", "", t, error)); + CHECK(error.find("70000") != std::string::npos); + CHECK_FALSE(fdp::ResolveMdsipTarget("", "eighty", "", t, error)); +} + +TEST_CASE("the port parse is exact at both ends of the range and refuses trailing text") { + fdp::MdsipTarget t; + std::string error; + REQUIRE(fdp::ResolveMdsipTarget("", "1", "", t, error)); + CHECK(t.port == 1); + REQUIRE(fdp::ResolveMdsipTarget("", "65535", "", t, error)); + CHECK(t.port == 65535); + CHECK_FALSE(fdp::ResolveMdsipTarget("", "0", "", t, error)); + CHECK_FALSE(fdp::ResolveMdsipTarget("", "65536", "", t, error)); + CHECK_FALSE(fdp::ResolveMdsipTarget("", "80abc", "", t, error)); + CHECK(error.find("80abc") != std::string::npos); +} + +TEST_CASE("a relayed call over a Unix socket returns exactly one answer") { + const std::string path = TestSocketPath(); + FakeMdsip server(path); + fdp::MdsipTarget target; + target.socket_path = path; + fdp::MdsipSessions sessions(target, 300, 0, 5000); + + std::string error; + const std::string token = sessions.Open(error); + REQUIRE_MESSAGE(!token.empty(), error); + + std::string answer; + REQUIRE(sessions.Relay(token, Message("hello"), answer, error)); + CHECK(BodyOf(answer) == "HELLO"); + CHECK(server.connections() == 1); + sessions.Close(token); +} + +TEST_CASE("a Unix target with no socket behind it fails to open, naming the path") { + const std::string path = TestSocketPath() + ".absent"; + fdp::MdsipTarget target; + target.socket_path = path; + fdp::MdsipSessions sessions(target, 300, 0, 5000); + + std::string error; + const std::string token = sessions.Open(error); + CHECK(token.empty()); + CHECK(error.find("cannot connect to mdsip at " + path) != std::string::npos); +} + +TEST_CASE("a Unix socket path that cannot fit sun_path is refused, not truncated") { + fdp::MdsipTarget target; + target.socket_path = "/" + std::string(200, 'x') + ".sock"; + fdp::MdsipSessions sessions(target, 300, 0, 5000); + + std::string error; + const std::string token = sessions.Open(error); + CHECK(token.empty()); + CHECK(error.find("mdsip socket path too long") != std::string::npos); +}