Skip to content

Improve ClientDriveQueryDirectoryResponse.encode()#12912

Merged
ibeckermayer merged 139 commits intowindows-desktop-directory-sharingfrom
isaiah/fix-query-dir-resp-encode
Jun 29, 2022
Merged

Improve ClientDriveQueryDirectoryResponse.encode()#12912
ibeckermayer merged 139 commits intowindows-desktop-directory-sharingfrom
isaiah/fix-query-dir-resp-encode

Conversation

@ibeckermayer
Copy link
Copy Markdown
Contributor

Takes some of the ad hoc "correctness" checking that was built into ClientDriveQueryDirectoryResponse.encode() and moves it to ClientDriveQueryDirectoryResponse.new(), allowing encode() to be significantly simplified.

Isaiah Becker-Mayer added 30 commits March 31, 2022 15:35
…and cliprdr::Client's have the vchan::Client as a field.
…o trigger it by right-clicking, however it isn't working. One reason is that the vchannel PDU header isn't being added (see rdpdr::encode_message for how that's added to other messages). Noticing that made me notice that there is another cliprdr function for breaking outgoing messages into chunks that should be refactored into vchan to do that work + add the necessary vchan headers. This is a checkpoint commit while I go attend to that.
…lient parses the DeviceCreateRequest that's immediately sent back
…ugh in most cases, no need to neurotically add every bit of the documentation text to the code itself
Comment on lines +2538 to +2539
NTSTATUS::STATUS_SUCCESS => {
if buffer.is_none() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
NTSTATUS::STATUS_SUCCESS => {
if buffer.is_none() {
NTSTATUS::STATUS_SUCCESS if buffer.is_none() => {

Up to you whether using match guards makes this read cleaner.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I just tried that, but it turns out that the issue with using a match guard here is that if the match guard fails, then the control gets bumped down to the wildcard match (which isn't what we want in the case of i.e. NTSTATUS::STATUS_SUCCESS && buffer.is_some()):

            _ => {
                return Err(invalid_data_error(&format!(
                    "received unsupported io_status for ClientDriveQueryDirectoryResponse: {:?}",
                    io_status
                )))
            }

So if I were to use a match guard here, I would need to then do something like another match in that wildcard to confirm a legitimate NTSTATUS was received, or add another match case for NTSTATUS::STATUS_SUCCESS if buffer.is_some() (it would read worse).

In a loose sense, adding the match guard sidesteps typical match semantics of matching on all possible values of NTSTATUS, and instead matches on all NTSTATUS && <match guards>.

@ibeckermayer ibeckermayer changed the base branch from isaiah/FileFullDirectoryInformation to windows-desktop-directory-sharing June 29, 2022 18:27
@ibeckermayer ibeckermayer merged commit d016b4b into windows-desktop-directory-sharing Jun 29, 2022
ibeckermayer pushed a commit that referenced this pull request Aug 23, 2022
* `IRP_MJ_CREATE` (#12665)

* `IRP_MJ_QUERY_INFORMATION` (#12717)

* `IRP_MJ_CLOSE` (#12729)

* Refactor rdpdr client (#12750)

* Adding logic for `FILE_SUPERSEDE` (#12829)

* Improve `process_irp_create` (#12830)

* adds return statements that got lost in a merge

* `IRP_MJ_DIRECTORY_CONTROL` (#12870)

* `FileFullDirectoryInformation` (#12908)

* Improve `ClientDriveQueryDirectoryResponse.encode()` (#12912)

* `IRP_MJ_QUERY_VOLUME_INFORMATION` (#13071)

* Fix Shared Directory Request handling when feature is disabled (#13439)

* IRP_MJ_READ, IRP_MJ_WRITE, and IRP_MJ_SET_INFORMATION (#13995)

* Adds constants for sizing calculations (#14051)

Co-authored-by: Łukasz Kozłowski <lukasz.kozlowski@goteleport.com>
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
ibeckermayer pushed a commit that referenced this pull request Aug 23, 2022
* `IRP_MJ_CREATE` (#12665)

* `IRP_MJ_QUERY_INFORMATION` (#12717)

* `IRP_MJ_CLOSE` (#12729)

* Refactor rdpdr client (#12750)

* Adding logic for `FILE_SUPERSEDE` (#12829)

* Improve `process_irp_create` (#12830)

* adds return statements that got lost in a merge

* `IRP_MJ_DIRECTORY_CONTROL` (#12870)

* `FileFullDirectoryInformation` (#12908)

* Improve `ClientDriveQueryDirectoryResponse.encode()` (#12912)

* `IRP_MJ_QUERY_VOLUME_INFORMATION` (#13071)

* Fix Shared Directory Request handling when feature is disabled (#13439)

* IRP_MJ_READ, IRP_MJ_WRITE, and IRP_MJ_SET_INFORMATION (#13995)

* Adds constants for sizing calculations (#14051)

Co-authored-by: Łukasz Kozłowski <lukasz.kozlowski@goteleport.com>
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
ibeckermayer pushed a commit that referenced this pull request Sep 7, 2022
…ring) (#15770)

* Windows Desktop Directory Sharing (#13630)

* `IRP_MJ_CREATE` (#12665)

* `IRP_MJ_QUERY_INFORMATION` (#12717)

* `IRP_MJ_CLOSE` (#12729)

* Refactor rdpdr client (#12750)

* Adding logic for `FILE_SUPERSEDE` (#12829)

* Improve `process_irp_create` (#12830)

* adds return statements that got lost in a merge

* `IRP_MJ_DIRECTORY_CONTROL` (#12870)

* `FileFullDirectoryInformation` (#12908)

* Improve `ClientDriveQueryDirectoryResponse.encode()` (#12912)

* `IRP_MJ_QUERY_VOLUME_INFORMATION` (#13071)

* Fix Shared Directory Request handling when feature is disabled (#13439)

* IRP_MJ_READ, IRP_MJ_WRITE, and IRP_MJ_SET_INFORMATION (#13995)

* Adds constants for sizing calculations (#14051)

Co-authored-by: Łukasz Kozłowski <lukasz.kozlowski@goteleport.com>
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* `UnixPath` and `WindowsPath` (#14267)

* `SharedDirectoryMoveRequest` and `SharedDirectoryMoveResponse` (#14959)

* `SharedDirectoryCreateResponse` update (#15289)

* Fix `process_irp_set_information` (#15364)

* Sanitize Rust Debug Logs (#15743)

* updates rdp-rs ref to include licensing changes

* Updates rdp-rs ref and fixes Cargo

Co-authored-by: Łukasz Kozłowski <lukasz.kozlowski@goteleport.com>
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
@zmb3 zmb3 deleted the isaiah/fix-query-dir-resp-encode branch September 9, 2022 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants