Skip to content

feat(client): add resilient sync primitives for large and DFS-backed shares - #4

Open
kalicyh wants to merge 9 commits into
vdavid:mainfrom
kalicyh:feat/streaming-directory-reader
Open

feat(client): add resilient sync primitives for large and DFS-backed shares#4
kalicyh wants to merge 9 commits into
vdavid:mainfrom
kalicyh:feat/streaming-directory-reader

Conversation

@kalicyh

@kalicyh kalicyh commented Aug 3, 2026

Copy link
Copy Markdown

Summary

This remains one integrated PR for long-running SMB sync and backup workflows. It adds the primitives needed to enumerate large directories incrementally, identify and mutate the exact remote object, checkpoint streamed writes safely, and keep owned handles pinned to the correct DFS target and SMB session.

What changes

Incremental directory enumeration

Adds DirectoryReader, exposed through Tree and SmbClient.

  • next_batch() returns one server QUERY_DIRECTORY response at a time; list_directory() collects the same reader internally.
  • EOF closes automatically; early exit supports explicit close().
  • Cancelling a QUERY or EOF CLOSE retains the same in-flight future, so the server cursor cannot silently skip a batch.
  • Readers capture the connection generation and SessionId; stale TreeId/FileId values are never reused after reconnect.
  • The reader stores only the copied TreeId, avoiding a cloned Tree and its strings.
  • Queries prefer the compact 80-byte FileIdFullDirectoryInformation layout. Servers that return STATUS_NOT_SUPPORTED or STATUS_INVALID_INFO_CLASS are restarted on the same open handle with FileBothDirectoryInformation.

Stable identity and exact mutations

  • DirectoryEntry exposes changed and optional file_index.
  • FileInfo exposes changed and optional FileIdentity; stable identity requires both a non-zero file index and its volume serial.
  • RenameOptions maps directly to the server's atomic ReplaceIfExists behavior.
  • MutationHandle obtains metadata and renames or deletes through the same open FileId, avoiding a path-based stat-to-mutation race.
  • Mutable handles use session-bound requests. A reconnect cannot send an old TreeId or FileId into the replacement session.
  • If an optional identity query cascades its failure into a related compound CLOSE, single and batch stat issue a standalone CLOSE instead of leaking the successful CREATE handle.

Durable writer checkpoints and safe resume

Adds FileWriter::flush_checkpoint().

A successful checkpoint sends all accepted data, drains outstanding WRITEs, issues SMB FLUSH, and returns the absolute durable file offset without closing the handle. The durable offset advances only after FLUSH succeeds; cancellation keeps accepted data owned by the writer, and short WRITE responses are rejected.

open_existing_file_writer_at() now requires the checkpoint's expected FileIdentity. A compounded FileOpen plus two QUERY_INFO operations validates identity and CREATE's current EndOfFile on the exact handle retained for writing. Missing, replaced, identity-unavailable, or truncated targets are closed and rejected before any WRITE.

DFS and reconnect safety

  • Routed owned-handle variants resolve an initial DFS referral before returning the handle and never replay an ambiguous writer CREATE.
  • Target tree-connects are cached per (server, share, connection generation, SessionId).
  • A dead extra connection is replaced; stale routed trees are removed before a fresh TREE_CONNECT.
  • Mutable writers, directory readers, mutation handles, and related compounds are bound to the session that allocated their IDs.
  • Credit reservations retain the exact semaphore generation they debited, so an old unsent request cannot refund credits into a newly reset connection.

Credential handling

  • Passwords are redacted from ClientConfig and Kerberos credential Debug output.
  • Retained client, reviver, NTLM, and Kerberos credential buffers are zeroized on drop.
  • Adds the zeroize dependency.

Public API and compatibility

New public API includes DirectoryReader, FileIdentity, RenameOptions, MutationHandle, FileWriter::flush_checkpoint(), safe existing-file positioned writer opens, and DFS-routed owned-handle opens.

Source-compatibility notes:

  • DirectoryEntry gains changed and file_index.
  • FileInfo gains changed and identity.
  • Both metadata structs are #[non_exhaustive].
  • open_existing_file_writer_at() requires an expected FileIdentity.
  • Credential structs implement Drop, so callers must borrow or clone individual public fields instead of moving them out.
  • stat remains one normal wire round trip but expands from four to six compound operations; the two identity queries are optional.
  • Directory enumeration prefers FileIdFullDirectoryInformation and has a FileBothDirectoryInformation fallback.
  • zeroize is a new runtime dependency.

Existing list_directory, rename, and FileWriter::finish entry points remain available.

Validation

Current head: 7663ab1 (rebased onto upstream v0.18.1, b5baae4)

  • just passes locally: formatting, Clippy (regular and all features), tests (regular and all features), and documentation.
  • Default tests: 1,044 passed, 1 ignored; all features: 1,066 passed, 1 ignored.
  • cargo clippy --all-targets --all-features -- -D warnings passes.
  • MSRV 1.85 check passes.
  • The new reconnect, credit-generation, resume identity/length, stat cleanup, directory fallback, and cancellation tests pass.
  • The pre-rebase head passed full fork CI, including Linux/macOS/Windows, MSRV, Docker, and consumer integration. For the rebased head, PR CI is the authoritative Docker/OS validation once it is available or approved.
  • No live NAS validation is claimed.

Changelog

No CHANGELOG.md entry is included; release-note wording is left to the maintainer.

Copilot AI review requested due to automatic review settings August 3, 2026 02:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new incremental, bounded-memory directory enumeration API (DirectoryReader) to the SMB2 client layer, integrating it into existing listing functionality while preserving cancellation safety for stateful QUERY_DIRECTORY enumeration.

Changes:

  • Introduce DirectoryReader with next_batch() and explicit close(); re-export it from client and crate root.
  • Add DFS-aware SmbClient::open_directory_reader and refactor Tree::list_directory to collect via the new reader (single wire implementation).
  • Expand documentation and benchmarks notes; add unit tests covering batching, EOF close, early close, and cancellation-resume behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/lib.rs Re-exports DirectoryReader from the crate root.
src/client/tree.rs Implements DirectoryReader, refactors list_directory to use it, and adds focused mock-transport tests.
src/client/mod.rs Exposes DFS-aware SmbClient::open_directory_reader and re-exports DirectoryReader.
src/client/CLAUDE.md Documents the new DirectoryReader design and cancellation-safety invariant.
README.md Adds the new API to the “one thing and get the result” list.
docs/benchmark-findings.md Updates listing compounding notes to reflect the “no last batch marker” constraint.
CHANGELOG.md Adds an Unreleased entry describing incremental directory enumeration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/client/tree.rs
@kalicyh
kalicyh force-pushed the feat/streaming-directory-reader branch from 088be61 to cdc2a75 Compare August 3, 2026 03:51
@kalicyh
kalicyh requested a review from Copilot August 3, 2026 03:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/client/tree.rs:583

  • Tree::list_directory now calls open_directory_reader, which already normalizes the path and emits a TRACE log. The additional trace!("tree: list_directory path=...", self.format_path(path)) both duplicates log volume and forces a second path normalization/allocation (including DFS prefix formatting) for every listing when TRACE is enabled.
        // TRACE, not DEBUG: a recursive scan calls list_directory once per directory
        // (millions of times on a large share), so at DEBUG it dominates a consumer's
        // log. Per-operation mutations (rename/delete/write) stay at DEBUG. See AGENTS.md.
        trace!("tree: list_directory path={}", self.format_path(path));

@gitguardian

gitguardian Bot commented Aug 3, 2026

Copy link
Copy Markdown

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@kalicyh
kalicyh force-pushed the feat/streaming-directory-reader branch from fae5bf9 to 40e5bb7 Compare August 3, 2026 17:09
@kalicyh kalicyh changed the title feat: add cancellation-safe incremental directory enumeration feat(client): add resilient sync primitives for large and DFS-backed shares Aug 3, 2026
@kalicyh
kalicyh force-pushed the feat/streaming-directory-reader branch from 40e5bb7 to d874fba Compare August 4, 2026 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants