Devolutions / Devolutions/devolutions-gateway

Simplify VMConnect RDCleanPath handling

Open
#1,930 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
159
Forks
28
Avg merge
2d 23h
Merged PRs (30d)
21

Description

Follow-up to [Benoit's review][review] on #1372.

The VMConnect path works, but `rd_clean_path.rs` carries more compatibility code and branching than it needs.
Simplify it without changing the protocol behavior:

- Match once on `(x224_connection_pdu, preconnection_blob)` and classify the request beside its handling.
- Remove the redundant X.224 check from the credential-injection path.
- Keep the 10-second timeout only around the VMConnect PCB write.
- Remove the two front-sequence `flush()` calls and the buffering comment.
- Build one response by mapping the optional X.224 response field.
- Merge the request-shape cases into a table-driven test.
- Keep the non-BMP UTF-16 wire-format regression test.

Once Gateway can consume a compatible IronRDP release exposing `RDCleanPathMessage` and the fixed `PreconnectionBlob` encoder, replace the local classifier, response builder, and PCB encoder with those APIs.

The ordinary path must stay X.224-first.
The VMConnect path must stay PCB -> TLS on Gateway, followed by CredSSP -> X.224 on the client.

[review]: https://github.com/Devolutions/devolutions-gateway/pull/1372#pullrequestreview-4918631862

> [!NOTE]
> Human-tuned, LLM-assisted content.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in rd_clean_path.rs and read the existing request-shape tests plus the non-BMP UTF-16 wire-format regression test. Run the relevant Rust tests before and after the refactor; done means the cases are table-driven, the timeout and flush behavior are simplified as specified, and ordinary X.224-first and VMConnect protocol behavior remain unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
networking
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.