-
Notifications
You must be signed in to change notification settings - Fork 13
Proper sharing violation, remove allow_async #111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
956f9fe to
3d2c4d7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @morbridge! Thanks for you contribution.
The "allow async" flag should still be implemented some how, but there is definitely an issue regarding where it is on - I missed some spots where it should be set. Taking a look at MS-SMB2 (SMB2 protocol spec) part 3.3.4.2 There's a reference to the side comment here that indicates what requests may have an async response:
<239> Section 3.3.4.2: Windows-based servers send interim responses for the following operations if they cannot be completed immediately:
- SMB2_CREATE, if the underlying object store indicates an Oplock/Lease Break Notification or if access/sharing modes are incompatible with another existing open
- SMB2_CHANGE_NOTIFY
- Byte Range Lock
- Named Pipe Read on a blocking named pipe
- Named Pipe Write on a blocking named pipe
- Large file write
- FSCTL_PIPE_TRANSCEIVE
- FSCTL_SRV_COPYCHUNK or FSCTL_SRV_COPYCHUNK_WRITE, when oplock break happens
- SMB2 FLUSH on a named pipe
- FSCTL_GET_DFS_REFERRALS
Let's implement the logic of adding allow_async in all those places (if it is not there, yet). LMK if you have any more questions.
Thanks!
ad73edb to
7781e02
Compare
|
Okay. I've added in file create and flush. Change notify is okay. Pipes and fsctls looks like already are. But I'm not entirely sure. With respect to large file writes. I'm not sure, is it the special operation, or should we just allow that in all writes, just in case? Do we have Byte Range Lock operation in smb-rs? |
7781e02 to
576e0f4
Compare
|
@morbridge we don't actually have any oplocks implemented right now, including byte range locks, so that's cool. It's also good for any write, since we never know what micorosft consider a large write (and we should not assume a certain behavior according to any binary, especially if not documented) You may see some linter issues, make sure they're fixed - and we're good to go. Your code's good. Thanks! |
There were a couple of places where allow_async was missed. So I've
added ones, that allows proper error propagation.
Previously smb-rs if remote file is open for write by another process,
rust client would fail with:
```
[2025-08-08T07:14:44Z ERROR smb_cli] Error: Invalid argument: Async command is not allowed in this context.
Error: InvalidArgument("Async command is not allowed in this context.")
```
And now it fails with proper error:
```
[2025-08-08T07:21:07Z ERROR smb_cli] Error: Server returned an error message with status: Sharing Violation (0xc0000043).
Error: ReceivedErrorMessage(3221225539, ErrorResponse { error_data: [] })
```
576e0f4 to
f9acc11
Compare
Previously smb-rs if remote file is open for write by another process, rust client would fail with:
And now it fails with proper error:
Also for
allow_async. It looks like something that was mistakenly added. As windows returns async-flagged packets (including STATUS_PENDING) even when original create request does not have this flag.