fix(async): reject a zero-length unmatched PDU in read_by_hint

[clintcan]

Problem

Framed::read_by_hint loops to skip PDUs that don't match the requested hint:

loop {
    match hint.find_size(self.peek()).map_err(io::Error::other)? {
        Some((matched, length)) => {
            let bytes = self.read_exact(length).await?.freeze();
            if matched { return Ok(bytes); } else { debug!("Received and lost an unexpected PDU"); }
        }
        None => { /* read more / handle EOF */ }
    }
}

If a PduHint reports Some((false, 0)) — an unmatched, zero-length PDU — then read_exact(0) consumes nothing and performs no I/O, so the loop spins forever at 100% CPU: it never .awaits a real read, so it never yields to the runtime and never observes EOF. Because it doesn't yield, it also starves whatever task drives it (e.g. an acceptor's connection loop), so a single malformed frame can take a whole server down.

I hit this in practice via the built-in X.224 hint: a 2-byte fast-path-shaped header (first byte with & 0b11 == 0, e.g. 04 00) made find_size return a zero-length fast-path PduInfo, read_by_hint treated it as an unexpected PDU, and spun — unauthenticated, before TLS, from one connection.

#1515 hardened find_size so the built-in X.224 / fast-path hint no longer returns a zero-length PDU, which resolves that concrete trigger. This PR is defense-in-depth for the primitive itself: read_by_hint accepts arbitrary dyn PduHint implementations, so it shouldn't rely on every hint avoiding the degenerate case to avoid an infinite loop.

Fix

Guard the single non-progress case — an unmatched zero-length PDU can't be skipped, so fail instead of looping:

if length == 0 && !matched {
    return Err(io::Error::new(
        io::ErrorKind::InvalidData,
        "PduHint reported a zero-length unmatched PDU; cannot make progress",
    ));
}

The invariant becomes: read_by_hint always makes forward progress or returns an error.

Testing

ironrdp-async and ironrdp-tokio are both test = false, and there are currently no async-framing unit tests in the repo (find_size, where the parsing lives, has the regression coverage added in #1515). Verified manually that 04 00 / 00 00 no longer spin. Happy to add a read_by_hint regression test in ironrdp-testsuite-core (it would need ironrdp-async / ironrdp-tokio dev-deps + a small mock PduHint) if you'd prefer one included.

[github-actions]

Single changed file: a zero-length/unmatched guard in `Framed::read_by_hint` (ironrdp-async). The hazard is real — `read_exact(0)` returns without touching the stream, so the loop re-evaluates the same buffer forever and never reaches the EOF check. Nothing blocking: small, local, wire-neutral, no API break. Non-blocking concerns: (1) the comment claims reachability from a malformed frame, but every in-tree hint either always reports matched or takes a length already floored above zero by `find_size`, and `ErrorKind::InvalidData` inherits that mislabelling; (2) the identical loop in ironrdp-blocking is left unguarded, so the goal holds for one of two mirror impls, and the invariant belongs on `PduHint::find_size` docs; (3) `&& !matched` lets `Some((true, 0))` return an empty frame where plain `length == 0` is simpler and stronger; (4) the guard is untested and in-tree unreachable, though `[lib] test = false` makes a test costly.

Protocol analysis: partially_accepted — Kept mapping 2: `Some((true, 0))` still takes `read_exact(0)` and returns an empty frame; I report that asymmetry. Refined mapping 1: the MS-RDPBCGR 3.2.5.2/3.3.5.2 length-consistency duty is already discharged by `find_size` and `TpktHeader::read`, which floor TPKT at 7 and fast-path at the header size, so calling this guard spec conformance overstates it — it hardens a `PduHint` contract no wire input can violate in-tree. Kept mapping 3 as accurate but immaterial. Rejected both potential discrepancies: absent T123/T125 coverage and the lack of an explicit MUST against zero-length fast-path frames are corpus caveats about bounds this diff never changes. No protocol concern was missed; no wire format or state transition is touched.

1. non_blocking / medium — crates/ironrdp-async/src/framed.rs:173-184
   The comment claims the condition "is reachable from a single malformed frame", which no in-tree evidence supports. `RdpHint`, `X224Hint` and `FastPathHint` all derive their length from `ironrdp_pdu::find_size`, which rejects TPKT `packet_length < 7` (ironrdp-pdu/src/tpkt.rs:67) and fast-path lengths below the header (ironrdp-pdu/src/lib.rs:158); the CredSSP hints and both `RDCleanPathHint` copies always return `matched = true`. So no byte sequence from a peer can produce `Some((false, 0))` — only a faulty out-of-tree `PduHint` can. That mislabelling propagates into the chosen `io::ErrorKind::InvalidData`, which tells callers and telemetry the peer sent malformed data when the actual fault is a local trait-contract violation. Please state the real trigger in the comment and reconsider the error kind, so this reads as hardening of a public trait contract rather than a remotely triggerable DoS fix.
2. non_blocking / medium — crates/ironrdp-async/src/framed.rs:179-184
   The stated motivation is to not rely on every hint behaving, but `crates/ironrdp-blocking/src/framed.rs:91-112` contains the byte-for-byte identical loop with the identical `read_exact(0)` non-progress path and is left unguarded, so a third-party hint returning `Some((false, 0))` still spins there via `ironrdp-blocking/src/connector.rs`. As shipped, the invariant holds for one of two mirror `Framed` implementations, which is the weaker half of a guarantee and duplicates the same responsibility at two call sites. Either mirror the guard in the blocking crate in this PR (two lines), or state the non-zero-length requirement once on the `PduHint::find_size` doc comment (ironrdp-pdu/src/lib.rs:174-178) so implementors learn the contract instead of each consumer defending against it.
3. non_blocking / low — crates/ironrdp-async/src/framed.rs:179-179
   The `&& !matched` qualifier means a hint reporting `Some((true, 0))` still takes `read_exact(0)` and hands the caller an empty `Bytes`, which then fails somewhere downstream with a less specific decode error. A zero-length frame is not meaningful in either case, so the plain `if length == 0` guard is both simpler and strictly stronger, and avoids an undocumented asymmetry a reader must reverse-engineer. If the matched case is deliberately tolerated, say why in the comment.
4. non_blocking / low — crates/ironrdp-async/src/framed.rs:179-184
   The guard is unreachable through any in-tree hint, so no existing or future test exercises it and a later refactor can delete it with nothing failing. A regression test is not free here: `ironrdp-async/Cargo.toml` sets `[lib] test = false` and the crate has no dev-dependencies, so covering this needs either flipping that flag or a `tests/` file plus an executor dependency. Given that cost, documenting the invariant on the `PduHint` trait (see the previous finding) is the cheaper way to keep the guard from silently rotting; a stub-hint test asserting `InvalidData` without looping remains the stronger option if the crate's test configuration is revisited.

[github-actions]

non_blocking / medium: The comment claims the condition "is reachable from a single malformed frame", which no in-tree evidence supports. `RdpHint`, `X224Hint` and `FastPathHint` all derive their length from `ironrdp_pdu::find_size`, which rejects TPKT `packet_length < 7` (ironrdp-pdu/src/tpkt.rs:67) and fast-path lengths below the header (ironrdp-pdu/src/lib.rs:158); the CredSSP hints and both `RDCleanPathHint` copies always return `matched = true`. So no byte sequence from a peer can produce `Some((false, 0))` — only a faulty out-of-tree `PduHint` can. That mislabelling propagates into the chosen `io::ErrorKind::InvalidData`, which tells callers and telemetry the peer sent malformed data when the actual fault is a local trait-contract violation. Please state the real trigger in the comment and reconsider the error kind, so this reads as hardening of a public trait contract rather than a remotely triggerable DoS fix.

[github-actions]

non_blocking / medium: The stated motivation is to not rely on every hint behaving, but `crates/ironrdp-blocking/src/framed.rs:91-112` contains the byte-for-byte identical loop with the identical `read_exact(0)` non-progress path and is left unguarded, so a third-party hint returning `Some((false, 0))` still spins there via `ironrdp-blocking/src/connector.rs`. As shipped, the invariant holds for one of two mirror `Framed` implementations, which is the weaker half of a guarantee and duplicates the same responsibility at two call sites. Either mirror the guard in the blocking crate in this PR (two lines), or state the non-zero-length requirement once on the `PduHint::find_size` doc comment (ironrdp-pdu/src/lib.rs:174-178) so implementors learn the contract instead of each consumer defending against it.

[github-actions]

non_blocking / low: The `&& !matched` qualifier means a hint reporting `Some((true, 0))` still takes `read_exact(0)` and hands the caller an empty `Bytes`, which then fails somewhere downstream with a less specific decode error. A zero-length frame is not meaningful in either case, so the plain `if length == 0` guard is both simpler and strictly stronger, and avoids an undocumented asymmetry a reader must reverse-engineer. If the matched case is deliberately tolerated, say why in the comment.

[github-actions]

non_blocking / low: The guard is unreachable through any in-tree hint, so no existing or future test exercises it and a later refactor can delete it with nothing failing. A regression test is not free here: `ironrdp-async/Cargo.toml` sets `[lib] test = false` and the crate has no dev-dependencies, so covering this needs either flipping that flag or a `tests/` file plus an executor dependency. Given that cost, documenting the invariant on the `PduHint` trait (see the previous finding) is the cheaper way to keep the guard from silently rotting; a stub-hint test asserting `InvalidData` without looping remains the stronger option if the crate's test configuration is revisited.

[clintcan]

Real-world impact + reproduction (from a downstream server)

Adding context that this is a live, exploitable issue rather than just a defensive nicety — I hit it in macrdp, a downstream IronRDP-based RDP server.

Impact: unauthenticated, pre-TLS, remote. A single 2-byte fast-path frame (04 00 / 00 00 — first byte & 0b11 == 0, length 0) makes Framed::read_by_hint spin at 100% CPU: find_size returns Some((matched: false, length: 0)), the "skip the unmatched PDU" branch does read_exact(0) (no I/O, no forward progress, no .await yield), and the loop never terminates. Because it never yields, it also starves the executor driving it — for a server that's the acceptor's connection loop, so the whole process stops accepting new connections = full outage. It's reachable before TLS/CredSSP, so connection-level auth / rate-limiting never sees it, and a liveness watchdog that probes the runtime can miss it (the spin pegs one worker while the probe still runs on the others).

Verified on unfixed vs fixed builds: firing that exact 2-byte trigger at the unpatched server pegs a worker at ~100% CPU and wedges the accept loop (needs a restart to recover); with this guard, CPU stays at 0%, the accept loop stays live, and each malformed connection is cleanly rejected.

Re #1515: that hardened find_size so the built-in X.224 / fast-path hint no longer returns a zero-length PDU — which covers the built-in path. This guards the framing primitive itself, so it also covers arbitrary dyn PduHint impls (which downstreams provide) rather than relying on every hint to avoid the degenerate case. Either fix alone breaks the loop; together they're belt-and-suspenders.

macrdp is shipping this as a vendored patch in the meantime. Happy to add a unit test here — a stub PduHint returning (false, 0), asserting read_by_hint errors instead of hanging — or adjust anything if that helps it land. Thanks!