From 30b08124b4f60e8e91eca034f049d1a4ece22b01 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 8 May 2026 02:19:03 +0000 Subject: [PATCH 1/3] fix: harden clipboard handling and unstick Windows screen switch Consolidated bug fixes layered on top of upstream master, addressing clipboard corruption, large-image transfer crashes, and a cursor lockup on the Windows server side. Clipboard protocol & bounds: - IClipboard::unmarshall: check buffer length before readUInt32 so a truncated header cannot trigger an out-of-bounds read. - ClipboardChunk::assemble: validate the toULong parse result and use %zu for size_t in error messages. - StreamChunker::sendClipboard: stop the loop on size==0 instead of spinning forever. Windows clipboard converters: - AnyText: bounds check before scan[1] access; ensure GlobalUnlock when srcSize <= 1 in the text converter. - Bitmap: null-check GetDC / CreateDIBSection / CreateCompatibleDC; guard against integer overflow on huge bitmap dimensions; validate biBitCount range; handle V4/V5 DIB headers (BITMAPV4HEADER / BITMAPV5HEADER) so non-V1 images no longer crash on conversion. - HTML: bounds check for fragment start/end offsets. macOS / X11 clipboard: - BMP fromIClipboard: validate biSize range (40-1024) before reading DIB fields; handle V4/V5 headers consistently with Windows. - OSXClipboard: skip file-promise pasteboard data so a Finder copy of files no longer disconnects the macOS client. - Text/HTML converters: check CFStringGetBytes buffSize > 0 before allocation (was checking the wrong return value). Windows screen switch: - MSWindowsScreen::isAnyMouseButtonDown: verify cached m_buttons[] against GetAsyncKeyState before reporting the cursor as locked. A button-up event from the low-level mouse hook can be silently lost when the hook proc exceeds LowLevelHooksTimeout (~300 ms) during a multi-MB clipboard image transfer; without this check, m_buttons[Left] stays "down" forever and Server::isSwitchOkay() blocks every screen switch ("locked by Left Button" log spam). Stale entries are now cleared with a WARN so the condition is observable. https://claude.ai/code/session_012uK8ms6TCHrBsJGmHRDeWN --- src/lib/deskflow/ClipboardChunk.cpp | 12 +++- src/lib/deskflow/IClipboard.cpp | 2 +- src/lib/deskflow/StreamChunker.cpp | 5 +- .../MSWindowsClipboardAnyTextConverter.cpp | 9 ++- .../MSWindowsClipboardBitmapConverter.cpp | 66 +++++++++++++++---- .../MSWindowsClipboardHTMLConverter.cpp | 2 +- src/lib/platform/MSWindowsScreen.cpp | 20 ++++-- src/lib/platform/MSWindowsScreen.h | 2 +- src/lib/platform/OSXClipboard.cpp | 14 ++++ src/lib/platform/OSXClipboardBMPConverter.cpp | 59 +++++++++++++---- .../platform/OSXClipboardHTMLConverter.cpp | 6 +- .../platform/OSXClipboardTextConverter.cpp | 6 +- .../XWindowsClipboardBMPConverter.cpp | 58 ++++++++++++---- 13 files changed, 201 insertions(+), 60 deletions(-) diff --git a/src/lib/deskflow/ClipboardChunk.cpp b/src/lib/deskflow/ClipboardChunk.cpp index 7abbdba1aa8b..1a5f3fd61099 100644 --- a/src/lib/deskflow/ClipboardChunk.cpp +++ b/src/lib/deskflow/ClipboardChunk.cpp @@ -74,7 +74,12 @@ ClipboardChunk::assemble(deskflow::IStream *stream, std::string &dataCached, Cli } if (mark == ChunkType::DataStart) { - s_expectedSize = QString::fromStdString(data).toULong(); + bool ok = false; + s_expectedSize = QString::fromStdString(data).toULong(&ok); + if (!ok) { + LOG_ERR("invalid clipboard size string"); + return Error; + } LOG_DEBUG("start receiving clipboard data"); dataCached.clear(); return Started; @@ -86,7 +91,10 @@ ClipboardChunk::assemble(deskflow::IStream *stream, std::string &dataCached, Cli if (id >= kClipboardEnd) { return Error; } else if (s_expectedSize != dataCached.size()) { - LOG_ERR("corrupted clipboard data, expected size=%d actual size=%d", s_expectedSize, dataCached.size()); + LOG_ERR( + "corrupted clipboard data, expected size=%zu actual size=%zu", static_cast(s_expectedSize), + dataCached.size() + ); return Error; } return Finished; diff --git a/src/lib/deskflow/IClipboard.cpp b/src/lib/deskflow/IClipboard.cpp index 729ca1680f18..3122d4c2bcd7 100644 --- a/src/lib/deskflow/IClipboard.cpp +++ b/src/lib/deskflow/IClipboard.cpp @@ -28,12 +28,12 @@ void IClipboard::unmarshall(IClipboard *clipboard, const std::string_view &data, clipboard->empty(); // read the number of formats - const uint32_t numFormats = readUInt32(index); if (end - index < 4) { LOG_ERR("clipboard unmarshall: truncated header"); clipboard->close(); return; } + const uint32_t numFormats = readUInt32(index); index += 4; // read each format diff --git a/src/lib/deskflow/StreamChunker.cpp b/src/lib/deskflow/StreamChunker.cpp index b24b712c9cd3..2d8cee5677a0 100644 --- a/src/lib/deskflow/StreamChunker.cpp +++ b/src/lib/deskflow/StreamChunker.cpp @@ -27,7 +27,7 @@ void StreamChunker::sendClipboard( size_t sentLength = 0; size_t chunkSize = g_chunkSize; - while (true) { + while (sentLength < size) { // make sure we don't read too much from the mock data. if (sentLength + chunkSize > size) { chunkSize = size - sentLength; @@ -39,9 +39,6 @@ void StreamChunker::sendClipboard( events->addEvent(Event(EventTypes::ClipboardSending, eventTarget, dataChunk)); sentLength += chunkSize; - if (sentLength == size) { - break; - } } // send last message diff --git a/src/lib/platform/MSWindowsClipboardAnyTextConverter.cpp b/src/lib/platform/MSWindowsClipboardAnyTextConverter.cpp index 655b2becf2e9..b157e3764251 100644 --- a/src/lib/platform/MSWindowsClipboardAnyTextConverter.cpp +++ b/src/lib/platform/MSWindowsClipboardAnyTextConverter.cpp @@ -42,10 +42,13 @@ MSWindowsClipboardAnyTextConverter::fromIClipboard(const std::string &data) cons std::string MSWindowsClipboardAnyTextConverter::toIClipboard(HANDLE data) const { - // get datator + // get data pointer const char *src = (const char *)GlobalLock(data); uint32_t srcSize = (uint32_t)GlobalSize(data); if (src == nullptr || srcSize <= 1) { + if (src != nullptr) { + GlobalUnlock(data); + } return std::string(); } @@ -97,7 +100,7 @@ std::string MSWindowsClipboardAnyTextConverter::convertLinefeedToUnix(const std: uint32_t numNewlines = 0; uint32_t n = (uint32_t)src.size(); for (const char *scan = src.c_str(); n > 0; ++scan, --n) { - if (scan[0] == '\r' && scan[1] == '\n') { + if (scan[0] == '\r' && n > 1 && scan[1] == '\n') { ++numNewlines; } } @@ -112,7 +115,7 @@ std::string MSWindowsClipboardAnyTextConverter::convertLinefeedToUnix(const std: // copy string, converting newlines n = (uint32_t)src.size(); for (const char *scan = src.c_str(); n > 0; ++scan, --n) { - if (scan[0] != '\r' || scan[1] != '\n') { + if (scan[0] != '\r' || n <= 1 || scan[1] != '\n') { dst += scan[0]; } } diff --git a/src/lib/platform/MSWindowsClipboardBitmapConverter.cpp b/src/lib/platform/MSWindowsClipboardBitmapConverter.cpp index 16150c32fc16..69194df94e80 100644 --- a/src/lib/platform/MSWindowsClipboardBitmapConverter.cpp +++ b/src/lib/platform/MSWindowsClipboardBitmapConverter.cpp @@ -45,13 +45,18 @@ MSWindowsClipboardBitmapConverter::fromIClipboard(const std::string &data) const std::string MSWindowsClipboardBitmapConverter::toIClipboard(HANDLE data) const { - // get datator + // get data pointer LPVOID src = GlobalLock(data); if (src == nullptr) { return std::string(); } uint32_t srcSize = (uint32_t)GlobalSize(data); + if (srcSize < sizeof(BITMAPINFOHEADER)) { + GlobalUnlock(data); + return std::string(); + } + // check image type const BITMAPINFO *bitmap = static_cast(src); LOG( @@ -66,12 +71,26 @@ std::string MSWindowsClipboardBitmapConverter::toIClipboard(HANDLE data) const return image; } + // validate dimensions + LONG w = bitmap->bmiHeader.biWidth; + LONG h = bitmap->bmiHeader.biHeight; + if (w <= 0 || h == 0) { + GlobalUnlock(data); + return std::string(); + } + LONG absH = (h > 0) ? h : -h; + + // check for integer overflow in pixel data size (4 bytes per pixel) + if (w > 32767 || absH > 32767 || static_cast(w) * absH > 0x40000000ULL) { + LOG_WARN("bitmap too large: %dx%d", w, absH); + GlobalUnlock(data); + return std::string(); + } + // create a destination DIB section LOG_INFO("convert image from: depth=%d comp=%d", bitmap->bmiHeader.biBitCount, bitmap->bmiHeader.biCompression); void *raw; BITMAPINFOHEADER info; - LONG w = bitmap->bmiHeader.biWidth; - LONG h = bitmap->bmiHeader.biHeight; info.biSize = sizeof(BITMAPINFOHEADER); info.biWidth = w; info.biHeight = h; @@ -84,33 +103,52 @@ std::string MSWindowsClipboardBitmapConverter::toIClipboard(HANDLE data) const info.biClrUsed = 0; info.biClrImportant = 0; HDC dc = GetDC(nullptr); + if (dc == nullptr) { + GlobalUnlock(data); + return std::string(); + } HBITMAP dst = CreateDIBSection(dc, (BITMAPINFO *)&info, DIB_RGB_COLORS, &raw, nullptr, 0); + if (dst == nullptr) { + ReleaseDC(nullptr, dc); + GlobalUnlock(data); + return std::string(); + } // find the start of the pixel data const char *srcBits = (const char *)bitmap + bitmap->bmiHeader.biSize; - if (bitmap->bmiHeader.biBitCount >= 16) { - if (bitmap->bmiHeader.biCompression == BI_BITFIELDS && - (bitmap->bmiHeader.biBitCount == 16 || bitmap->bmiHeader.biBitCount == 32)) { - srcBits += 3 * sizeof(DWORD); + // Only BITMAPINFOHEADER (40 bytes) has color masks/table after the header. + // BITMAPV4HEADER (108 bytes) and BITMAPV5HEADER (124 bytes) include + // masks and color space info within the header itself. + if (bitmap->bmiHeader.biSize == sizeof(BITMAPINFOHEADER)) { + if (bitmap->bmiHeader.biBitCount >= 16) { + if (bitmap->bmiHeader.biCompression == BI_BITFIELDS && + (bitmap->bmiHeader.biBitCount == 16 || bitmap->bmiHeader.biBitCount == 32)) { + srcBits += 3 * sizeof(DWORD); + } + } else if (bitmap->bmiHeader.biClrUsed != 0) { + srcBits += bitmap->bmiHeader.biClrUsed * sizeof(RGBQUAD); + } else if (bitmap->bmiHeader.biBitCount > 0 && bitmap->bmiHeader.biBitCount <= 8) { + srcBits += (1i64 << bitmap->bmiHeader.biBitCount) * sizeof(RGBQUAD); } - } else if (bitmap->bmiHeader.biClrUsed != 0) { - srcBits += bitmap->bmiHeader.biClrUsed * sizeof(RGBQUAD); - } else { - // http://msdn.microsoft.com/en-us/library/ke55d167(VS.80).aspx - srcBits += (1i64 << bitmap->bmiHeader.biBitCount) * sizeof(RGBQUAD); } // copy source image to destination image HDC dstDC = CreateCompatibleDC(dc); + if (dstDC == nullptr) { + DeleteObject(dst); + ReleaseDC(nullptr, dc); + GlobalUnlock(data); + return std::string(); + } HGDIOBJ oldBitmap = SelectObject(dstDC, dst); - SetDIBitsToDevice(dstDC, 0, 0, w, h, 0, 0, 0, h, srcBits, bitmap, DIB_RGB_COLORS); + SetDIBitsToDevice(dstDC, 0, 0, w, absH, 0, 0, 0, absH, srcBits, bitmap, DIB_RGB_COLORS); SelectObject(dstDC, oldBitmap); DeleteDC(dstDC); GdiFlush(); // extract data std::string image((const char *)&info, info.biSize); - image.append((const char *)raw, 4 * w * h); + image.append((const char *)raw, 4LL * w * absH); // clean up GDI DeleteObject(dst); diff --git a/src/lib/platform/MSWindowsClipboardHTMLConverter.cpp b/src/lib/platform/MSWindowsClipboardHTMLConverter.cpp index 9f55bb070a9f..3ee36d450d98 100644 --- a/src/lib/platform/MSWindowsClipboardHTMLConverter.cpp +++ b/src/lib/platform/MSWindowsClipboardHTMLConverter.cpp @@ -67,7 +67,7 @@ std::string MSWindowsClipboardHTMLConverter::doToIClipboard(const std::string &d // convert args to integers int32_t start = (int32_t)atoi(startArg.c_str()); int32_t end = (int32_t)atoi(endArg.c_str()); - if (start <= 0 || end <= 0 || start >= end) { + if (start <= 0 || end <= 0 || start >= end || static_cast(end) > data.size()) { return std::string(); } diff --git a/src/lib/platform/MSWindowsScreen.cpp b/src/lib/platform/MSWindowsScreen.cpp index f2f5a164695d..fe5ce01c057f 100644 --- a/src/lib/platform/MSWindowsScreen.cpp +++ b/src/lib/platform/MSWindowsScreen.cpp @@ -669,13 +669,25 @@ bool MSWindowsScreen::isAnyMouseButtonDown(uint32_t &buttonID) const { static const char *buttonToName[] = {"", "Left Button", "Middle Button", "Right Button", "X Button 1", "X Button 2"}; + // index by ButtonID (kButtonNone, kButtonLeft, kButtonMiddle, kButtonRight, X1, X2) + static const int vkCodes[] = {0, VK_LBUTTON, VK_MBUTTON, VK_RBUTTON, VK_XBUTTON1, VK_XBUTTON2}; for (uint32_t i = 1; i < sizeof(m_buttons) / sizeof(m_buttons[0]); ++i) { - if (m_buttons[i]) { - buttonID = i; - LOG_DEBUG("locked by \"%s\"", buttonToName[i]); - return true; + if (!m_buttons[i]) { + continue; + } + // verify against actual hardware state: a button-up event from the + // low-level hook can be lost (e.g. when the hook is bypassed during + // heavy clipboard transfer), leaving the cached state stuck "down" + // and the cursor permanently locked to the screen. + if (!(GetAsyncKeyState(vkCodes[i]) & 0x8000)) { + LOG_WARN("clearing stuck mouse button state: \"%s\"", buttonToName[i]); + m_buttons[i] = false; + continue; } + buttonID = i; + LOG_DEBUG("locked by \"%s\"", buttonToName[i]); + return true; } return false; diff --git a/src/lib/platform/MSWindowsScreen.h b/src/lib/platform/MSWindowsScreen.h index 4d6e951d4bcd..e6bcad5ea7d7 100644 --- a/src/lib/platform/MSWindowsScreen.h +++ b/src/lib/platform/MSWindowsScreen.h @@ -316,7 +316,7 @@ class MSWindowsScreen : public PlatformScreen HotKeyToIDMap m_hotKeyToIDMap; // map of button state - bool m_buttons[NumButtonIDs]; + mutable bool m_buttons[NumButtonIDs]; // m_hasMouse is true if there's a mouse attached to the system or // mouse keys is simulating one. we track this so we can force the diff --git a/src/lib/platform/OSXClipboard.cpp b/src/lib/platform/OSXClipboard.cpp index 02608108ce57..7d76cd9e0ae1 100644 --- a/src/lib/platform/OSXClipboard.cpp +++ b/src/lib/platform/OSXClipboard.cpp @@ -139,6 +139,20 @@ bool OSXClipboard::has(Format format) const PasteboardItemID item; PasteboardGetItemIdentifier(m_pboard, (CFIndex)1, &item); + // For bitmap format, only report available if the pasteboard actually + // contains native image data (TIFF or PNG). Without this check, macOS + // may claim com.microsoft.bmp is available via UTI auto-conversion + // for non-image pasteboard content (e.g. file references), but the + // actual conversion can hang and block the event loop, causing the + // client to disconnect due to keep-alive timeout. + if (format == IClipboard::Format::Bitmap) { + PasteboardFlavorFlags flags; + if (PasteboardGetItemFlavorFlags(m_pboard, item, CFSTR("public.tiff"), &flags) != noErr && + PasteboardGetItemFlavorFlags(m_pboard, item, CFSTR("public.png"), &flags) != noErr) { + return false; + } + } + for (ConverterList::const_iterator index = m_converters.begin(); index != m_converters.end(); ++index) { IOSXClipboardConverter *converter = *index; if (converter->getFormat() == format) { diff --git a/src/lib/platform/OSXClipboardBMPConverter.cpp b/src/lib/platform/OSXClipboardBMPConverter.cpp index 637a1cf6d728..28655759d392 100644 --- a/src/lib/platform/OSXClipboardBMPConverter.cpp +++ b/src/lib/platform/OSXClipboardBMPConverter.cpp @@ -20,6 +20,11 @@ struct CBMPHeader }; // BMP is little-endian +static inline uint16_t fromLEU16(const uint8_t *data) +{ + return static_cast(data[0]) | (static_cast(data[1]) << 8); +} + static inline uint32_t fromLEU32(const uint8_t *data) { return static_cast(data[0]) | (static_cast(data[1]) << 8) | @@ -62,7 +67,43 @@ CFStringRef OSXClipboardBMPConverter::getOSXFormat() const std::string OSXClipboardBMPConverter::fromIClipboard(const std::string &bmp) const { LOG_DEBUG1("getting data from clipboard"); - // create BMP image + + if (bmp.size() < 4) { + return std::string(); + } + + // read the actual DIB header size from biSize + const uint8_t *rawDIB = reinterpret_cast(bmp.data()); + uint32_t biSize = fromLEU32(rawDIB); + + // validate biSize is reasonable + if (biSize < 40 || biSize > 1024) { + return std::string(); + } + + // compute pixel data offset: file header + DIB header + color table + uint32_t pixelOffset = 14 + biSize; + + if (bmp.size() >= biSize) { + uint16_t biBitCount = fromLEU16(rawDIB + 14); + uint32_t biCompression = fromLEU32(rawDIB + 16); + uint32_t biClrUsed = fromLEU32(rawDIB + 32); + + // BITMAPINFOHEADER with BI_BITFIELDS has 3 DWORD color masks after header + if (biSize == 40 && biCompression == 3 /* BI_BITFIELDS */) { + pixelOffset += 3 * 4; + } + + // add color table size + if (biBitCount <= 8) { + uint32_t colors = biClrUsed ? biClrUsed : (1u << biBitCount); + pixelOffset += colors * 4; + } else if (biClrUsed > 0) { + pixelOffset += biClrUsed * 4; + } + } + + // create BMP file header uint8_t header[14]; uint8_t *dst = header; toLE(dst, 'B'); @@ -70,13 +111,13 @@ std::string OSXClipboardBMPConverter::fromIClipboard(const std::string &bmp) con toLE(dst, static_cast(14 + bmp.size())); toLE(dst, static_cast(0)); toLE(dst, static_cast(0)); - toLE(dst, static_cast(14 + 40)); + toLE(dst, pixelOffset); return std::string(reinterpret_cast(header), 14) + bmp; } std::string OSXClipboardBMPConverter::toIClipboard(const std::string &bmp) const { - // make sure data is big enough for a BMP file + // make sure data is big enough for a BMP file header + minimal DIB header if (bmp.size() <= 14 + 40) { return std::string(); } @@ -87,13 +128,7 @@ std::string OSXClipboardBMPConverter::toIClipboard(const std::string &bmp) const return std::string(); } - // get offset to image data - uint32_t offset = fromLEU32(rawBMPHeader + 10); - - // construct BMP - if (offset == 14 + 40) { - return bmp.substr(14); - } else { - return bmp.substr(14, 40) + bmp.substr(offset, bmp.size() - offset); - } + // strip the 14-byte BMP file header, keep the entire DIB + // (info header + optional color table + pixel data) + return bmp.substr(14); } diff --git a/src/lib/platform/OSXClipboardHTMLConverter.cpp b/src/lib/platform/OSXClipboardHTMLConverter.cpp index 460c86893ef0..8c4ba4a39978 100644 --- a/src/lib/platform/OSXClipboardHTMLConverter.cpp +++ b/src/lib/platform/OSXClipboardHTMLConverter.cpp @@ -34,13 +34,13 @@ std::string OSXClipboardHTMLConverter::convertString( CFStringGetBytes(stringRef, entireString, toEncoding, 0, false, nullptr, 0, &buffSize); - char *buffer = new char[buffSize]; - - if (buffer == nullptr) { + if (buffSize <= 0) { CFRelease(stringRef); return std::string(); } + char *buffer = new char[buffSize]; + CFStringGetBytes(stringRef, entireString, toEncoding, 0, false, (uint8_t *)buffer, buffSize, nullptr); std::string result(buffer, buffSize); diff --git a/src/lib/platform/OSXClipboardTextConverter.cpp b/src/lib/platform/OSXClipboardTextConverter.cpp index 51b24fc305c4..651ee48fb0a5 100644 --- a/src/lib/platform/OSXClipboardTextConverter.cpp +++ b/src/lib/platform/OSXClipboardTextConverter.cpp @@ -31,13 +31,13 @@ std::string OSXClipboardTextConverter::convertString( CFStringGetBytes(stringRef, entireString, toEncoding, 0, false, nullptr, 0, &buffSize); - char *buffer = new char[buffSize]; - - if (buffer == nullptr) { + if (buffSize <= 0) { CFRelease(stringRef); return std::string(); } + char *buffer = new char[buffSize]; + CFStringGetBytes(stringRef, entireString, toEncoding, 0, false, (uint8_t *)buffer, buffSize, nullptr); std::string result(buffer, buffSize); diff --git a/src/lib/platform/XWindowsClipboardBMPConverter.cpp b/src/lib/platform/XWindowsClipboardBMPConverter.cpp index 8d103e3e93a0..038183f94ecc 100644 --- a/src/lib/platform/XWindowsClipboardBMPConverter.cpp +++ b/src/lib/platform/XWindowsClipboardBMPConverter.cpp @@ -19,6 +19,11 @@ struct CBMPHeader }; // BMP is little-endian +static inline uint16_t fromLEU16(const uint8_t *data) +{ + return static_cast(data[0]) | (static_cast(data[1]) << 8); +} + static inline uint32_t fromLEU32(const uint8_t *data) { return static_cast(data[0]) | (static_cast(data[1]) << 8) | @@ -74,7 +79,42 @@ int XWindowsClipboardBMPConverter::getDataSize() const std::string XWindowsClipboardBMPConverter::fromIClipboard(const std::string &bmp) const { - // create BMP image + if (bmp.size() < 4) { + return std::string(); + } + + // read the actual DIB header size from biSize + const uint8_t *rawDIB = reinterpret_cast(bmp.data()); + uint32_t biSize = fromLEU32(rawDIB); + + // validate biSize is reasonable + if (biSize < 40 || biSize > 1024) { + return std::string(); + } + + // compute pixel data offset: file header + DIB header + color table + uint32_t pixelOffset = 14 + biSize; + + if (bmp.size() >= biSize) { + uint16_t biBitCount = fromLEU16(rawDIB + 14); + uint32_t biCompression = fromLEU32(rawDIB + 16); + uint32_t biClrUsed = fromLEU32(rawDIB + 32); + + // BITMAPINFOHEADER with BI_BITFIELDS has 3 DWORD color masks after header + if (biSize == 40 && biCompression == 3 /* BI_BITFIELDS */) { + pixelOffset += 3 * 4; + } + + // add color table size + if (biBitCount <= 8) { + uint32_t colors = biClrUsed ? biClrUsed : (1u << biBitCount); + pixelOffset += colors * 4; + } else if (biClrUsed > 0) { + pixelOffset += biClrUsed * 4; + } + } + + // create BMP file header uint8_t header[14]; uint8_t *dst = header; toLE(dst, 'B'); @@ -82,13 +122,13 @@ std::string XWindowsClipboardBMPConverter::fromIClipboard(const std::string &bmp toLE(dst, static_cast(14 + bmp.size())); toLE(dst, static_cast(0)); toLE(dst, static_cast(0)); - toLE(dst, static_cast(14 + 40)); + toLE(dst, pixelOffset); return std::string(reinterpret_cast(header), 14) + bmp; } std::string XWindowsClipboardBMPConverter::toIClipboard(const std::string &bmp) const { - // make sure data is big enough for a BMP file + // make sure data is big enough for a BMP file header + minimal DIB header if (bmp.size() <= 14 + 40) { return std::string(); } @@ -99,13 +139,7 @@ std::string XWindowsClipboardBMPConverter::toIClipboard(const std::string &bmp) return std::string(); } - // get offset to image data - uint32_t offset = fromLEU32(rawBMPHeader + 10); - - // construct BMP - if (offset == 14 + 40) { - return bmp.substr(14); - } else { - return bmp.substr(14, 40) + bmp.substr(offset, bmp.size() - offset); - } + // strip the 14-byte BMP file header, keep the entire DIB + // (info header + optional color table + pixel data) + return bmp.substr(14); } From 75379885648da54afe76d4961b4fa18e1eea4054 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 8 May 2026 12:23:35 +0000 Subject: [PATCH 2/3] fix(net): reset keep-alive watchdog on any recognized message Both ClientProxy1_3 (server) and ServerProxy (client) only reset their heartbeat / keep-alive alarm when they receive an explicit kMsgCKeepAlive echo. Every other inbound message - mouse moves, key events, clipboard chunks - leaves the watchdog running. That breaks under bulk clipboard transfer: while the peer ships a multi-MB clipboard payload to us, its outgoing socket queue is also where its keep-alive echo has to wait. The echo can be delayed past the 3 * kKeepAliveRate (=9s) deadline even though data is actively flowing the whole time. Result: "client/server is dead" while the connection is in fact perfectly healthy and busy. Once the watchdog fires we close the socket, the cursor cannot return to the peer screen, and the user has to wait for a reconnect. Reset the watchdog whenever we successfully parse a frame from the peer - any recognized inbound traffic is proof the link is alive. The explicit keep-alive path still works as before; it is just no longer the only thing that resets the timer. Repro from logs (2026-05-08T10:46:38 session): - 10:46:45.139 server: start receiving clipboard data (8 MB x2) - 10:46:45.252 server: clipboard fully written to Windows - 10:46:46-51 server: button/key events from client (link clearly alive) - 10:46:53.358 server: "client \"hikaris-Mac-Pro.local\" is dead" - 10:46:53.885 client: tls error occurred (server killed the socket) https://claude.ai/code/session_012uK8ms6TCHrBsJGmHRDeWN --- src/lib/client/ServerProxy.cpp | 8 ++++++++ src/lib/server/ClientProxy1_3.cpp | 19 ++++++++++++++----- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/lib/client/ServerProxy.cpp b/src/lib/client/ServerProxy.cpp index a04aa35e293b..3c965dc8c5ab 100644 --- a/src/lib/client/ServerProxy.cpp +++ b/src/lib/client/ServerProxy.cpp @@ -201,6 +201,8 @@ ServerProxy::ConnectionResult ServerProxy::parseHandshakeMessage(const uint8_t * return Unknown; } + // any recognized inbound traffic proves the server is alive + resetKeepAliveAlarm(); return Okay; } @@ -321,6 +323,12 @@ ServerProxy::ConnectionResult ServerProxy::parseMessage(const uint8_t *code) return Unknown; } + // any recognized inbound traffic proves the server is alive; reset + // the watchdog so a long bulk transfer (e.g. multi-MB clipboard + // chunks from the server) does not starve the explicit keep-alive + // echo and trigger a false "server is dead" alarm. + resetKeepAliveAlarm(); + // send a reply. this is intended to work around a delay when // running a linux server and an OS X (any BSD?) client. the // client waits to send an ACK (if the system control flag diff --git a/src/lib/server/ClientProxy1_3.cpp b/src/lib/server/ClientProxy1_3.cpp index e1ee0476fa73..6b2c4777b6c4 100644 --- a/src/lib/server/ClientProxy1_3.cpp +++ b/src/lib/server/ClientProxy1_3.cpp @@ -39,14 +39,23 @@ void ClientProxy1_3::mouseWheel(int32_t xDelta, int32_t yDelta) bool ClientProxy1_3::parseMessage(const uint8_t *code) { - // process message + bool handled; if (memcmp(code, kMsgCKeepAlive, 4) == 0) { - // reset alarm - resetHeartbeatTimer(); - return true; + // explicit keep-alive echo from the client + handled = true; } else { - return ClientProxy1_2::parseMessage(code); + handled = ClientProxy1_2::parseMessage(code); + } + + // any recognized inbound traffic proves the client is alive. without + // this, a long bulk transfer (e.g. multi-MB clipboard chunks) can + // starve the keep-alive echo path - the echo sits behind the bulk + // data in the client's send queue - and the heartbeat alarm fires + // even though data is actively flowing. + if (handled) { + resetHeartbeatTimer(); } + return handled; } void ClientProxy1_3::resetHeartbeatRate() From 728ed04be90fb75e3b932094ff5febb44e8afe6a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 8 May 2026 12:44:40 +0000 Subject: [PATCH 3/3] perf(clipboard): skip duplicate kClipboardSelection grab on Mac/Win Both OSXScreen::checkClipboards and MSWindowsScreen (checkClipboards and onClipboardChange) fired ClipboardGrabbed for kClipboardClipboard *and* kClipboardSelection back-to-back whenever the local clipboard changed. On these platforms there is only one OS clipboard, so the two events carry identical content and lead to two identical chunked transfers on the wire. For an 8 MB BMP screenshot that meant 16 MB on the network, every clipboard change. With the keep-alive fix this no longer disconnects the client, but it still doubles bandwidth, marshalling cost, and the window during which a peer might mispaste from a stale state. Drop the kClipboardSelection grab on Mac/Win. Net effect: clipboard traffic from these platforms is halved. Caveat: X11 PRIMARY (mouse-selection) sync from a Mac/Win sender is not supported anymore. Mac/Win never had a PRIMARY-equivalent in the first place, so the previous "support" was a no-op for the user (payload 1 was just a copy of payload 0). X11 senders still ship both clipboards independently, so X11 -> X11 is unchanged. https://claude.ai/code/session_012uK8ms6TCHrBsJGmHRDeWN --- src/lib/platform/MSWindowsScreen.cpp | 6 ++++-- src/lib/platform/OSXScreen.mm | 6 +++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/lib/platform/MSWindowsScreen.cpp b/src/lib/platform/MSWindowsScreen.cpp index fe5ce01c057f..45079c976622 100644 --- a/src/lib/platform/MSWindowsScreen.cpp +++ b/src/lib/platform/MSWindowsScreen.cpp @@ -349,8 +349,9 @@ void MSWindowsScreen::checkClipboards() if (m_ownClipboard && !MSWindowsClipboard::isOwnedByDeskflow()) { LOG_DEBUG("clipboard changed: lost ownership and no notification received"); m_ownClipboard = false; + // Windows has a single OS clipboard, so we only grab kClipboardClipboard. + // See OSXScreen::checkClipboards for the reasoning. sendClipboardEvent(EventTypes::ClipboardGrabbed, kClipboardClipboard); - sendClipboardEvent(EventTypes::ClipboardGrabbed, kClipboardSelection); } } @@ -1354,8 +1355,9 @@ void MSWindowsScreen::onClipboardChange() if (m_ownClipboard) { LOG_DEBUG("clipboard changed: lost ownership"); m_ownClipboard = false; + // Windows has a single OS clipboard, so we only grab kClipboardClipboard. + // See OSXScreen::checkClipboards for the reasoning. sendClipboardEvent(EventTypes::ClipboardGrabbed, kClipboardClipboard); - sendClipboardEvent(EventTypes::ClipboardGrabbed, kClipboardSelection); } } else if (!m_ownClipboard) { LOG_DEBUG("clipboard changed: %s owned", kAppId); diff --git a/src/lib/platform/OSXScreen.mm b/src/lib/platform/OSXScreen.mm index c1ad2fc09d4e..4956581c3692 100644 --- a/src/lib/platform/OSXScreen.mm +++ b/src/lib/platform/OSXScreen.mm @@ -810,8 +810,12 @@ LOG_DEBUG2("checking clipboard"); if (m_pasteboard.synchronize()) { LOG_DEBUG("clipboard changed"); + // macOS has a single pasteboard, so we only grab kClipboardClipboard. + // Firing kClipboardSelection too would duplicate every payload on the + // wire (an N-byte clipboard becomes 2N on the network, both halves + // identical). X11 PRIMARY-selection sync from this client is not + // supported as a result; X11 still ships its CLIPBOARD via id 0. sendClipboardEvent(EventTypes::ClipboardGrabbed, kClipboardClipboard); - sendClipboardEvent(EventTypes::ClipboardGrabbed, kClipboardSelection); } }