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

Clipboard PDU support #170

Merged
merged 9 commits into from
Aug 15, 2023
Merged

Clipboard PDU support #170

merged 9 commits into from
Aug 15, 2023

Conversation

pacmancoder
Copy link
Contributor

@pacmancoder pacmancoder commented Aug 11, 2023

Add support for clipboard virtual channel PDUs encode/decode logic [MS-RDPECLIP].

Although only textual representation is needed in the first iteration(#107), this PR implements all PDUs defined in [MS-RDPECLIP] specification.

The feature was tested against PDU blobs presented in section 4 (examples) of [MS-RDPECLIP]

@pacmancoder pacmancoder marked this pull request as ready for review August 14, 2023 13:55
@pacmancoder
Copy link
Contributor Author

pacmancoder commented Aug 14, 2023

I am currently hunting a few issues found by fuzzing, but other than that PR is ready for review!
UPD - done

@CBenoit CBenoit self-assigned this Aug 14, 2023
Copy link
Member

@CBenoit CBenoit left a comment

Choose a reason for hiding this comment

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

This is looking pretty good.

Do you think you could move the clipboard PDUs into its own crate? I’m thinking about a crate named after the name of the channel: ironrdp-cliprdr.
This crate will also provides the implementation of the static channel itself in the future.

What do you think?

crates/ironrdp-pdu/src/clipboard/capabilities.rs Outdated Show resolved Hide resolved
@pacmancoder
Copy link
Contributor Author

Do you think you could move the clipboard PDUs into its own crate? I’m thinking about a crate named after the name of the channel: ironrdp-cliprdr.
This crate will also provides the implementation of the static channel itself in the future.

Sounds good to me, will do that 👍

@pacmancoder pacmancoder changed the title [WIP] Clipboard PDU support Clipboard PDU support Aug 15, 2023
@pacmancoder
Copy link
Contributor Author

@CBenoit I made the refactoring, PR is ready for review again 😃

Copy link
Member

@CBenoit CBenoit left a comment

Choose a reason for hiding this comment

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

Excellent work, minor comments

crates/ironrdp-cliprdr/src/pdu/capabilities.rs Outdated Show resolved Hide resolved
crates/ironrdp-cliprdr/src/pdu/capabilities.rs Outdated Show resolved Hide resolved
crates/ironrdp-cliprdr/src/pdu/format_data/file_list.rs Outdated Show resolved Hide resolved
crates/ironrdp-cliprdr/src/pdu/capabilities.rs Outdated Show resolved Hide resolved
@pacmancoder
Copy link
Contributor Author

pacmancoder commented Aug 15, 2023

@CBenoit I fixed last batch of comments 😄

@devolutionsbot
Copy link
Contributor

devolutionsbot commented Aug 15, 2023

Coverage Report 🤖 ⚙️

Past:
Total lines: 20090
Covered lines: 14483 (72.09%)

New:
Total lines: 21257
Covered lines: 15149 (71.27%)

Diff: -0.82%

[this comment will be updated automatically]

Copy link
Member

@CBenoit CBenoit left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@CBenoit CBenoit merged commit 4c38be2 into master Aug 15, 2023
6 checks passed
@CBenoit CBenoit deleted the feat/clipboard-pdu branch August 15, 2023 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants