Reply with @ERROR on unknown module so clients exit non-zero#32
Merged
Conversation
Real rsyncd answers a request for an unknown module with "@error: Unknown module '<name>'". 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: <name>\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.
There was a problem hiding this comment.
Pull request overview
Aligns rsync-proxy’s “unknown module” response with real rsyncd wire behavior so rsync clients treat it as a fatal protocol error (non-zero exit), preventing silent “success” on misconfiguration or retired modules.
Changes:
- Emit
@ERROR: Unknown module '<name>'(and stop sending@RSYNCD: EXIT) when a requested module isn’t configured. - Add a unit test to assert the server replies with
@ERROR:and does not send@RSYNCD: EXITfor unknown modules. - Add an e2e test that runs a real
rsyncclient and asserts it exits non-zero and reports the@ERRORline.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
pkg/server/server.go |
Changes unknown-module response to @ERROR: format to trigger non-zero rsync client exit. |
pkg/server/server_test.go |
Adds regression test ensuring @ERROR: is sent and @RSYNCD: EXIT is not sent for unknown modules. |
test/e2e/e2e_test.go |
Adds e2e regression test verifying real rsync exits non-zero and includes @ERROR on unknown module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
taoky
approved these changes
May 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a client requests a module that the proxy is not configured to serve,
rsync-proxycurrently writes:A real
rsyncdanswers the same situation with:Rsync's client treats
@RSYNCD: EXITas a normal end-of-listing and exits 0, while it treats an@ERROR:line as a fatal protocol error and exits 5 (RERR_FERROR_XFER). The current proxy reply therefore looks like a successful no-op to anything that checks rsync's exit code.Impact
This silently broke long-running mirrors. Concrete example: NJU's
loongnixmirror runs daily againstrsync://rsync.mirrors.ustc.edu.cn/loongnix/. USTC retired that module roughly 10 months ago, so the proxy began returningunknown module: loongnix, but tunasync kept marking each run assuccessbecause rsync exited 0. Local data sat at the July 2024 snapshot for the better part of a year while the dashboard stayed green.You can reproduce the difference today against any healthy upstream:
The wire capture (via
nc) confirms the proxy is sending plain text rather than@ERROR::The relevant rsync source (
clientserver.c):Fix
Drop the trailing
@RSYNCD: EXITand prefix the message with@ERROR:so the wire format matchesrsyncd. The client closes the connection on its own and exits with a non-zero status. The format string mirrors rsyncd's exactly (@ERROR: Unknown module '<name>').Tests
pkg/server:TestUnknownModuleSendsErrorPrefixasserts the proxy emits@ERROR:and does not emit@RSYNCD: EXITfor an unknown module. Verified red/green by reverting the fix locally — the test fails on the old behavior.test/e2e:TestUnknownModuleExitCodedrives a realrsyncbinary at the proxy, assertscmd.Run()returns a non-zero*exec.ExitError, and that the captured output contains@ERRORand the requested module name. Both new tests pass undergo test -race ./...together with the existing suite.No behavior change for known modules, and no protocol/version bump.