Skip to content

fix(client): forward --iconv to remote rsync server args#3565

Merged
oferchen merged 1 commit into
masterfrom
fix/iconv-server-arg-forward
May 2, 2026
Merged

fix(client): forward --iconv to remote rsync server args#3565
oferchen merged 1 commit into
masterfrom
fix/iconv-server-arg-forward

Conversation

@oferchen
Copy link
Copy Markdown
Owner

@oferchen oferchen commented May 2, 2026

Summary

Adds --iconv to the long-form server-args forwarded by RemoteInvocationBuilder::append_long_form_args. Without this, the spawned rsync --server ran with no iconv context and emitted raw bytes on the wire - gap 1 of the standalone:iconv-local-ssh known failure.

The mapping mirrors upstream options.c:2716-2723 exactly:

IconvSetting variant Forwarded flag
Unspecified (none)
Disabled (none - upstream nulls iconv_opt at options.c:2052-2054)
LocaleDefault --iconv=.
Explicit { local, remote: Some(r) } --iconv={r} (post-comma half only)
Explicit { local, remote: None } --iconv={local} (no comma => forward whole)

Five new unit tests cover each row.

Upstream Reference

// options.c:2716-2723
if (iconv_opt) {
    char *set = strchr(iconv_opt, ',');
    if (set) set++;          // points to char *after* the comma -> "REMOTE"
    else set = iconv_opt;
    args[ac++] = safe_arg("--iconv", set);
}

Scope: this does NOT close the KF

This is a focused fix for gap 1 only. The standalone:iconv-local-ssh known failure stays open because:

  • Gap 2: UTF-8 wire mismatch on the file-list side is still unresolved
  • Iconv audit findings 1, 2, 3, and 5 remain open

The KF entry in tools/ci/known_failures.conf is intentionally unchanged.

Test plan

  • CI: fmt + clippy
  • CI: nextest (stable) - 5 new iconv_* tests in crates/core/src/client/remote/invocation/tests.rs
  • CI: Windows / macOS / Linux musl matrices
  • Confirm standalone:iconv-local-ssh remains in KNOWN_FAILURES (intentional)

The remote invocation builder did not include --iconv among the long-form
flags it forwards to the spawned `rsync --server`, so the remote peer ran
without an iconv context and emitted raw bytes on the wire. This closes
gap 1 of the standalone:iconv-local-ssh known failure.

Mapping mirrors upstream options.c:2716-2723 exactly: when iconv_opt
contains a comma, only the post-comma half (the remote charset) is
forwarded; without a comma the whole spec is forwarded; --iconv=- and the
default forward nothing because options.c:2052-2054 nulls iconv_opt
before this branch runs.

This change does NOT close the standalone:iconv-local-ssh KF on its own:
gap 2 (the UTF-8 wire mismatch on the file-list side) and the iconv
audit findings 1, 2, 3, and 5 remain open. The KF entry in
tools/ci/known_failures.conf stays unchanged.
@github-actions github-actions Bot added the bug Something isn't working label May 2, 2026
@oferchen oferchen merged commit f2d48ba into master May 2, 2026
38 checks passed
@oferchen oferchen deleted the fix/iconv-server-arg-forward branch May 2, 2026 16:54
oferchen added a commit that referenced this pull request May 5, 2026
The remote invocation builder did not include --iconv among the long-form
flags it forwards to the spawned `rsync --server`, so the remote peer ran
without an iconv context and emitted raw bytes on the wire. This closes
gap 1 of the standalone:iconv-local-ssh known failure.

Mapping mirrors upstream options.c:2716-2723 exactly: when iconv_opt
contains a comma, only the post-comma half (the remote charset) is
forwarded; without a comma the whole spec is forwarded; --iconv=- and the
default forward nothing because options.c:2052-2054 nulls iconv_opt
before this branch runs.

This change does NOT close the standalone:iconv-local-ssh KF on its own:
gap 2 (the UTF-8 wire mismatch on the file-list side) and the iconv
audit findings 1, 2, 3, and 5 remain open. The KF entry in
tools/ci/known_failures.conf stays unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant