Skip to content

Commit df0cec0

Browse files
lamco-officelamco-office
committed
feat(error): populate decode/encode error offsets from cursor positions
Audit-driven sweep through every call site that passed `0` as the offset to a decode or encode error producer. Foundation in Devolutions#1266 documented `0` as the "offset unknown" sentinel; this commit replaces it with a real cursor position wherever one is in scope. Coverage: 408 call sites total reviewed via brace-depth enclosing-fn analysis. - 333 macro-form sites in `invalid_field_err!`, `not_enough_bytes_err!`, `unexpected_message_type_err!`, `unsupported_version_err!`, `unsupported_value_err!`, `cast_length!`, `cast_int!` upgraded from `at: 0` to `in: <cursor>`. The cursor's name comes from the immediate enclosing function signature (most are `src`, several `dst`, a few `cursor`, and the BER routines use `stream`). - 9 direct-function-call sites (`invalid_field_err(...)`, `unexpected_message_type_err(...)`, `unsupported_value_err(...)`, `invalid_field_err_with_source(...)`) updated to pass `src.pos()` or `dst.pos()` from the enclosing decode/encode method. - The `per_field_err!` helper macro in `ironrdp-pdu/src/mcs.rs` now takes `in: $cursor` and captures the cursor's position into the returned closure, so PER read/write failures carry the byte offset of the field that failed. All 26 callers updated. - The `validate_dimensions!` and `check_masks_alignment!` local macros in displaycontrol/pointer now take an explicit `$offset:expr`. Their call sites pass `0` from constructors and `<cursor>.pos()` from decode/encode paths. - 57 sites remain at `at: 0`. These are in `try_from` impls on enum discriminants, `new`/`with_*` constructors, accessors on already decoded structs, dimension/size validators, and one `find_size` helper that takes `&[u8]` and reports byte-0 errors. None of them have a cursor in scope, so no offset is knowable. Foundation-crate macros were tightened to enforce the discipline at compile time: - `ensure_size!`'s inner offset is now `$buf.pos()` instead of `0`, enabled by refactoring `encode_string` from `dst: &mut [u8]` to `dst: &mut WriteCursor<'_>` so the cursor invariant holds for every caller. The two slice-form callers (`cliprdr/format_data/file_list.rs` and `rdp/capability_sets/input.rs`) were updated to pass the cursor directly and use `write_padding!` for fixed-width fields. - `cast_length!` and `cast_int!` no longer accept a bare form. Callers must pick `in: <cursor>` to extract the position or `at: <offset>` to pass one explicitly. - Doc comments for `cast_length!` and `cast_int!` updated to show the new in:/at: forms. Test snapshots that print full `Error` debug output were regenerated through `UPDATE_EXPECT=1`; the new fixtures now record the real byte offsets where decode failures originate. Examples: pcb null_size at offset 4 (after the u32 cbSize read), pcb_v2_string_too_big at 18, x224 nego_request_unexpected_rdp_msg_type at 35. The `encode_string` API change is the only user-visible API change in this commit beyond Devolutions#1266's new error variant fields. Refs Devolutions#1266, Devolutions#1120.
1 parent 381b3ae commit df0cec0

99 files changed

Lines changed: 765 additions & 789 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎crates/ironrdp-ainput/src/lib.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ impl<'de> Decode<'de> for ServerPdu {
160160
ensure_fixed_part_size!(in: src);
161161

162162
let pdu_type = ServerPduType::from_u16(src.read_u16())
163-
.ok_or_else(|| invalid_field_err!("pduType", "invalid pdu type", at: 0))?;
163+
.ok_or_else(|| invalid_field_err!("pduType", "invalid pdu type", in: src))?;
164164

165165
let server_pdu = match pdu_type {
166166
ServerPduType::Version => ServerPdu::Version(VersionPdu::decode(src)?),
@@ -257,7 +257,7 @@ impl<'de> Decode<'de> for ClientPdu {
257257
ensure_fixed_part_size!(in: src);
258258

259259
let pdu_type = ClientPduType::from_u16(src.read_u16())
260-
.ok_or_else(|| invalid_field_err!("pduType", "invalid pdu type", at: 0))?;
260+
.ok_or_else(|| invalid_field_err!("pduType", "invalid pdu type", in: src))?;
261261

262262
let client_pdu = match pdu_type {
263263
ClientPduType::Mouse => ClientPdu::Mouse(MousePdu::decode(src)?),

‎crates/ironrdp-cliprdr-format/src/bitmap.rs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -243,19 +243,19 @@ impl BitmapInfoHeader {
243243

244244
let width = src.read_i32();
245245
check_invariant(width != i32::MIN && width.abs() <= 10_000)
246-
.ok_or_else(|| invalid_field_err!("biWidth", "width is too big", at: 0))?;
246+
.ok_or_else(|| invalid_field_err!("biWidth", "width is too big", in: src))?;
247247

248248
let height = src.read_i32();
249249
check_invariant(height != i32::MIN && height.abs() <= 10_000)
250-
.ok_or_else(|| invalid_field_err!("biHeight", "height is too big", at: 0))?;
250+
.ok_or_else(|| invalid_field_err!("biHeight", "height is too big", in: src))?;
251251

252252
let planes = src.read_u16();
253253
if planes != 1 {
254-
return Err(invalid_field_err!("biPlanes", "invalid planes count", at: 0));
254+
return Err(invalid_field_err!("biPlanes", "invalid planes count", in: src));
255255
}
256256

257257
let bit_count = src.read_u16();
258-
check_invariant(bit_count <= 32).ok_or_else(|| invalid_field_err!("biBitCount", "invalid bit count", at: 0))?;
258+
check_invariant(bit_count <= 32).ok_or_else(|| invalid_field_err!("biBitCount", "invalid bit count", in: src))?;
259259

260260
let compression = BitmapCompression(src.read_u32());
261261
let size_image = src.read_u32();
@@ -301,7 +301,7 @@ impl BitmapInfoHeader {
301301

302302
impl Encode for BitmapInfoHeader {
303303
fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
304-
let size = cast_int!("biSize", Self::FIXED_PART_SIZE)?;
304+
let size = cast_int!("biSize", Self::FIXED_PART_SIZE, in: dst)?;
305305
self.encode_with_size(dst, size)
306306
}
307307

@@ -317,10 +317,10 @@ impl Encode for BitmapInfoHeader {
317317
impl<'a> Decode<'a> for BitmapInfoHeader {
318318
fn decode(src: &mut ReadCursor<'a>) -> DecodeResult<Self> {
319319
let (header, size) = Self::decode_with_size(src)?;
320-
let size: usize = cast_int!("biSize", size)?;
320+
let size: usize = cast_int!("biSize", size, in: src)?;
321321

322322
if size != Self::FIXED_PART_SIZE {
323-
return Err(invalid_field_err!("biSize", "invalid V1 bitmap info header size", at: 0));
323+
return Err(invalid_field_err!("biSize", "invalid V1 bitmap info header size", in: src));
324324
}
325325

326326
Ok(header)
@@ -369,7 +369,7 @@ impl Encode for BitmapV5Header {
369369
fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
370370
ensure_fixed_part_size!(in: dst);
371371

372-
let size = cast_int!("biSize", Self::FIXED_PART_SIZE)?;
372+
let size = cast_int!("biSize", Self::FIXED_PART_SIZE, in: dst)?;
373373
self.v1.encode_with_size(dst, size)?;
374374

375375
dst.write_u32(self.red_mask);
@@ -403,10 +403,10 @@ impl<'a> Decode<'a> for BitmapV5Header {
403403
ensure_fixed_part_size!(in: src);
404404

405405
let (header_v1, size) = BitmapInfoHeader::decode_with_size(src)?;
406-
let size: usize = cast_int!("biSize", size)?;
406+
let size: usize = cast_int!("biSize", size, in: src)?;
407407

408408
if size != Self::FIXED_PART_SIZE {
409-
return Err(invalid_field_err!("biSize", "invalid V5 bitmap info header size", at: 0));
409+
return Err(invalid_field_err!("biSize", "invalid V5 bitmap info header size", in: src));
410410
}
411411

412412
let red_mask = src.read_u32();

‎crates/ironrdp-cliprdr/src/pdu/capabilities.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,12 @@ impl Capabilities {
6060

6161
impl Encode for Capabilities {
6262
fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
63-
let header = PartialHeader::new(cast_int!("dataLen", self.inner_size())?);
63+
let header = PartialHeader::new(cast_int!("dataLen", self.inner_size(), in: dst)?);
6464
header.encode(dst)?;
6565

6666
ensure_size!(in: dst, size: self.inner_size());
6767

68-
dst.write_u16(cast_length!(Self::NAME, "cCapabilitiesSets", self.capabilities.len())?);
68+
dst.write_u16(cast_length!(Self::NAME, "cCapabilitiesSets", self.capabilities.len(), in: dst)?);
6969
write_padding!(dst, 2);
7070

7171
for capability in &self.capabilities {
@@ -141,7 +141,7 @@ impl Encode for CapabilitySet {
141141

142142
ensure_size!(in: dst, size: length);
143143
dst.write_u16(Self::CAPSTYPE_GENERAL);
144-
dst.write_u16(cast_int!("lengthCapability", length)?);
144+
dst.write_u16(cast_int!("lengthCapability", length, in: dst)?);
145145
caps.encode(dst)
146146
}
147147

@@ -171,7 +171,7 @@ impl<'de> Decode<'de> for CapabilitySet {
171171
Ok(Self::General(general))
172172
}
173173
_ => Err(invalid_field_err!( "capabilitySetType",
174-
"invalid clipboard capability set type", at: 0)),
174+
"invalid clipboard capability set type", in: src)),
175175
}
176176
}
177177
}

‎crates/ironrdp-cliprdr/src/pdu/client_temporary_directory.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ impl ClientTemporaryDirectory<'_> {
5858

5959
impl Encode for ClientTemporaryDirectory<'_> {
6060
fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
61-
let header = PartialHeader::new(cast_int!("dataLen", Self::INNER_SIZE)?);
61+
let header = PartialHeader::new(cast_int!("dataLen", Self::INNER_SIZE, in: dst)?);
6262
header.encode(dst)?;
6363

6464
ensure_size!(in: dst, size: Self::INNER_SIZE);

‎crates/ironrdp-cliprdr/src/pdu/file_contents.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ impl Encode for FileContentsResponse<'_> {
150150
ClipboardPduFlags::RESPONSE_OK
151151
};
152152

153-
let header = PartialHeader::new_with_flags(cast_int!("dataLen", self.inner_size())?, flags);
153+
let header = PartialHeader::new_with_flags(cast_int!("dataLen", self.inner_size(), in: dst)?, flags);
154154
header.encode(dst)?;
155155

156156
ensure_size!(in: dst, size: self.inner_size());
@@ -179,7 +179,7 @@ impl<'de> Decode<'de> for FileContentsResponse<'de> {
179179
ensure_size!(in: src, size: header.data_length());
180180

181181
if header.data_length() < Self::FIXED_PART_SIZE {
182-
return Err(invalid_field_err!("requestedFileContentsData", "invalid data size", at: 0));
182+
return Err(invalid_field_err!("requestedFileContentsData", "invalid data size", in: src));
183183
};
184184

185185
let data_size = header.data_length() - Self::FIXED_PART_SIZE;
@@ -230,7 +230,7 @@ impl Encode for FileContentsRequest {
230230
/// Callers that build these PDUs are responsible for setting fields correctly;
231231
/// use [`FileContentsFlags::validate`] to check flag consistency.
232232
fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
233-
let header = PartialHeader::new(cast_int!("dataLen", self.inner_size())?);
233+
let header = PartialHeader::new(cast_int!("dataLen", self.inner_size(), in: dst)?);
234234
header.encode(dst)?;
235235

236236
ensure_size!(in: dst, size: self.inner_size());
@@ -285,21 +285,21 @@ impl<'de> Decode<'de> for FileContentsRequest {
285285
// [MS-RDPECLIP] 2.2.5.3 - Validate lindex is non-negative
286286
if index < 0 {
287287
return Err(invalid_field_err!( "lindex",
288-
"file index must be non-negative per MS-RDPECLIP 2.2.5.3", at: 0));
288+
"file index must be non-negative per MS-RDPECLIP 2.2.5.3", in: src));
289289
}
290290

291291
// [MS-RDPECLIP] 2.2.5.3 - Validate flags are spec-compliant
292-
flags.validate().map_err(|e| invalid_field_err!("dwFlags", e, at: 0))?;
292+
flags.validate().map_err(|e| invalid_field_err!("dwFlags", e, in: src))?;
293293

294294
// [MS-RDPECLIP] 2.2.5.3 - Validate SIZE request constraints
295295
if flags.contains(FileContentsFlags::SIZE) {
296296
if requested_size != 8 {
297297
return Err(invalid_field_err!( "cbRequested",
298-
"SIZE request must have cbRequested=8 per MS-RDPECLIP 2.2.5.3", at: 0));
298+
"SIZE request must have cbRequested=8 per MS-RDPECLIP 2.2.5.3", in: src));
299299
}
300300
if position != 0 {
301301
return Err(invalid_field_err!( "position",
302-
"SIZE request must have position=0 per MS-RDPECLIP 2.2.5.3", at: 0));
302+
"SIZE request must have position=0 per MS-RDPECLIP 2.2.5.3", in: src));
303303
}
304304
}
305305

‎crates/ironrdp-cliprdr/src/pdu/format_data/file_list.rs‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,10 @@ impl Encode for FileDescriptor {
185185
let encoded_len = wire_name.encode_utf16().count() * 2 + 2;
186186
if NAME_LENGTH < encoded_len {
187187
return Err(ironrdp_core::invalid_field_err!( "cFileName",
188-
"encoded wire name exceeds NAME_LENGTH (520 bytes)", at: 0));
188+
"encoded wire name exceeds NAME_LENGTH (520 bytes)", in: dst));
189189
}
190190

191-
let written = encode_string(dst.remaining_mut(), &wire_name, CharacterSet::Unicode, true)?;
192-
dst.advance(written);
191+
let written = encode_string(dst, &wire_name, CharacterSet::Unicode, true)?;
193192

194193
// Pad with zeroes, overriding any previously written data
195194
write_padding!(dst, NAME_LENGTH - written);
@@ -271,7 +270,7 @@ impl Encode for PackedFileList {
271270
fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
272271
ensure_fixed_part_size!(in: dst);
273272

274-
dst.write_u32(cast_length!(Self::NAME, "cItems", self.files.len())?);
273+
dst.write_u32(cast_length!(Self::NAME, "cItems", self.files.len(), in: dst)?);
275274

276275
for file in &self.files {
277276
file.encode(dst)?;
@@ -292,11 +291,11 @@ impl Encode for PackedFileList {
292291
impl<'de> Decode<'de> for PackedFileList {
293292
fn decode(src: &mut ReadCursor<'de>) -> DecodeResult<Self> {
294293
ensure_fixed_part_size!(in: src);
295-
let file_count: usize = cast_length!(Self::NAME, "cItems", src.read_u32())?;
294+
let file_count: usize = cast_length!(Self::NAME, "cItems", src.read_u32(), in: src)?;
296295

297296
if MAX_FILE_COUNT < file_count {
298297
return Err(ironrdp_core::invalid_field_err!( "cItems",
299-
"file count exceeds maximum of 100000", at: 0));
298+
"file count exceeds maximum of 100000", in: src));
300299
}
301300

302301
// Cap pre-allocation against remaining bytes to prevent OOM from

‎crates/ironrdp-cliprdr/src/pdu/format_data/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ impl Encode for FormatDataResponse<'_> {
177177
ClipboardPduFlags::RESPONSE_OK
178178
};
179179

180-
let header = PartialHeader::new_with_flags(cast_int!("dataLen", self.data.len())?, flags);
180+
let header = PartialHeader::new_with_flags(cast_int!("dataLen", self.data.len(), in: dst)?, flags);
181181
header.encode(dst)?;
182182

183183
ensure_size!(in: dst, size: self.data.len());
@@ -228,7 +228,7 @@ impl FormatDataRequest {
228228

229229
impl Encode for FormatDataRequest {
230230
fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
231-
let header = PartialHeader::new(cast_int!("dataLen", Self::FIXED_PART_SIZE)?);
231+
let header = PartialHeader::new(cast_int!("dataLen", Self::FIXED_PART_SIZE, in: dst)?);
232232
header.encode(dst)?;
233233

234234
ensure_fixed_part_size!(in: dst);

‎crates/ironrdp-cliprdr/src/pdu/format_list.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ impl Encode for FormatList<'_> {
391391
ClipboardPduFlags::empty()
392392
};
393393

394-
let header = PartialHeader::new_with_flags(cast_int!("dataLen", self.encoded_formats.len())?, header_flags);
394+
let header = PartialHeader::new_with_flags(cast_int!("dataLen", self.encoded_formats.len(), in: dst)?, header_flags);
395395
header.encode(dst)?;
396396

397397
ensure_size!(in: dst, size: self.encoded_formats.len());
@@ -449,7 +449,7 @@ impl<'de> Decode<'de> for FormatListResponse {
449449
match header.message_flags {
450450
ClipboardPduFlags::RESPONSE_OK => Ok(FormatListResponse::Ok),
451451
ClipboardPduFlags::RESPONSE_FAIL => Ok(FormatListResponse::Fail),
452-
_ => Err(invalid_field_err!("msgFlags", "invalid format list message flags", at: 0)),
452+
_ => Err(invalid_field_err!("msgFlags", "invalid format list message flags", in: src)),
453453
}
454454
}
455455
}

‎crates/ironrdp-cliprdr/src/pdu/lock.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ impl LockDataId {
1818

1919
impl Encode for LockDataId {
2020
fn encode(&self, dst: &mut WriteCursor<'_>) -> EncodeResult<()> {
21-
let header = PartialHeader::new(cast_int!("dataLen", Self::FIXED_PART_SIZE)?);
21+
let header = PartialHeader::new(cast_int!("dataLen", Self::FIXED_PART_SIZE, in: dst)?);
2222
header.encode(dst)?;
2323

2424
ensure_fixed_part_size!(in: dst);

‎crates/ironrdp-cliprdr/src/pdu/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ impl<'de> Decode<'de> for ClipboardPdu<'de> {
243243
MSG_TYPE_FILE_CONTENTS_RESPONSE => ClipboardPdu::FileContentsResponse(FileContentsResponse::decode(src)?),
244244
MSG_TYPE_LOCK_CLIPDATA => ClipboardPdu::LockData(LockDataId::decode(src)?),
245245
MSG_TYPE_UNLOCK_CLIPDATA => ClipboardPdu::UnlockData(LockDataId::decode(src)?),
246-
_ => return Err(invalid_field_err!("msgType", "unknown clipboard PDU type", at: 0)),
246+
_ => return Err(invalid_field_err!("msgType", "unknown clipboard PDU type", in: src)),
247247
};
248248

249249
Ok(pdu)

0 commit comments

Comments
 (0)