feat(egfx): add ClearCodec client-side decode dispatch#1175
Open
Greg Lamberson (glamberson) wants to merge 7 commits intoDevolutions:masterfrom
Open
feat(egfx): add ClearCodec client-side decode dispatch#1175Greg Lamberson (glamberson) wants to merge 7 commits intoDevolutions:masterfrom
Greg Lamberson (glamberson) wants to merge 7 commits intoDevolutions:masterfrom
Conversation
Implement MS-RDPEGFX 2.2.4.1 ClearCodec (codecId=0x0008), a mandatory lossless codec for all EGFX versions. PDU layer (ironrdp-pdu): - Top-level bitmap stream with glyph index/hit/cache reset flags - Three-layer composite payload (residual, bands, subcodec) - Residual: BGR run-length encoding with variable-length factors - Bands: V-bar dispatch (full cache hit, short cache hit, cache miss) - RLEX: palette-indexed RLE with gradient suite encoding - Subcodec: container for Raw, NSCodec (stub), and RLEX regions Graphics layer (ironrdp-graphics): - ClearCodecDecoder with persistent V-bar and glyph cache state - V-bar cache: 32K full + 16K short entries in ring buffers - Glyph cache: 4,000 entries for small bitmap deduplication - Full three-layer compositing (residual -> bands -> subcodec) 35 tests covering all codec layers and round-trip encoding. NSCodec subcodec deferred (servers can use Raw or RLEX instead).
Add ClearCodecEncoder for server-side ClearCodec bitmap compression: - Residual-only encoding strategy (BGR RLE) for reliable compression - Glyph cache with automatic hit detection for small repeated bitmaps - Sequence number tracking matching the decoder protocol - Cache reset message encoding - BGRA-to-BGR run segment conversion with run-length coalescing 7 new encoder tests including encode-decode round trips that verify interoperability between the encoder and decoder.
Add send_clearcodec_frame() to GraphicsPipelineServer, following the same pattern as send_avc420_frame(). Takes pre-encoded ClearCodec bitmap data and queues it as a WireToSurface1 PDU with the standard three-PDU frame sequence (StartFrame, WireToSurface1, EndFrame). ClearCodec is mandatory for all EGFX versions and provides lossless compression optimized for text and UI elements.
…tion Security hardening informed by FreeRDP ClearCodec CVEs (GHSA-3frr, GHSA-32q9, CVE-2026-26955, CVE-2026-23531) and three rounds of targeted code review. Fixes: - Correct shortVBarYOff interpretation: 6-bit field is an absolute end-row offset, not a pixel count. Pixel count = yOff - yOn per MS-RDPEGFX 2.2.4.1.1.2.1.1.3. (Round 2) - Validate shortVBarYOff >= shortVBarYOn and <= band_height (Round 2) - Cap residual run_length iterations to output buffer size (Round 1) - Cap decoder allocation to 8192x8192 pixels max (Round 3) - Validate glyph index range 0-3999 per spec 2.2.4.1 (Round 2) - Cap encoder input to available pixel data (Round 1) Add integration tests in ironrdp-testsuite-core (35 tests) covering codec round-trip, adversarial input (DoS, OOM, malformed streams), bands layer compositing, cache state, compression quality, and multi-frame session simulation. Inline unit tests (42 tests) in the PDU and graphics crates cover wire format decode/encode.
Wire ClearCodec into the EGFX client's WireToSurface1 codec dispatch, following the same pattern as AVC420 and Uncompressed decode. - Add ClearCodecDecoder field (always enabled, no external codec needed) - Decode ClearCodec bitmap data and convert BGRA output to RGBA - Reset decoder caches on ResetGraphics (V-bar + glyph state) - Add 3 integration tests: basic decode, RGBA output, reset survival - Add unit test for BGRA-to-RGBA channel reordering Depends on the ClearCodec codec PR (Devolutions#1174).
There was a problem hiding this comment.
Pull request overview
This PR introduces ClearCodec support across the stack (PDU parsing, graphics encode/decode, and EGFX client/server integration) and adds a comprehensive test suite to validate correctness and resilience.
Changes:
- Added ClearCodec PDU parsing modules (residual, bands, subcodec, RLEX) under
ironrdp-pdu. - Implemented
ClearCodecDecoder/ClearCodecEncoder(with glyph/V-bar caching) underironrdp-graphics. - Integrated ClearCodec handling into EGFX client decode path and EGFX server frame queuing; added new tests in testsuite-core.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/ironrdp-testsuite-core/tests/graphics/mod.rs | Registers the new ClearCodec graphics test module. |
| crates/ironrdp-testsuite-core/tests/graphics/clearcodec.rs | Adds extensive round-trip and adversarial ClearCodec tests. |
| crates/ironrdp-testsuite-core/tests/egfx/client.rs | Adds EGFX pipeline tests for ClearCodec processing/reset behavior. |
| crates/ironrdp-pdu/src/codecs/mod.rs | Exposes the new ClearCodec codec module in the PDU crate. |
| crates/ironrdp-pdu/src/codecs/clearcodec/mod.rs | Implements ClearCodec bitmap stream + composite payload decoding. |
| crates/ironrdp-pdu/src/codecs/clearcodec/residual.rs | Implements residual layer (BGR RLE) encode/decode helpers. |
| crates/ironrdp-pdu/src/codecs/clearcodec/bands.rs | Implements bands layer decoding (V-bar structures). |
| crates/ironrdp-pdu/src/codecs/clearcodec/subcodec.rs | Implements subcodec layer decoding for regions. |
| crates/ironrdp-pdu/src/codecs/clearcodec/rlex.rs | Implements RLEX subcodec decoding. |
| crates/ironrdp-graphics/src/lib.rs | Exposes ironrdp_graphics::clearcodec. |
| crates/ironrdp-graphics/src/clearcodec/mod.rs | Adds ClearCodec encoder/decoder and decode compositing pipeline. |
| crates/ironrdp-graphics/src/clearcodec/glyph_cache.rs | Implements glyph cache storage for ClearCodec. |
| crates/ironrdp-graphics/src/clearcodec/vbar_cache.rs | Implements V-bar cache storage and reconstruction for bands. |
| crates/ironrdp-egfx/src/server.rs | Adds a send_clearcodec_frame helper to enqueue ClearCodec frames. |
| crates/ironrdp-egfx/src/client.rs | Adds ClearCodec decode path and BGRA→RGBA conversion for bitmap updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
bdc8772 to
31e29a7
Compare
- Cap RLEX segment loop to pixel budget (prevents CPU-spin DoS) - Validate ShortCacheHit y_on + pixel_count against band_height - Validate glyph hit cached dimensions match requested width/height - Add missing record_backpressure() in ClearCodec send path - Remove dead expected_seq field and comment - Use expect() instead of unwrap_or(u32::MAX) for residual length - Use checked_mul in encoder for consistency with decoder - Add debug_assert for BGRA input alignment in convert_bgra_to_rgba - Improve test to assert on captured RGBA pixel data via Arc<Mutex>
31e29a7 to
b164178
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1174.
Wires ClearCodec into the EGFX client's WireToSurface1 codec dispatch,
matching the existing AVC420 and Uncompressed decode patterns.
Changes:
pure Rust, no external codec dependency)
output to RGBA for the uniform BitmapUpdate pixel format
reset/re-decode cycle)
Unlike H.264 which requires an external decoder via the H264Decoder
trait, ClearCodec is pure Rust (ironrdp-graphics) and initialized
unconditionally.