From f4d2bdfe5c042a6b33459d75314c22d784cb9f2c Mon Sep 17 00:00:00 2001 From: Ge Yao Date: Sun, 24 May 2026 22:45:49 +0800 Subject: [PATCH] Reply '@ERROR' on unknown module so rsync clients exit non-zero Real rsyncd answers a request for an unknown module with "@ERROR: Unknown module ''". The rsync client recognises that prefix and exits with code 5 (RERR_FERROR_XFER), surfacing the configuration breakage to whatever invoked rsync. rsync-proxy was instead writing "unknown module: \n@RSYNCD: EXIT\n". The "@RSYNCD: EXIT" makes rsync treat the response as a normal end-of-listing and exit 0, so downstream tooling (notably tunasync) marked failed jobs as 'success'. NJU's loongnix mirror was stale for ~10 months because of this: USTC retired the module, the proxy returned 'unknown module', and tunasync recorded a successful run with no actual data transfer. Drop the trailing '@RSYNCD: EXIT' and use '@ERROR:' so the client treats it as fatal, matching rsyncd. Add unit and end-to-end regression tests. --- pkg/server/server.go | 10 ++++++++-- pkg/server/server_test.go | 32 ++++++++++++++++++++++++++++++++ test/e2e/e2e_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/pkg/server/server.go b/pkg/server/server.go index a2302e6..a8e0eba 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -542,8 +542,14 @@ func (s *Server) relay(ctx context.Context, index uint32, downConn net.Conn) err targets, ok := s.getTargetsForModule(moduleName) if !ok { - _, _ = writeWithTimeout(downConn, fmt.Appendf(nil, "unknown module: %s\n", moduleName), writeTimeout) - _, _ = writeWithTimeout(downConn, RsyncdExit, writeTimeout) + // Use the rsyncd "@ERROR:" wire format so that the rsync + // client treats this as a fatal protocol error and exits with + // a non-zero status (RERR_FERROR_XFER, exit 5), matching the + // behavior of a real rsyncd. Sending only "unknown module: + // ...\n" followed by "@RSYNCD: EXIT" caused the client to + // exit 0, which masked the failure for downstream tools such + // as tunasync (which then marked the job as success). + _, _ = writeWithTimeout(downConn, fmt.Appendf(nil, "@ERROR: Unknown module '%s'\n", moduleName), writeTimeout) s.accessLog.F("client %s requests non-existing module %s", ip, moduleName) return nil } diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 291c453..7480721 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -129,6 +129,38 @@ func TestMotdFromServer(t *testing.T) { r.Equal(proxyMotd+"\n"+serverMotd, string(allData)) } +// TestUnknownModuleSendsErrorPrefix verifies that requesting a module that +// the proxy is not configured to serve makes the proxy reply with an +// "@ERROR:" line, matching real rsyncd's wire format. The rsync client +// treats an "@ERROR:" reply as a fatal protocol error and exits 5, +// while a plain message followed by "@RSYNCD: EXIT" causes the client +// to exit 0, which historically masked configuration breakage in tools +// such as tunasync. +func TestUnknownModuleSendsErrorPrefix(t *testing.T) { + srv := startServer(t) + defer srv.Close() + + srv.modules = map[string][]Target{} + srv.upstreamQueues = map[string]*queue.Queue{} + + r := require.New(t) + + rawConn, err := net.Dial("tcp", srv.TCPListener.Addr().String()) + r.NoError(err) + conn := rsync.NewConn(rawConn) + defer conn.Close() + + _, err = doClientHandshake(conn, RsyncdServerVersion, "does-not-exist") + r.NoError(err) + + allData, err := io.ReadAll(conn) + r.NoError(err) + + r.Contains(string(allData), "@ERROR:", "proxy should reply with @ERROR: prefix so client exits non-zero") + r.Contains(string(allData), "does-not-exist") + r.NotContains(string(allData), "@RSYNCD: EXIT", "should not send EXIT after @ERROR; rsync client must treat the response as fatal") +} + // See also: https://github.com/ustclug/rsync-proxy/commit/d581c18dab8008c5bc9c1a5d667b49d67a4edfed func TestClientReadTimeout(t *testing.T) { srv := startServer(t) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 8ffa75c..42665d8 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -5,6 +5,7 @@ import ( "fmt" "net" "os" + "os/exec" "path/filepath" "testing" @@ -247,3 +248,27 @@ func TestDiscoverModulesFromUpstream(t *testing.T) { r.Equal("bar\nbaz\nfoo\n", string(outputBytes)) } + +// TestUnknownModuleExitCode verifies that a real rsync client exits with +// a non-zero status when it requests a module that the proxy does not +// know about. This is a regression test for an earlier behavior where +// the proxy emitted "unknown module: \n@RSYNCD: EXIT\n", which +// rsync interprets as a normal end-of-listing (exit 0). The fix makes +// the proxy emit "@ERROR:" instead, matching real rsyncd; rsync then +// treats it as a fatal protocol error and exits with code 5 +// (RERR_FERROR_XFER). +func TestUnknownModuleExitCode(t *testing.T) { + proxy := startProxy(t) + + r := require.New(t) + + cmd := newRsyncCommand(getRsyncPath(proxy, "/does-not-exist/")) + out, err := cmd.CombinedOutput() + r.Error(err, "rsync should fail when module does not exist; got output: %s", out) + + exitErr, ok := err.(*exec.ExitError) + r.True(ok, "expected *exec.ExitError, got %T: %v", err, err) + r.NotEqual(0, exitErr.ExitCode(), "rsync should exit non-zero on unknown module; output: %s", out) + r.Contains(string(out), "@ERROR", "rsync should report the @ERROR line; output: %s", out) + r.Contains(string(out), "does-not-exist", "rsync should report the module name; output: %s", out) +}