Skip to content

Commit 92ebf1e

Browse files
committed
feat(egfx): cascade Arbitrary derives across ironrdp-egfx public PDU 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.
1 parent 059ca90 commit 92ebf1e

6 files changed

Lines changed: 98 additions & 0 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/ironrdp-egfx/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ doctest = false
1717
# test = false # FIXME: turn off and keep tests in testsuite crates
1818

1919
[dependencies]
20+
arbitrary = { version = "1", features = ["derive"], optional = true }
2021
bit_field = "0.10"
2122
bitflags = "2.11"
2223
ironrdp-core = { path = "../ironrdp-core", version = "0.1" } # public
@@ -27,6 +28,7 @@ openh264 = { version = "0.9", optional = true, default-features = false }
2728
tracing = { version = "0.1", features = ["log"] }
2829

2930
[features]
31+
arbitrary = ["dep:arbitrary", "bitflags/arbitrary", "ironrdp-pdu/arbitrary"]
3032
openh264 = ["dep:openh264"]
3133
openh264-bundled = ["openh264", "openh264/source"]
3234
openh264-libloading = ["openh264", "openh264/libloading"]

crates/ironrdp-egfx/src/pdu/avc.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,21 @@ pub struct QuantQuality {
1515
pub quality: u8,
1616
}
1717

18+
// Manual `Arbitrary` impl: the encoder packs `quantization_parameter` into bits 0..6
19+
// via `set_bits`, which panics when the value exceeds 6 bits. Mask the field to its
20+
// wire-allowed range so fuzz inputs always round-trip through `Encode`. The other
21+
// fields use their full type range.
22+
#[cfg(feature = "arbitrary")]
23+
impl<'a> arbitrary::Arbitrary<'a> for QuantQuality {
24+
fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result<Self> {
25+
Ok(Self {
26+
quantization_parameter: u.arbitrary::<u8>()? & 0x3F, // 6 bits
27+
progressive: u.arbitrary()?,
28+
quality: u.arbitrary()?,
29+
})
30+
}
31+
}
32+
1833
impl QuantQuality {
1934
const NAME: &'static str = "GfxQuantQuality";
2035

@@ -58,6 +73,7 @@ impl<'de> Decode<'de> for QuantQuality {
5873
}
5974
}
6075

76+
#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
6177
#[derive(Clone, PartialEq, Eq)]
6278
pub struct Avc420BitmapStream<'a> {
6379
pub rectangles: Vec<InclusiveRectangle>,
@@ -152,6 +168,19 @@ bitflags! {
152168
}
153169
}
154170

171+
// Manual `Arbitrary` impl: the encoder packs `encoding.bits()` into 2 bits via
172+
// `set_bits(30..32, ...)` on the Avc444BitmapStream stream-info field. The bitflag
173+
// otherwise accepts any u8 value (via `const _ = !0`), so the bitflags-crate-provided
174+
// derive would generate values that exceed the 2-bit wire range and panic the encoder.
175+
// Mask to 2 bits.
176+
#[cfg(feature = "arbitrary")]
177+
impl<'a> arbitrary::Arbitrary<'a> for Encoding {
178+
fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result<Self> {
179+
Ok(Self::from_bits_retain(u.arbitrary::<u8>()? & 0x03))
180+
}
181+
}
182+
183+
#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
155184
#[derive(Debug, Clone, PartialEq, Eq)]
156185
pub struct Avc444BitmapStream<'a> {
157186
pub encoding: Encoding,
@@ -263,6 +292,7 @@ impl<'de> Decode<'de> for Avc444BitmapStream<'de> {
263292
/// assert_eq!(region.left, 0);
264293
/// assert_eq!(region.right, 1919);
265294
/// ```
295+
#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
266296
#[derive(Debug, Clone, PartialEq, Eq)]
267297
pub struct Avc420Region {
268298
/// Left edge of the region (inclusive)

0 commit comments

Comments
 (0)