Server resize follow-up

[elmarco]

I forgot to squash BitmapEncoder support in #530.

The main part is handling pending incoming client message during deact-react; queuing and replaying them after acceptation.

[CBenoit]

@elmarco Thank you!
Is it ready for review?

[elmarco]

@elmarco Thank you! Is it ready for review?

yes, thanks

[CBenoit]

There are two commits with the same summary: feat(async): teach single_sequence_step() to keep unmatched PDUs

Did you intend to change the scope?

[CBenoit]

Refactor code into something like this:

diff --git a/crates/ironrdp-async/src/framed.rs b/crates/ironrdp-async/src/framed.rs
index 2597f644..fa4cf600 100644
--- a/crates/ironrdp-async/src/framed.rs
+++ b/crates/ironrdp-async/src/framed.rs
@@ -60,10 +60,6 @@ impl<S> Framed<S> {
     pub fn peek(&self) -> &[u8] {
         &self.buf
     }
-
-    pub fn leftover_mut(&mut self) -> &mut BytesMut {
-        &mut self.buf
-    }
 }

 impl<S> Framed<S>
diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs
index 63888d3d..e15deacb 100644
--- a/crates/ironrdp-server/src/server.rs
+++ b/crates/ironrdp-server/src/server.rs
@@ -17,6 +17,7 @@ use ironrdp_pdu::{self, decode, encode_vec, mcs, nego, rdp, Action, PduResult};
 use ironrdp_svc::{impl_as_any, server_encode_svc_messages, StaticChannelId, StaticChannelSet, SvcProcessor};
 use ironrdp_tokio::{Framed, FramedRead, FramedWrite, TokioFramed};
 use rdpsnd::server::{RdpsndServer, RdpsndServerMessage};
+use tokio::io::{AsyncRead, AsyncWrite};
 use tokio::net::{TcpListener, TcpStream};
 use tokio::sync::{mpsc, Mutex};
 use tokio::task;
@@ -797,25 +798,29 @@ impl RdpServer {
         }
     }

-    async fn accept_finalize<S>(&mut self, mut framed: Framed<S>, mut acceptor: Acceptor) -> Result<()>
+    async fn accept_finalize<S>(&mut self, mut framed: TokioFramed<S>, mut acceptor: Acceptor) -> Result<()>
     where
-        S: FramedRead + FramedWrite,
+        S: AsyncRead + AsyncWrite + Sync + Send + Unpin,
     {
         let mut other_pdus = None;
+
         loop {
             let (new_framed, result) = ironrdp_acceptor::accept_finalize(framed, &mut acceptor, other_pdus.as_mut())
                 .await
                 .context("failed to accept client during finalize")?;

-            framed = new_framed;
+            let (stream, mut leftover) = new_framed.into_inner();

             debug!(other_pdus = other_pdus.as_ref().map_or(0, |o| o.len()));
             if let Some(pdus) = other_pdus.take() {
-                let mut leftover: Vec<_> = pdus.iter().flat_map(|p| p.iter().copied()).collect();
-                leftover.extend_from_slice(&framed.leftover_mut().split()[..]);
-                framed.leftover_mut().extend_from_slice(&leftover);
+                let unmatched_frames = pdus.into_iter().flatten();
+                let previous_leftover = leftover.split();
+                leftover.extend(unmatched_frames);
+                leftover.extend_from_slice(&previous_leftover);
             }

+            framed = TokioFramed::new_with_leftover(stream, leftover);
+
             match self.client_accepted(&mut framed, result).await? {
                 RunState::Continue => {
                     unreachable!();

[CBenoit]

@elmarco By the way, are you getting these client messages from mstsc as well? I’m wondering if the real fix should not be about modifying the IronRDP client to not send messages unrelated to the connection sequence before the connection sequence is over.

[elmarco]

@elmarco By the way, are you getting these client messages from mstsc as well? I’m wondering if the real fix should not be about modifying the IronRDP client to not send messages unrelated to the connection sequence before the connection sequence is over.

Yes. The reactivation sequence is triggered by the server, so there can always be client messages in flight.

[CBenoit]

@elmarco By the way, are you getting these client messages from mstsc as well? I’m wondering if the real fix should not be about modifying the IronRDP client to not send messages unrelated to the connection sequence before the connection sequence is over.

Yes. The reactivation sequence is triggered by the server, so there can always be client messages in flight.

Oh right. It becomes a problem during the reactivation sequence. Sounds good.

[CBenoit]

LGTM!