clipboard: fail a size request for an entry that has no size

The descriptor now carries FD_FILESIZE only when the size was actually read,
which leaves both size fields at zero for an entry where it was not. The
non-stream FILECONTENTS_SIZE handler passed those fields straight back, so a
receiver asking about such an entry would be told it is empty and would write a
zero-length file without any error. The previous code returned whatever
GetFileSize() had left behind, which for a failure is INVALID_FILE_SIZE -- also
wrong, but wrong loudly.

Answer only when the descriptor has a size, and fail the request otherwise. The
receiver then fails the paste instead of completing it with an empty file, which
is the same class of outcome the old code produced for that entry.

The IStream_Stat path is unchanged; it already answers from the stream.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EZ49AbZJYfm8NTp5yDPMab

libs/clipboard/src/windows/wf_cliprdr.c

@@ -2443,9 +2443,6 @@ static FILEDESCRIPTORW *wf_cliprdr_get_file_descriptor(WCHAR *file_name, size_t
 		return NULL;
 	}
 
-	// to-do: use `fd->dwFlags = FD_ATTRIBUTES | FD_FILESIZE | FD_WRITESTIME | FD_PROGRESSUI`.
-	// We keep `fd->dwFlags = FD_ATTRIBUTES | FD_WRITESTIME | FD_PROGRESSUI` for compatibility.
-	// fd->dwFlags = FD_ATTRIBUTES | FD_FILESIZE | FD_WRITESTIME | FD_PROGRESSUI;
 	fd->dwFlags = FD_ATTRIBUTES | FD_WRITESTIME | FD_PROGRESSUI;
 	fd->dwFileAttributes = GetFileAttributesW(file_name);
 	if (fd->dwFileAttributes == INVALID_FILE_ATTRIBUTES)
@@ -2458,7 +2455,34 @@ static FILEDESCRIPTORW *wf_cliprdr_get_file_descriptor(WCHAR *file_name, size_t
 		fd->dwFlags &= ~FD_WRITESTIME;
 	}
 
-	fd->nFileSizeLow = GetFileSize(hFile, &fd->nFileSizeHigh);
+	// Announce the size in the file list. Without FD_FILESIZE the receiving side cannot
+	// trust the size fields, so CliprdrStream_New() asks for each file's size with its own
+	// FILECONTENTS_SIZE request and blocks on the reply. Those requests are made for every
+	// entry up front, while the shell is inside IDataObject::GetData(), so the cost grows
+	// with the number of files and not with their size.
+	//
+	// GetFileSize() reports failure as INVALID_FILE_SIZE, which cannot be told apart from a
+	// genuine 4GB-1 file without GetLastError(), and it fails outright on the directory
+	// handles FILE_FLAG_BACKUP_SEMANTICS lets us open above. A directory gets no size. A
+	// file whose size cannot be read is rejected rather than sent with the flag off: the
+	// Unix receiver reads the size fields whether or not the flag is set, for compatibility
+	// with older Windows senders, and would take the zero for an empty file.
+	if ((fd->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0)
+	{
+		LARGE_INTEGER file_size = {0};
+
+		if (!GetFileSizeEx(hFile, &file_size) || file_size.QuadPart < 0)
+		{
+			CloseHandle(hFile);
+			free(fd);
+			return NULL;
+		}
+
+		fd->nFileSizeLow = file_size.LowPart;
+		fd->nFileSizeHigh = (DWORD)file_size.HighPart;
+		fd->dwFlags |= FD_FILESIZE;
+	}
+
 	if ((wcslen(file_name + pathLen) + 1) > sizeof(fd->cFileName) / sizeof(fd->cFileName[0]))
 	{
 		// The file name is too long, which is not a normal case.
@@ -3515,15 +3539,24 @@ wf_cliprdr_server_file_contents_request(CliprdrClientContext *context,
 	{
 		if (fileContentsRequest->dwFlags == FILECONTENTS_SIZE)
 		{
+			FILEDESCRIPTORW *fd;
+
 			if (clipboard->nFiles <= fileContentsRequest->listIndex)
 			{
 				rc = ERROR_INTERNAL_ERROR;
 				goto exit;
 			}
-			*((UINT32 *)&pData[0]) =
-				clipboard->fileDescriptor[fileContentsRequest->listIndex]->nFileSizeLow;
-			*((UINT32 *)&pData[4]) =
-				clipboard->fileDescriptor[fileContentsRequest->listIndex]->nFileSizeHigh;
+
+			fd = clipboard->fileDescriptor[fileContentsRequest->listIndex];
+			// The size fields only mean anything when FD_FILESIZE says so. Answering with
+			// them regardless would describe an entry whose size could not be read as empty.
+			if ((fd->dwFlags & FD_FILESIZE) == 0)
+			{
+				rc = ERROR_INTERNAL_ERROR;
+				goto exit;
+			}
+			*((UINT32 *)&pData[0]) = fd->nFileSizeLow;
+			*((UINT32 *)&pData[4]) = fd->nFileSizeHigh;
 			uSize = cbRequested;
 		}
 		else if (fileContentsRequest->dwFlags == FILECONTENTS_RANGE)

src/client.rs

@@ -32,7 +32,7 @@ use crate::{
     common::input::{MOUSE_BUTTON_LEFT, MOUSE_BUTTON_RIGHT, MOUSE_TYPE_DOWN, MOUSE_TYPE_UP},
     create_symmetric_key_msg, decode_id_pk, decode_id_pk_dtls, get_rs_pk, is_keyboard_mode_supported,
     kcp_stream::KcpStream,
-    secure_tcp, secure_tcp_required,
+    secure_tcp,
     ui_interface::{get_builtin_option, resolve_avatar_url, use_texture_render},
     ui_session_interface::{InvokeUiSession, Session},
 };
@@ -832,7 +832,7 @@ impl Client {
         }
         log::info!("rendezvous server: {}", rendezvous_server);
         let mut socket = socket?;
-        let mut my_addr = socket.local_addr();
+        let my_addr = socket.local_addr();
         let mut signed_id_pk = Vec::new();
         let mut relay_server = "".to_owned();
         let mut peer_addr = Config::get_any_listen_addr(true);
@@ -848,44 +848,12 @@ impl Client {
         };
 
         let switch_code = interface.get_switch_code();
-        let legacy_secure = !key.is_empty() && (!token.is_empty() || !switch_code.is_empty());
-        let carries_offer = webrtc_offerer.as_ref().and_then(|g| g.stream()).is_some();
-        // Counted from before the key exchange, so the exchange spends the UDP NAT test's own
-        // wait rather than replacing it: the test runs beside both.
-        let udp_nat_wait_from = Instant::now();
-        let mut exchanged = false;
-        if carries_offer {
-            // An offer puts both sides' ICE candidates, every interface address of both
-            // machines, on this socket, so it goes out only once the server's key exchange has
-            // encrypted it. When the server does not complete one, an hbbs from before the
-            // exchange, the offer is dropped and this becomes a punch without WebRTC, on a fresh
-            // socket since the failed exchange may have consumed a message on this one. Degrade
-            // to no WebRTC, never to WebRTC signalling in the clear.
-            match secure_tcp_required(&mut socket, &key).await {
-                Ok(()) => exchanged = true,
-                Err(err) => {
-                    log::warn!(
-                        "WebRTC signalling to {} cannot be encrypted, punching without WebRTC: {}",
-                        rendezvous_server,
-                        err
-                    );
-                    webrtc_offerer = None;
-                    socket = connect_tcp(&*rendezvous_server, CONNECT_TIMEOUT).await?;
-                    my_addr = socket.local_addr();
-                }
-            }
-        }
-        if !exchanged && legacy_secure {
+        if !key.is_empty() && (!token.is_empty() || !switch_code.is_empty()) {
             secure_tcp(&mut socket, &key)
                 .await
                 .map_err(|e| anyhow!("Failed to secure tcp: {}", e))?;
-        }
-        // A token or switch code has always taken this socket straight to the punch without
-        // waiting for the UDP NAT test. The WebRTC exchange does not replace that wait, it only
-        // spends part of the same budget, so what is left of it is waited out here and a result
-        // that has already arrived is taken at once.
-        if let Some(udp) = udp.1.as_ref().filter(|_| !legacy_secure) {
-            let tm = udp_nat_wait_from;
+        } else if let Some(udp) = udp.1.as_ref() {
+            let tm = Instant::now();
             // rtt is the TCP connect time. When it is too short to be a real WAN round trip it
             // says nothing about the UDP path (a TUN VPN or the LAN gateway answered the
             // handshake, not the server), so fall back to the flat grace; otherwise trust it.

src/common.rs

@@ -2079,13 +2079,6 @@ async fn secure_tcp_impl(conn: &mut Stream, key: &str, log_on_success: bool) ->
     if use_ws() {
         return Ok(());
     }
-    key_exchange(conn, key, log_on_success).await.map(|_| ())
-}
-
-/// The server's key exchange on `conn`. `Ok(true)` once the stream is encrypted. `Ok(false)`
-/// when the server sent something else first, nothing parseable, or closed: `secure_tcp`
-/// tolerates that for servers from before the exchange, `secure_tcp_required` does not.
-async fn key_exchange(conn: &mut Stream, key: &str, log_on_success: bool) -> ResultType<bool> {
     let rs_pk = get_rs_pk(key);
     let Some(rs_pk) = rs_pk else {
         bail!("Handshake failed: invalid public key from rendezvous server");
@@ -2114,7 +2107,6 @@ async fn key_exchange(conn: &mut Stream, key: &str, log_on_success: bool) -> Res
                         if log_on_success {
                             log::info!("Connection secured");
                         }
-                        return Ok(true);
                     }
                     _ => {}
                 }
@@ -2122,7 +2114,7 @@ async fn key_exchange(conn: &mut Stream, key: &str, log_on_success: bool) -> Res
         }
         _ => {}
     }
-    Ok(false)
+    Ok(())
 }
 
 pub async fn secure_tcp(conn: &mut Stream, key: &str) -> ResultType<()> {
@@ -2133,22 +2125,6 @@ async fn secure_tcp_silent(conn: &mut Stream, key: &str) -> ResultType<()> {
     secure_tcp_impl(conn, key, false).await
 }
 
-/// Like [`secure_tcp`], but returns only once the server's key exchange has actually encrypted
-/// the stream; a server that answers with anything else, or with nothing, is an error, so the
-/// caller can withhold what it was about to send instead of sending it in the clear.
-/// `secure_tcp` keeps tolerating such a server, which the paths from before the exchange depend
-/// on. WebSocket is treated as `secure_tcp` treats it, as a transport that is encrypted already.
-pub async fn secure_tcp_required(conn: &mut Stream, key: &str) -> ResultType<()> {
-    if use_ws() {
-        return Ok(());
-    }
-    if key_exchange(conn, key, true).await? {
-        Ok(())
-    } else {
-        bail!("the rendezvous server did not complete the key exchange");
-    }
-}
-
 #[inline]
 fn get_pk(pk: &[u8]) -> Option<[u8; 32]> {
     if pk.len() == 32 {
@@ -3287,88 +3263,4 @@ mod tests {
         assert_eq!(combined_mask & MOUSE_TYPE_MASK, MOUSE_TYPE_DOWN);
         assert_eq!(combined_mask >> 3, MOUSE_BUTTON_LEFT | MOUSE_BUTTON_RIGHT);
     }
-
-    /// A stand-in rendezvous server on loopback: accepts one connection and hands it to `serve`.
-    async fn rendezvous_stub<F, Fut>(serve: F) -> String
-    where
-        F: FnOnce(hbb_common::tcp::FramedStream) -> Fut + Send + 'static,
-        Fut: std::future::Future<Output = ()> + Send + 'static,
-    {
-        let listener = hbb_common::tcp::new_listener("127.0.0.1:0", false)
-            .await
-            .unwrap();
-        let host = listener.local_addr().unwrap().to_string();
-        tokio::spawn(async move {
-            if let Ok((stream, addr)) = listener.accept().await {
-                serve(hbb_common::tcp::FramedStream::from(stream, addr)).await;
-            }
-        });
-        host
-    }
-
-    fn server_key() -> (String, sign::SecretKey) {
-        let (pk, sk) = sign::gen_keypair();
-        (encode64(pk.0), sk)
-    }
-
-    async fn connect(host: &str) -> Stream {
-        hbb_common::socket_client::connect_tcp(host.to_owned(), 3000)
-            .await
-            .unwrap()
-    }
-
-    #[tokio::test]
-    async fn test_secure_tcp_required_refuses_a_server_without_the_exchange() {
-        let (key, _) = server_key();
-        // A server from before the exchange answers the first message with something else.
-        let serve = |mut s: hbb_common::tcp::FramedStream| async move {
-            let mut msg = RendezvousMessage::new();
-            msg.set_register_peer_response(RegisterPeerResponse::new());
-            s.send(&msg).await.unwrap();
-            sleep(Duration::from_secs(2)).await;
-        };
-        let host = rendezvous_stub(serve).await;
-        let mut conn = connect(&host).await;
-        assert!(secure_tcp_required(&mut conn, &key).await.is_err());
-        assert!(!conn.is_secured());
-        // The legacy call tolerates the same server, and the stream stays in the clear.
-        let host = rendezvous_stub(serve).await;
-        let mut conn = connect(&host).await;
-        secure_tcp(&mut conn, &key).await.unwrap();
-        assert!(!conn.is_secured());
-    }
-
-    #[tokio::test]
-    async fn test_secure_tcp_required_refuses_a_closed_connection() {
-        let (key, _) = server_key();
-        let host = rendezvous_stub(|s| async move { drop(s) }).await;
-        let mut conn = connect(&host).await;
-        assert!(secure_tcp_required(&mut conn, &key).await.is_err());
-        assert!(!conn.is_secured());
-    }
-
-    #[tokio::test]
-    async fn test_secure_tcp_required_accepts_a_completed_exchange() {
-        let (key, sk) = server_key();
-        let host = rendezvous_stub(move |mut s| async move {
-            let (eph_pk, eph_sk) = box_::gen_keypair();
-            let mut msg = RendezvousMessage::new();
-            msg.set_key_exchange(KeyExchange {
-                keys: vec![sign::sign(&eph_pk.0, &sk).into()],
-                ..Default::default()
-            });
-            s.send(&msg).await.unwrap();
-            // The client's reply must decode to a key with the ephemeral secret half.
-            let reply = s.next_timeout(3000).await.unwrap().unwrap();
-            let reply = RendezvousMessage::parse_from_bytes(&reply).unwrap();
-            let Some(rendezvous_message::Union::KeyExchange(ex)) = reply.union else {
-                panic!("expected the client's key exchange");
-            };
-            hbb_common::tcp::Encrypt::decode(&ex.keys[1], &ex.keys[0], &eph_sk).unwrap();
-        })
-        .await;
-        let mut conn = connect(&host).await;
-        secure_tcp_required(&mut conn, &key).await.unwrap();
-        assert!(conn.is_secured());
-    }
 }

src/platform/linux.rs

@@ -2370,17 +2370,10 @@ mod desktop {
             last
         }
 
-        /// Preserves an active seat0 session's cached identity so the service loop only retries
-        /// late Wayland display discovery instead of repeating the full seat lookup.
         pub fn refresh(&mut self) {
             if !self.sid.is_empty() && is_active_and_seat0(&self.sid) {
                 // Xwayland display and xauth may not be available in a short time after login.
-                // Avoid scanning processes on X11, where Xwayland discovery cannot provide any
-                // useful session information.
-                if self.is_wayland()
-                    && !self.is_login_wayland()
-                    && is_xwayland_running(&self.uid)
-                {
+                if is_xwayland_running(&self.uid) && !self.is_login_wayland() {
                     self.get_display_xauth_xwayland();
                 } else if self.is_wayland() {
                     self.get_display_xauth_wayland();

src/rendezvous_mediator.rs

@@ -625,20 +625,6 @@ impl RendezvousMediator {
         );
 
         let mut socket = connect_tcp(&*self.host, CONNECT_TIMEOUT).await?;
-        // A relay response carrying an answer carries this machine's ICE candidates with it, so
-        // that half goes out only on an encrypted channel. A server that does not complete the
-        // exchange loses the answer, not the relay: the response goes without it, on a fresh
-        // socket since the failed exchange may have consumed a message on this one, and the
-        // controller falls back to its other transports.
-        let mut webrtc_sdp_answer = webrtc_sdp_answer;
-        if !webrtc_sdp_answer.is_empty() {
-            let key = crate::get_key(true).await;
-            if let Err(err) = crate::secure_tcp_required(&mut socket, &key).await {
-                log::warn!("relaying without the WebRTC answer, it cannot be encrypted: {err}");
-                webrtc_sdp_answer = String::new();
-                socket = connect_tcp(&*self.host, CONNECT_TIMEOUT).await?;
-            }
-        }
 
         let mut msg_out = Message::new();
         let mut rr = RelayResponse {
@@ -820,7 +806,6 @@ impl RendezvousMediator {
                 // trickle, and TCP reliability replaces the old 400ms duplicate re-send
                 // (the controller keeps its own re-send for the server->peer UDP downlink).
                 let mut conn = None;
-                let key = crate::get_key(true).await;
                 while let Some(candidate) = local_ice_rx.recv().await {
                     let mut msg = Message::new();
                     msg.set_ice_candidate(IceCandidate {
@@ -834,20 +819,7 @@ impl RendezvousMediator {
                     for _ in 0..2 {
                         if conn.is_none() {
                             match connect_tcp(&*host, CONNECT_TIMEOUT).await {
-                                Ok(mut s) => {
-                                    // Candidates are every interface address of this machine:
-                                    // sent only on a channel that is actually encrypted, else
-                                    // this WebRTC attempt goes without them.
-                                    if let Err(err) = crate::secure_tcp_required(&mut s, &key).await
-                                    {
-                                        log::warn!(
-                                            "failed to secure the WebRTC ICE candidate connection: {}",
-                                            err
-                                        );
-                                        break;
-                                    }
-                                    conn = Some(s);
-                                }
+                                Ok(s) => conn = Some(s),
                                 Err(err) => {
                                     log::warn!(
                                         "failed to connect for WebRTC ICE candidate: {}",
@@ -1021,9 +993,6 @@ impl RendezvousMediator {
             let mut msg_out = Message::new();
             msg_out.set_punch_hole_sent(msg_punch);
             let mut socket = connect_tcp(&*self.host, CONNECT_TIMEOUT).await?;
-            // The answer goes out only on a channel that is actually encrypted; otherwise this
-            // WebRTC attempt is abandoned and the controller falls back to its other transports.
-            crate::secure_tcp_required(&mut socket, &crate::get_key(true).await).await?;
             socket.send(&msg_out).await?;
             return Ok(());
         }