Skip to content
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

Improve ClientDriveQueryDirectoryResponse.encode() #12912

Merged
Merged
Changes from all commits
Commits
Show all changes
139 commits
Select commit Hold shift + click to select a range
24c1a90
Creates a general vchan Client that can read chunked messages. rdpdr …
Mar 31, 2022
41ac367
Tidying up
Mar 31, 2022
58cac52
Merge branch 'master' into isaiah/reuse-vchan-read
Mar 31, 2022
fa3531f
small fixes
Mar 31, 2022
ab4d66d
Merge branch 'master' into isaiah/reuse-vchan-read
Mar 31, 2022
aaa8369
fixes linting errors
Apr 1, 2022
54b7e3d
Merge branch 'master' into isaiah/reuse-vchan-read
Apr 1, 2022
071620b
Merge branch 'master' into isaiah/reuse-vchan-read
Apr 1, 2022
52a3d5d
Merge branch 'master' into isaiah/reuse-vchan-read
Apr 1, 2022
ceddf84
Adds drive redirection capability set to the Client Core Capability R…
Apr 1, 2022
6baccf6
Adds the ClientDeviceListAnnounce message, plus some prototype code t…
Apr 4, 2022
9036618
Adds to_unicode util
Apr 4, 2022
770f6e7
refactor of vchan and rdpdr to use it
Apr 4, 2022
17a35e8
refactors cliprdr to use generalized vchan header and chunking
Apr 4, 2022
c946b52
nits
Apr 4, 2022
787b67e
Merge branch 'master' into isaiah/reuse-vchan-write
Apr 4, 2022
6cd9ac2
small fixes from CR
Apr 4, 2022
997b7d6
Merge branch 'master' into isaiah/reuse-vchan-write
Apr 4, 2022
eb8ed1a
Merge branch 'isaiah/reuse-vchan-write' into isaiah/mock-drive-redire…
Apr 4, 2022
54b53d0
A right click sends an ClientDeviceListAnnounce message and the rdp c…
Apr 6, 2022
8f1ead3
adds prototype logic for handling IRP_MJ_QUERY_INFORMATION and IRP_MJ…
Apr 8, 2022
9a88738
removes some un needed comments. the link to the documentation is eno…
Apr 11, 2022
fb60ebb
Successfully mocks a drive for redirection
Apr 15, 2022
51f83e5
adding important todo
Apr 19, 2022
b4192a6
removes unneeded pub decl
Apr 25, 2022
6cf694a
refactors rdpdr into its own self-contained folder module
Apr 25, 2022
fa0d555
moving scard into rdpdr
Apr 26, 2022
4b9c330
allowing dead code
Apr 26, 2022
66eb895
stubs out request_file_info
Apr 26, 2022
687b84c
fixes mismatched declaration
Apr 27, 2022
058530b
completes IRP_MJ_DEVICE_CONTROL
Apr 27, 2022
1543585
adds a check for allow_directory_sharing so that we guarantee its fun…
Apr 27, 2022
549eb02
adds SharedDirectoryAnnounce and removes the right click hack from cl…
Apr 27, 2022
8c5e2b1
Adds SharedDirectoryAcknowledge tdp message, refactors rust-go interf…
Apr 28, 2022
de13b64
updates naming convention
Apr 28, 2022
8017ef2
Making sense of handle_client_id_confirm
Apr 28, 2022
1ce587f
finishes out shared directory ack
Apr 28, 2022
66b5b00
updates to new sd_acknowledge
Apr 29, 2022
de8e052
uses a struct for ack
Apr 30, 2022
0feae71
roadblock at 'static lifetime error
May 2, 2022
20d9aeb
refactors codebase to only contain shared directory announce and shar…
May 3, 2022
ec900b8
Merge branch 'master' into isaiah/tdp-sd-announce-ack
May 3, 2022
e43694b
Merge branch 'master' into isaiah/tdp-sd-announce-ack
May 3, 2022
95af26b
fixes function name typo
May 4, 2022
2adf291
checking in to go work on other stuff
May 6, 2022
eec39f5
rather than passing errors via C-strings, we pass them as integers wh…
May 6, 2022
c43d688
Merge branch 'master' into isaiah/cgo-err-code
May 6, 2022
a56236f
giving CGOErrCode explicit values
May 7, 2022
20efe3e
removing useless format
May 7, 2022
dac6c41
adding feature flag
May 9, 2022
6251502
adding #[allow(dead_code)]
May 9, 2022
2cb663d
Merge branch 'master' into isaiah/cgo-err-code
May 9, 2022
3615580
Merge branch 'isaiah/cgo-err-code' into isaiah/tdp-sd-announce-ack
May 9, 2022
7fe5b95
Debugf
May 9, 2022
9274c31
Merge branch 'master' into isaiah/cgo-err-code
May 9, 2022
7975209
Merge branch 'isaiah/cgo-err-code' into isaiah/tdp-sd-announce-ack
May 9, 2022
b0072f9
Merge branch 'isaiah/tdp-sd-announce-ack' into isaiah/stub-irp-mj-create
May 9, 2022
ce1c6c5
removing mentions of code side
May 9, 2022
1732e90
Merge branch 'isaiah/cgo-err-code' into isaiah/tdp-sd-announce-ack
May 9, 2022
6605005
Merge branch 'isaiah/tdp-sd-announce-ack' into isaiah/stub-irp-mj-create
May 9, 2022
224db3a
removing dangling CGO_OK
May 9, 2022
9ec0cba
making a mod.rs
May 9, 2022
ee34ddd
moving scard in rdpdr
May 9, 2022
dbedfc8
moves constants into consts.rs
May 9, 2022
df26b8f
Adds go build flags for directory sharing and some basic scaffolding …
May 9, 2022
e0bbfcf
Merge branch 'isaiah/shared-directory-build-flag' into isaiah/tdp-sd-…
May 9, 2022
eccab5a
Merge branch 'master' into isaiah/tdp-sd-announce-ack
May 9, 2022
51c956c
reverting e
May 9, 2022
8d532c8
Id to ID
May 10, 2022
750fe60
typo
May 10, 2022
f4959ee
proof of concept for calling the closures from golang. Checkpoing com…
May 11, 2022
e5d849e
trims rust back to bare minimum required for poc
May 11, 2022
f146e42
adds TypeSharedDirectoryAcknowledge, TypeSharedDirectoryInfoRequest, …
May 11, 2022
53cc341
Adds sharedDirectoryInfoRequest to go client
May 12, 2022
fe4bd3e
adds SharedDirectoryInfoResponse to go client
May 12, 2022
6bfc7a3
Merge branch 'isaiah/tdp-sd-announce-ack' into isaiah/rdp-tdp-transl-…
May 12, 2022
27d7c53
CR
May 12, 2022
f00eeb6
fixes compiler errors
May 12, 2022
0227339
Adds the straightforward failure modes
May 14, 2022
cd4b5a2
adds file_cache
May 14, 2022
9ebf970
adds Shared Directory Create Request
May 14, 2022
8d97123
adds Shared Directory Delete Request
May 15, 2022
034f396
fills out the rest of IRP_MJ_CREATE
May 15, 2022
b01c4ee
adds plumbing for calling handle_tdp_sd_create_response and handle_td…
May 15, 2022
d7950f6
adds logic for calling handle_tdp_sd_create_response and handle_tdp_s…
May 15, 2022
1db2ff5
reverting mistaken Makefile change
May 15, 2022
e1ce4ce
Merge branch 'isaiah/rdp-tdp-transl-arch' into isaiah/complete-irp-mj…
May 15, 2022
664a8e4
adds missing STATUS_SUCCESS messages
May 17, 2022
6ce109a
adds logic for IRP_MJ_QUERY_INFORMATION
May 17, 2022
3dc58ad
adding dead_code tag
May 17, 2022
90350d1
reverting Makefile
May 17, 2022
a71b390
Adds handling of IRP_MJ_CLOSE
May 17, 2022
c4371da
Merge branch 'master' into isaiah/tdp-sd-announce-ack
May 18, 2022
9e4db09
refactored all available, now realizing that I should've branched off…
May 18, 2022
511b1d2
Merge branch 'isaiah/irp-mj-close' into isaiah/irp-mj-refactor
May 18, 2022
02f3ec9
moves IRP_MJ_CLOSE handling into process_irp_close
May 18, 2022
750e7ff
fixing build bug and go lint errors
May 19, 2022
5bd7968
merging branch isaiah/tdp-sd-announce-ack
May 19, 2022
15fb5c5
Merge branch 'isaiah/rdp-tdp-transl-arch' into isaiah/complete-irp-mj…
May 19, 2022
efa9ac1
reverting Makefile
May 19, 2022
23ddd4d
Merge branch 'isaiah/complete-irp-mj-create' into isaiah/irp-mj-query…
May 19, 2022
67ee1b8
Merge branch 'isaiah/irp-mj-query-information' into isaiah/irp-mj-close
May 19, 2022
8e0a0ed
Merge branch 'isaiah/irp-mj-close' into isaiah/irp-mj-refactor
May 19, 2022
61d2509
Adds CGOTdpErrCode. Go behaves oddly with enums being placed in struc…
May 19, 2022
f6f7c69
I decided there's no need to use errCodeToTdpErrCode at all. We can j…
May 19, 2022
8dc6616
adding CGOFileType
May 19, 2022
57a826c
changed up the naming convention for these enums, since the primary r…
May 19, 2022
b449124
fixing rust linting problems
May 19, 2022
6b304ff
Merge branch 'isaiah/tdp-sd-announce-ack' into isaiah/rdp-tdp-transl-…
May 19, 2022
766c0ca
fixing clippy errors
May 19, 2022
93ba0bf
Merge branch 'isaiah/rdp-tdp-transl-arch' into isaiah/complete-irp-mj…
May 19, 2022
3c3db15
Fixes linting errors
May 19, 2022
570019a
Merge branch 'isaiah/complete-irp-mj-create' into isaiah/irp-mj-query…
May 19, 2022
58cf5e2
fixing lint errors and reverting Makefile
May 19, 2022
7155ef4
Merge branch 'isaiah/irp-mj-query-information' into isaiah/irp-mj-close
May 19, 2022
7e4986f
fixing clippy errors
May 19, 2022
cde94ea
Merge branch 'isaiah/irp-mj-close' into isaiah/irp-mj-refactor
May 19, 2022
bd86280
fixing clippy errors
May 19, 2022
1abfd00
fixing Makefile
May 20, 2022
0732269
adds SharedDirectoryListRequest, support for IRP_MJ_DIRECTORY_CONTROL…
May 23, 2022
ab54ef5
Adding logic for FILE_SUPERSEDE
May 23, 2022
5c4aedb
taking an opportunity to improve code quality of process_irp_create a…
May 23, 2022
6213cb1
adds go code for encoding/decoding SharedDirectoryListResponse
May 23, 2022
91c855e
adds all the glue for getting SharedDirectoryListResponse's from the …
May 23, 2022
cd0a76a
updating a comment
May 24, 2022
25315f0
Merge branch 'isaiah/add-FILE-SUPERSEDE' into isaiah/irp-mj-directory…
May 24, 2022
1f93760
Merge branch 'isaiah/irp-mj-create-refactor' into isaiah/irp-mj-direc…
May 24, 2022
2edf592
Adding more detailed safety blocks for rust functions called from Go
May 24, 2022
5ed8bd0
Adding algorithm explanation to the doc string of process_irp_directo…
May 24, 2022
7427ea9
changing a mistaken cli.prep_file_cache_fail_drive_query_dir_response…
May 24, 2022
5b8a8e6
Adds explanatory comment
May 24, 2022
7b97bcf
fixing fsoList cgo/ffi passing so that empty lists can be properly pa…
May 24, 2022
57f2009
updates FileBothDirectoryInformation to use default values rather tha…
May 24, 2022
b53e375
Adds FileFullDirectoryInformation support
May 24, 2022
a009514
adds match block to ClientDriveQueryDirectoryResponse::new to ensure …
May 25, 2022
aeaa16e
updates encode() given the new constraints of new()
May 25, 2022
7fa4ec5
swapping to use buffer guard
Jun 29, 2022
9a33881
Merge branch 'windows-desktop-directory-sharing' into isaiah/fix-quer…
Jun 29, 2022
09d803f
reverts match guard
Jun 29, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 36 additions & 28 deletions lib/srv/desktop/rdp/rdpclient/src/rdpdr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2572,6 +2572,36 @@ impl ClientDriveQueryDirectoryResponse {
io_status: NTSTATUS,
buffer: Option<FsInformationClass>,
) -> RdpResult<Self> {
// This match block ensures that the passed parameters are in a configuration that's
// explicitly supported by the length calculation (below) and the self.encode() method.
match io_status {
NTSTATUS::STATUS_SUCCESS => {
if buffer.is_none() {
Comment on lines +2578 to +2579
Copy link
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
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>.

return Err(invalid_data_error(
"a ClientDriveQueryDirectoryResponse with NTSTATUS::STATUS_SUCCESS \
should have Some(FsInformationClass) buffer, got None",
));
}
}
NTSTATUS::STATUS_NOT_SUPPORTED
| NTSTATUS::STATUS_NO_MORE_FILES
| NTSTATUS::STATUS_UNSUCCESSFUL => {
if buffer.is_some() {
return Err(invalid_data_error(&format!(
"a ClientDriveQueryDirectoryResponse with NTSTATUS = {:?} \
should have a None buffer, got {:?}",
io_status, buffer,
)));
}
}
_ => {
return Err(invalid_data_error(&format!(
"received unsupported io_status for ClientDriveQueryDirectoryResponse: {:?}",
io_status
)))
}
}

let length = match buffer {
Some(ref fs_information_class) => match fs_information_class {
FsInformationClass::FileBothDirectoryInformation(fs_info_class) => {
Expand All @@ -2597,40 +2627,18 @@ impl ClientDriveQueryDirectoryResponse {
})
}

// TODO(isaiah): make this logic more sane
fn encode(&self) -> RdpResult<Vec<u8>> {
let mut w = vec![];
w.extend_from_slice(&self.device_io_reply.encode()?);

if self.device_io_reply.io_status == NTSTATUS::to_u32(&NTSTATUS::STATUS_SUCCESS).unwrap() {
w.write_u32::<LittleEndian>(self.length)?;
w.extend_from_slice(
&self
.buffer.as_ref()
.ok_or_else(|| invalid_data_error(
"ClientDriveQueryDirectoryResponse with NTSTATUS::STATUS_SUCCESS expects a FsInformationClass"
))?
.encode()?,
);
} else if self.device_io_reply.io_status
w.write_u32::<LittleEndian>(self.length)?;
if let Some(buffer) = &self.buffer {
w.extend_from_slice(&buffer.encode()?);
}
if self.device_io_reply.io_status
== NTSTATUS::to_u32(&NTSTATUS::STATUS_NO_MORE_FILES).unwrap()
{
// https://github.com/FreeRDP/FreeRDP/blob/511444a65e7aa2f537c5e531fa68157a50c1bd4d/channels/drive/client/drive_file.c#L935-L937
w.write_u32::<LittleEndian>(0)?;
// https://github.com/FreeRDP/FreeRDP/blob/511444a65e7aa2f537c5e531fa68157a50c1bd4d/channels/drive/client/drive_file.c#L937
w.write_u8(0)?;
} else if self.device_io_reply.io_status
== NTSTATUS::to_u32(&NTSTATUS::STATUS_NOT_SUPPORTED).unwrap()
|| self.device_io_reply.io_status
== NTSTATUS::to_u32(&NTSTATUS::STATUS_UNSUCCESSFUL).unwrap()
{
// https://github.com/FreeRDP/FreeRDP/blob/511444a65e7aa2f537c5e531fa68157a50c1bd4d/channels/drive/client/drive_main.c#L665
// https://github.com/FreeRDP/FreeRDP/blob/511444a65e7aa2f537c5e531fa68157a50c1bd4d/channels/drive/client/drive_main.c#L634
w.write_u32::<LittleEndian>(self.length)?;
} else {
return Err(invalid_data_error(&format!(
"Found ClientDriveQueryDirectoryResponse with invalid or unhandled NTSTATUS: {:?}",
self.device_io_reply.io_status
)));
}

Ok(w)
Expand Down