feat: implement Write handler for E2E file write#21
Open
tejas-claude-bot[bot] wants to merge 15 commits into
Open
feat: implement Write handler for E2E file write#21tejas-claude-bot[bot] wants to merge 15 commits into
tejas-claude-bot[bot] wants to merge 15 commits into
Conversation
Add the server-side handlers needed for a complete file read flow: Create → QueryInfo → Read → Close. Protocol types: - Add accessor methods to SMBCloseRequest, SMBReadRequest, SMBQueryInfoRequest - Add constructors for SMBCloseResponse, SMBReadResponse, SMBQueryInfoResponse - Make close::flags and query_info::info_type modules public Trait extensions: - Add read_data() to ResourceHandle trait with implementations for SMBFileSystemHandle (seeks + reads) and SMBIPCHandle (stub) - Add read_data() to Open trait, delegating to the underlying handle - Add open_table_mut() to Session trait for close cleanup - Add remove_open() to Server trait for global open table cleanup - Fix SMBOpen::inner() todo!() → return None (terminal handler) Handlers (tree_connect.rs): - handle_close: returns file metadata if POSTQUERY_ATTRIB flag set, removes open from server (outer) then session (inner) tables - handle_read: reads data from the open's underlying handle, enforces minimum_count, returns SMBReadResponse - handle_query_info: supports FileBasicInformation (class 4), FileStandardInformation (class 5), FileNetworkOpenInformation (class 34) per MS-FSCC; returns InvalidInfoClass for unsupported types - Fix lock ordering in handle_create: server write before session write Lock ordering: all handlers acquire locks outer→inner (server → connection → session → open), readers before writers. NTStatus: add FileClosed, EndOfFile, InvalidInfoClass, InvalidDeviceRequest Tests: - 13 unit tests for request accessor and response constructor round-trips - 3 integration tests (smbclient E2E: file read, dir listing, missing file)
- Fix SMBCreateRequest NameOffset subtract (68→64): offset is relative to SMB2 header start (64 bytes), not 68. This caused 4-byte misalignment producing leading NUL chars and filename truncation. - Fix SMBInfoType enum values to match MS-SMB2 spec: File=1, Filesystem=2, Security=3, Quota=4 (was incorrectly starting at 0). - Fix SMBQueryInfoResponse StructureSize: 9 per MS-SMB2 2.2.38, not 17. - Add FileAllInformation (class 18) handler in QueryInfo dispatch per MS-FSCC 2.4.2 (composite of Basic+Standard+Internal+EA+Access+Position+ Mode+Alignment+Name). - Fix tree_id in TreeConnect response header (use dynamic ID, not hardcoded 1). - Add SMB_SHARE_PATH env var support in main.rs for configurable share root. - Sanitize file paths in SMBFileSystemShare::handle_create: strip trailing NUL terminators and convert backslashes to forward slashes. - Improve integration test diagnostics: capture smbclient stdout/stderr, verify file contents in file_read test. All 66 unit tests and 10 integration tests pass.
Introduce protocol::body::file_info module with proper typed structs for MS-FSCC file information classes: - FileBasicInformation (2.4.7) - FileStandardInformation (2.4.41) - FileInternalInformation (2.4.20) - FileEaInformation (2.4.12) - FileAccessInformation (2.4.1) - FilePositionInformation (2.4.35) - FileModeInformation (2.4.26) - FileAlignmentInformation (2.4.3) - FileNameInformation (2.4.28) - FileNetworkOpenInformation (2.4.29) - FileAllInformation (2.4.2) All types use SMBFromBytes/SMBToBytes/SMBByteSize derive macros for automatic serialization, replacing manual Vec<u8> byte-pushing in tree_connect.rs build_file_* methods. Includes 14 unit tests (round-trip serialization + size assertions). All 80 unit tests and 10 integration tests pass.
…vention Move each MS-FSCC file information struct out of mod.rs into its own file (access.rs, alignment.rs, basic.rs, ea.rs, internal.rs, mode.rs, name.rs, network_open.rs, position.rs, standard.rs). mod.rs now only contains module declarations, re-exports, and the composite FileAllInformation type. All 80 unit tests and 10 integration tests pass.
…rification
B1: file_id() now uses global_id for persistent (DurableFileId) and
session_id for volatile (FileId) per MS-SMB2 §2.2.14.1 / §3.3.1.10.
B2: handle_close remove_open now passes file_id.persistent (global_id)
to the server's GlobalOpenTable instead of file_id.volatile.
B3: find_open and handle_close now verify Open.DurableFileId ==
FileId.Persistent per MS-SMB2 §3.3.5.10/12/20, returning
STATUS_FILE_CLOSED on mismatch.
B4: handle_create response header now passes 0 (STATUS_SUCCESS) instead
of echoing the request's ChannelSequence into the response Status
field (MS-SMB2 §2.2.1.2).
B6: handle_close logs warn! when server/connection lock acquisition
fails during GlobalOpenTable cleanup.
B7: FileAllInformation::to_bytes() capacity hint now accounts for the
variable-length file name.
Tests: added SMBFileId wire-format unit tests (size, layout, round-trip,
field independence).
…es derive FileAllInformation now uses #[derive(SMBByteSize, SMBFromBytes, SMBToBytes)] with #[smb_direct(start(fixed = N))] annotations for each sub-struct at its computed offset. Removes the manual to_bytes() impl. Adds a round-trip test for FileAllInformation (serialize + deserialize).
- FileAccessInformation.access_flags: u32 → FileAccessFlags bitflags (MS-FSCC §2.4.1 / MS-DTYP §2.4.3 ACCESS_MASK) - FileAlignmentInformation.alignment_requirement: u32 → FileAlignmentRequirement enum (MS-FSCC §2.4.3 device alignment boundaries) - FileModeInformation.mode: u32 → FileModeFlags bitflags (MS-FSCC §2.4.26 file mode flags) FileEaInformation.ea_size remains u32 — it is a plain byte count per MS-FSCC §2.4.12, not flags or an enum. All types exported from file_info module. Tests and tree_connect.rs call site updated to use typed constructors.
Security & correctness fixes: - Fix path traversal vulnerability: normalize_path() rejects "../" escaping the share root (STATUS_ACCESS_DENIED) - Cap read_data buffer to 8 MB (MAX_READ_SIZE) to prevent OOM from malicious clients per MS-SMB2 §3.3.5.12 - Return STATUS_END_OF_FILE when read returns 0 bytes at/past EOF per MS-SMB2 §3.3.5.12 - Enforce OutputBufferLength in handle_query_info: truncate and return STATUS_BUFFER_OVERFLOW per MS-SMB2 §3.3.5.20.1 - Add NTStatus::BufferOverflow (0x80000005) and InfoLengthMismatch (0xC0000004) Encapsulation: - Make SMBFileId fields private with new(), persistent(), volatile() - Make SMBFileMetadata fields private with new() and getters - Make all file_info struct fields private with constructors and getters - FileStandardInformation: accept bool for delete_pending/directory Tests: 95 unit tests pass (15 new: normalize_path, bool getters, FileAllInformation getters). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add missing SMBError import in ipc.rs - Move doc comments inside bitflags! macros - Remove unused SMBByteSize import - Fix unnecessary cast and needless borrow - Allow too_many_arguments on FileAllInformation::new (matches MS-FSCC spec) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… deduplicate close logic Build SMBCreateResponse after add_open so file_id reflects assigned global_id/session_id instead of zeroes. Replace single file.read() with take()+read_to_end() to handle OS short reads correctly. Refactor handle_close to reuse find_open() instead of duplicating lookup logic. Use try_into() for u64→u32 global_id conversion instead of silent truncation. Replace FileNameInformation::new with from_name() that auto-computes UTF-16 byte length. Fix pre-existing _e/e typos in error logging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tmthecoder
force-pushed
the
feat/file-write-handler
branch
from
9555cb6 to
f6a846e
Compare
…nings SMBByteSize is only used inside trace!() which is a no-op without the logging feature. Similarly, error variables in match arms are only consumed by tracing macros. Gate the import with #[cfg(feature = "logging")] and add #[allow(unused_variables)] to affected match arms. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add accessor methods to SMBWriteRequest (file_id, write_offset, etc.) - Add SMBWriteResponse::new() constructor - Add write_data() to ResourceHandle trait, Open trait, and all impls - Implement handle_write in tree_connect handler following read pattern - Cap write size to 8MB (MAX_WRITE_SIZE) matching read behavior - Add tests for write request/response serialization Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- file_write_uploads_file: verify smbclient put writes a file to share - file_write_then_read_round_trip: verify put+get round-trips data correctly - Fix useless format! lint on existing tests - Allow zombie_processes lint at module level (tests kill server explicitly) - Fix useless vec! in write request unit test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
All 12 smbclient E2E tests pass with the Write handler implementation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace file.write() with write_all() so all bytes are guaranteed to be written instead of silently accepting partial writes. Add unit tests for write_data covering basic writes, offset writes, size capping, directory error handling, and write-then-read round trips. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tmthecoder
force-pushed
the
feat/file-write-handler
branch
from
f6a846e to
8e7cbea
Compare
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.
Summary
SMBWriteRequest(file_id, write_offset, write_length, data_to_write)SMBWriteResponse::new()constructorwrite_data()method toResourceHandletrait,Opentrait, and all implementationshandle_writein tree_connect handler following the existinghandle_readpatternMAX_WRITE_SIZE) matching read behaviorThis is the first feature from Phase 1 of the SMB feature roadmap — enabling a functional read-write file server.
Test plan
cargo fmt --checkpassescargo clippy --workspace --features server -- -D warningspassescargo test --workspace --features serverpasses🤖 Generated with Claude Code