Skip to content

feat(egfx): cascade Arbitrary derives across ironrdp-egfx public PDU types#1334

Merged
Benoît Cortier (CBenoit) merged 1 commit into
Devolutions:masterfrom
lamco-admin:feat/egfx-arbitrary-cascade
Jun 1, 2026
Merged

feat(egfx): cascade Arbitrary derives across ironrdp-egfx public PDU types#1334
Benoît Cortier (CBenoit) merged 1 commit into
Devolutions:masterfrom
lamco-admin:feat/egfx-arbitrary-cascade

Conversation

@glamberson
Copy link
Copy Markdown
Contributor

Summary

  • Extends the Arbitrary cascade from PR feat(pdu): add arbitrary feature for structure-aware fuzzing #1272 (ironrdp-pdu) and PR refactor(egfx)!: mark consumer-facing types as #[non_exhaustive] for pre-publish API stability #1284 (extension to pdu types in egfx-adjacent modules) to the ironrdp-egfx crate's own public type surface.
  • Adds the arbitrary feature to ironrdp-egfx/Cargo.toml, cascading through ironrdp-pdu/arbitrary and bitflags/arbitrary so variant payload types and bitflag types pick up their Arbitrary impls automatically.
  • Adds #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] to every public PDU type in crates/ironrdp-egfx/src/pdu/:
    • common.rs: Point, Color, PixelFormat
    • avc.rs: QuantQuality, Avc420BitmapStream<'a>, Avc444BitmapStream<'a>, Avc420Region, Encoding (bitflags)
    • cmd.rs: GfxPdu plus all 23 variant types, RawCapabilitySet, CapabilitySet, CapabilityVersion, Codec1Type, Codec2Type, Timestamp, QueueDepth, CacheEntryMetadata, and the six CapabilitiesV*Flags bitflag structs.
  • Adds the ironrdp-egfx/arbitrary case to the xtask feature matrix (xtask/src/features.rs), mirroring the existing ironrdp-pdu/arbitrary entry so CI verifies the feature builds cleanly.

Validation

  • cargo xtask ci clean locally on 1.89.0: fmt, lints, tests, typos, locks all green.
  • cargo xtask check features --case ironrdp-egfx/arbitrary clean.
  • Existing 12 ironrdp-egfx unit tests continue to pass.
  • Default build (no arbitrary feature) unchanged: no new dependencies pulled, no behavior change.

Notes

  • Prerequisite cascade for the egfx_multi_frame fuzz target (target 5 under egfx: extend fuzz coverage to OpenH264-wrapper, ZGFX, multi-frame state, and encoder round-trip #1316), which will use Arbitrary-derived Vec<GfxPdu> to drive a single decoder + surface-cache pair across each fuzz iteration. The target itself follows in a separate PR.
  • Pure additive change at the SemVer surface: every derive is gated behind #[cfg_attr(feature = "arbitrary", ...)], and the feature is optional = true in Cargo.toml. Default consumers see no change.
  • Bitflags structs need bitflags/arbitrary activation in the feature cascade so the bitflags crate's own Arbitrary support reaches the CapabilitiesV*Flags and Encoding types.
  • The 23 GfxPdu variant types all use owned types (Vec<u8> for bitmap payloads, fixed-width integers, owned ExclusiveRectangle from ironrdp-pdu); none requires a manual Arbitrary impl. Lifetime-parameterized types (Avc420BitmapStream<'a>, Avc444BitmapStream<'a>) get Arbitrary<'a> impls automatically per the standard derive behavior.

Refs #1316.

…types

Extends the Arbitrary cascade from PR Devolutions#1272 (ironrdp-pdu) and PR Devolutions#1284
(extension to pdu types in ironrdp-pdu's egfx-related modules) to the
ironrdp-egfx crate's own public type surface.

Adds the arbitrary feature to ironrdp-egfx and adds the standard
#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] gate
to every public PDU type in crates/ironrdp-egfx/src/pdu/:

- common.rs: Point, Color, PixelFormat
- avc.rs: Avc420BitmapStream, Avc444BitmapStream, Avc420Region
- cmd.rs: GfxPdu plus all 23 variant types, RawCapabilitySet,
  CapabilitySet, CapabilityVersion, Codec1Type, Codec2Type, QueueDepth,
  CacheEntryMetadata, and the six CapabilitiesV*Flags bitflags structs.

Three types use a manual Arbitrary impl rather than the derive because
their fields have implicit wire-bit-width constraints the derive cannot
express. derive(Arbitrary) on these types generates values in the full
underlying type range, which the encoder then panics on when packing
into the narrower wire range. Manual impls mask each field to its
spec-defined width:

- Timestamp (cmd.rs): milliseconds and hours into 10 bits each,
  seconds and minutes into 6 bits each. The encoder packs all four
  into a single u32 via set_bits.
- QuantQuality (avc.rs): quantization_parameter into 6 bits. Encoder
  packs into a u8 via set_bits(0..6).
- Encoding bitflag (avc.rs): masked to 2 bits. The bitflag's
  `const _ = !0` clause otherwise accepts any u8 value, but the
  encoder for Avc444BitmapStream packs encoding.bits() into bits
  30..32 of the stream-info field.

The arbitrary feature cascades through ironrdp-pdu/arbitrary so types
in egfx's variant payloads (ExclusiveRectangle, etc.) pick up their
existing Arbitrary impls. bitflags/arbitrary is activated so the
CapabilitiesV*Flags bitflag types get Arbitrary via the bitflags
crate's own derive support.

Adds the ironrdp-egfx/arbitrary case to the xtask feature matrix
(xtask/src/features.rs), mirroring the ironrdp-pdu/arbitrary entry,
so CI verifies the feature builds cleanly.

Default build (no arbitrary feature) is unchanged. Existing tests
pass. cargo xtask check features --case ironrdp-egfx/arbitrary clean.

This is the prerequisite cascade for the egfx_multi_frame fuzz target
(target 5 under Devolutions#1316), which will use Arbitrary-derived Vec<GfxPdu>
to drive a single decoder + surface-cache pair across each fuzz
iteration. The target itself follows in a separate PR. Per the Devolutions#1122
guidance ("either implement Arbitrary manually and reject invalid
values, or generate a raw value and then sanitize it"), the three
manual impls here follow the sanitize-via-mask shape so generated
values always round-trip through Encode.

Refs Devolutions#1316.
Copy link
Copy Markdown
Member

@CBenoit Benoît Cortier (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!

@CBenoit Benoît Cortier (CBenoit) merged commit 479a13a into Devolutions:master Jun 1, 2026
21 checks passed
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.

2 participants