Skip to content

feat: implement Close, Read, and QueryInfo handlers for E2E file read#14

Open
tejas-claude-bot[bot] wants to merge 11 commits intomainfrom
feat/file-read-handlers
Open

feat: implement Close, Read, and QueryInfo handlers for E2E file read#14
tejas-claude-bot[bot] wants to merge 11 commits intomainfrom
feat/file-read-handlers

Conversation

@tejas-claude-bot
Copy link
Copy Markdown
Contributor

Summary

Implements the server-side handlers needed for a complete file read flow: Create → QueryInfo → Read → Close.

Changes

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 returning InvalidDeviceRequest)
  • 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!() → returns None (terminal handler)

Handlers (tree_connect.rs)

Handler Behavior
handle_close Returns file metadata if POSTQUERY_ATTRIB flag set; removes open from server then session 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

Lock Ordering

All handlers acquire locks outer → inner (server → connection → session → open), readers before writers. Also fixes lock ordering in the existing handle_create.

NTStatus

Added: FileClosed, EndOfFile, InvalidInfoClass, InvalidDeviceRequest

Tests

  • 13 unit tests — request accessor validation + response constructor serialization round-trips for Close, Read, and QueryInfo types
  • 3 integration tests (#[ignore]d) — smbclient E2E: file read, directory listing, nonexistent file error handling

How to Test

# Unit tests
cargo test --lib --features "server,anyhow" -- close::tests read::tests query_info::tests

# Integration tests (requires smbclient)
cargo test --test smbclient --features "server,anyhow" -- --ignored

tejas-claude-bot Bot and others added 9 commits April 4, 2026 18:39
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>
@tmthecoder tmthecoder force-pushed the feat/file-read-handlers branch from 75cdd4e to 2fa3140 Compare April 5, 2026 00:38
tejas-claude-bot Bot and others added 2 commits April 6, 2026 21:18
… 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>
…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>
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.

0 participants