Repository navigation
fix(server): send RDP as the Demand Active source descriptor - #2119
Conversation
GitHub Actions
Inactive
There was a problem hiding this comment.
🟡 Changes recommended
The acceptor output needs a regression test asserting the emitted source descriptor.
1 open finding
What changed in this PR
Updates the acceptor’s Demand Active PDU for Jump Desktop compatibility.
Changes:
- Sends
RDP\0instead of an empty source descriptor. - Aligns acceptor output with existing server fixtures.
| File | Description |
|---|---|
crates/ironrdp-acceptor/src/connection.rs |
Sets the server Demand Active source descriptor to RDP. |
🧠 Review effort: Balanced
github-actions
Bot
added
kind/protocol
GitHub Actions
Inactive
github-actions
Bot
added
the
automation-failed
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟢 One-line server fix: the acceptor's Demand Active PDU now carries source_descriptor "RDP" instead of an empty string, which some strict clients (e.g. Jump Desktop) reject. Encoded as RDP\0, it matches FreeRDP and the existing SERVER_DEMAND_ACTIVE fixture; the length field is derived from the descriptor length so no size regressions arise. A focused regression test drives the acceptor through LicensingExchange and MultitransportBootstrapping to CapabilitiesSendServer and asserts the emitted descriptor via the existing decode_share_control API, reusing file fixtures. Independently verified against pr-head: the change is confined to the sole Demand Active producer, all referenced helpers and imports exist, and no code or fixture depends on the prior empty value. Correct, minimal, and well-scoped.
The server Demand Active PDU used an empty source descriptor, sent as a
single NUL byte. Jump Desktop rejects it ("bad packet - source Descriptor
not set to RDP") and drops the connection right after the TLS upgrade.
RDP encodes as RDP\0, the same four bytes the FreeRDP server sends.
Signed-off-by: Alex P <dtk077@gmail.com>
Signed-off-by: Alex P <dtk077@gmail.com>
4210cb6 to
bca3da4
Compare
GitHub Actions
Inactive
github-actions
Bot
removed
the
automation-failed
There was a problem hiding this comment.
PR 2119 changes the acceptor's server Demand Active source descriptor from an empty string (encoded as a single NUL byte, rejected by Jump Desktop) to "RDP" (encoded as "RDP" plus a NUL, matching FreeRDP and the repo's own SERVER_DEMAND_ACTIVE fixture bytes) and adds a targeted regression test. The one-line production change at connection.rs:1386 was verified, as were the fixture bytes in capsets.rs already encoding "RDP" with a NUL and length 4. The change is minimal, protocol-conformant, and interop-motivated; no correctness or protocol defect was found. The sole publishable finding, from the test reviewer, is that the new test lacks an `// oracle:` marker tying its expected value to MS-RDPBCGR 2.2.1.13.1.1 and regression #2118; the value itself checks out independently, so this is a low-severity reviewability gap.
Push a commit after addressing these findings. If no code change is needed, you may resolve inline threads and comment @github-actions review-ready to request human review.
github-actions
Bot
added
ai-reviewed/2
d921607
into
Devolutions:master
github-actions
Bot
removed
the
needs-author-action
…ktop-scale-factor * upstream/master: (92 commits) fix(server): advertise horizontal mouse wheel input (Devolutions#2236) fix(pdu): reject RSA moduli below 512 bits (Devolutions#2225) ci(ffi): verify generated bindings (Devolutions#2166) build(ffi)!: move to Diplomat 0.16.1-devo.2 (Devolutions#2165) build: make the client's video, camera and decoder crates publishable (Devolutions#2239) ci(release): release the newly published crates (Devolutions#2218) perf(graphics): replace the RLGR decoder's bit reader (Devolutions#2150) chore(graphics): add RemoteFX decode benchmarks (Devolutions#2149) fix(server)!: pass a GfxContext to the GfxServerFactory build methods (Devolutions#2104) fix(pdu): check remaining length when skipping padding and reading client core data (Devolutions#2130) fix(session): avoid hang on empty QOIZ updates (Devolutions#2147) feat(session): expose EGFX compositing (Devolutions#2120) fix(server): send RDP as the Demand Active source descriptor (Devolutions#2119) feat(activex)!: broaden mstscax compatibility (Devolutions#2211) feat(viewer): run RemoteApp programs in native windows (Devolutions#2201) fix(graphics): reject RLGR3 values larger than their remainder (Devolutions#2132) feat(egfx)!: composite surfaces mapped to RemoteApp windows (Devolutions#2197) feat: redirect USB devices over URBDRC with nusb and RemoteFX USB backends (Devolutions#2181) build: mark exposed dependencies as public (Devolutions#2213) chore(release): prepare for publishing (Devolutions#2212) ... # Conflicts: # crates/ironrdp-testsuite-extra/tests/e2e.rs
The server Demand Active PDU used an empty source descriptor, which
DemandActiveencodes as a single NUL byte. Jump Desktop rejects it (bad packet - source Descriptor not set to RDP) and drops the connection right after the TLS upgrade.Send
RDPinstead. It encodes asRDP\0, the same four bytes the FreeRDP server writes, and the value already used by the server Demand Active fixture inironrdp-testsuite-core.Fixes #2118