Skip to content

feat(clipboard): add bidirectional text/image clipboard sync#608

Open
skyhua0224 wants to merge 6 commits intoAlkaidLab:masterfrom
skyhua0224:feature/clipboard-sync
Open

feat(clipboard): add bidirectional text/image clipboard sync#608
skyhua0224 wants to merge 6 commits intoAlkaidLab:masterfrom
skyhua0224:feature/clipboard-sync

Conversation

@skyhua0224
Copy link
Copy Markdown

背景

Foundation Sunshine 目前缺少与兼容 Moonlight 客户端对接的通用剪贴板同步主机路径,文本和图片内容无法通过现有控制通道完成双向同步。

修改

stream / protocol 接入:

  1. stream.cpp 中接入 clipboard 控制消息处理,支持 session bind / unbind / snapshot 以及 item 分片收发
  2. 仅在客户端协商到对应 capability 时启用文本 / 图片剪贴板能力,不影响未支持客户端的原有串流流程

Windows clipboard backend:

  1. 新增 Windows 剪贴板读写 backend,支持文本与单张图片的读取、写入与变更监听
  2. 维护活动剪贴板会话,只向当前绑定的会话推送主机侧剪贴板更新
  3. 增加 echo suppression,避免主机与客户端之间回弹同步

配置与 Web UI:

  1. 新增 clipboard_sync 配置项,默认启用
  2. 在 Inputs 页面增加剪贴板同步开关及中英文文案

行为

  • 复用现有 control stream,不新增端口
  • 当前仅支持文本与单张图片
  • 客户端绑定后可请求主机快照用于初始同步
  • 失焦、解绑或断流后停止同步

兼容性

  • 未协商 clipboard capability 的客户端行为不变
  • 关闭 clipboard_sync 后不启用该同步路径

Add the Windows-side clipboard sync path for text and single-image items, and expose a default-enabled clipboard toggle in the 47990 Web UI Input settings.
Copilot AI review requested due to automatic review settings April 23, 2026 08:16
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 23, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Summary by CodeRabbit

新功能

  • 剪贴板同步 - 新增主机与客户端之间的双向剪贴板同步功能,支持文本和图像(单张)内容同步,兼容Moonlight客户端
  • 配置选项 - 在输入设置中新增剪贴板同步开关,用户可灵活启用或禁用此功能
  • Web界面 - 更新配置界面以支持剪贴板同步选项,并提供多语言本地化支持

Walkthrough

为 Windows 引入系统剪贴板后端并将其接入控制流:新增剪贴板读写实现、平台能力位、配置开关与前端选项;在流层实现剪贴板分块传输与重组、回显抑制及主机→客户端广播;添加相关单元测试与构建配置调整。

Changes

Cohort / File(s) Summary
CMake 构建配置
cmake/compile_definitions/windows.cmake
clipboard.hclipboard.cpp 加入 Windows 平台目标列表,修正末尾换行。
配置定义
src/config.h, src/config.cpp
新增 input.clipboard_sync 布尔配置(默认 true);在 apply_config 中解析该项;调整 input.amf_draw_mouse_cursor 默认为 false
平台能力声明
src/platform/common.h
platf::platform_caps 中新增剪贴板能力标志:clipboard_text (0x04) 与 clipboard_image (0x08)。
Windows 剪贴板实现
src/platform/windows/clipboard.h, src/platform/windows/clipboard.cpp
新增 Windows 剪贴板后端:定义 item_t、4MB 图像限制、读/写 API、PNG↔DIB 转换、UTF-8/UTF-16 文本处理、FNV-1a 内容哈希、OpenClipboard 重试与 RAII 保护,并在失败路径返回可选错误原因。
平台能力集成
src/platform/windows/input.cpp
clipboard::supported_capabilities() 并入 Windows 平台能力位掩码的返回值。
流控制协议
src/stream.cpp
新增 IDX_CLIPBOARD/控制包类型,控制广播线程处理,剪贴板会话状态(绑定/重组/分块/回显抑制)、加密 control 消息的编码/分片/重组逻辑,以及主机↔客户端 的剪贴板发送与接收处理。
Web UI 配置
src_assets/.../composables/useConfig.js, src_assets/.../configs/tabs/Inputs.vue
在 Input 选项卡加入 Windows 专属 clipboard_sync 控件(包含分隔线、标签与描述),并在默认选项中初始化该项。
本地化字符串
src_assets/.../public/assets/locale/en.json, src_assets/.../public/assets/locale/zh.json
新增 clipboard_syncclipboard_sync_desc 本地化文案,说明双向剪贴板同步及支持项(文本与单张图像)。
单元测试
tests/unit/test_stream.cpp
新增剪贴板传输验证相关单元测试与辅助声明(总长/分块次序/接收长度校验等)。

Sequence Diagram(s)

sequenceDiagram
    participant Client as Moonlight 客户端
    participant Stream as 控制流
    participant WinClip as Windows 剪贴板
    participant Enc as 加密层

    Client->>Stream: 绑定并发送 ITEM_START
    Client->>Stream: 发送 ITEM_CHUNK...ITEM_END
    Stream->>Stream: 校验分块并重组
    Stream->>WinClip: 写入完整 item(文本或图像)

    WinClip->>Stream: 主机剪贴板变化(轮询/事件)
    Stream->>WinClip: 读取当前 item(类型/数据/哈希)
    Stream->>Stream: 构建 ITEM_START/ITEM_CHUNK/ITEM_END
    Stream->>Enc: 对控制负载加密
    Enc-->>Stream: 返回加密控制包
    Stream-->>Client: 发送分块控制包
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.77% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed 标题准确总结了 PR 的核心变更:实现了双向的文本和图像剪贴板同步功能。
Description check ✅ Passed PR 描述详细阐述了背景、修改内容、预期行为和兼容性考虑,与变更集内容高度相关。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/stream.cpp (1)

2180-2252: ⚠️ Potential issue | 🔴 Critical

#ifdef _WIN32 条件块移出 KITTY_WHILE_LOOP 宏实参。

Line 2240 的 #ifdef 位于从 Line 2180 开始的宏调用参数块内,这违反了预处理器的标准规则。Cppcheck 在此处报 invalid to use a preprocessor directive as macro parameter 是正确的。建议在宏外定义平台条件 helper,然后在循环体内只调用 helper。

建议修复方向
+#ifdef _WIN32
+    auto maybe_send_clipboard_update = [&](session_t *session) {
+      if (config::input.clipboard_sync &&
+          session->control.clipboard.bound &&
+          platf::clipboard::is_backend_available()) {
+        const auto host_sequence = platf::clipboard::current_sequence_number();
+        if (host_sequence != 0 &&
+            host_sequence != session->control.clipboard.last_host_sequence) {
+          if (send_host_clipboard_snapshot(session, 0, true) != 0) {
+            BOOST_LOG(warning) << "Failed to send clipboard change update to client " << session->client_name;
+          }
+        }
+      }
+    };
+#else
+    auto maybe_send_clipboard_update = [](session_t *) {};
+#endif
+
     while (!shutdown_event->peek() && !broadcast_shutdown_event->peek()) {
@@
-#ifdef _WIN32
-            if (config::input.clipboard_sync &&
-                session->control.clipboard.bound &&
-                platf::clipboard::is_backend_available()) {
-              const auto host_sequence = platf::clipboard::current_sequence_number();
-              if (host_sequence != 0 &&
-                  host_sequence != session->control.clipboard.last_host_sequence) {
-                if (send_host_clipboard_snapshot(session, 0, true) != 0) {
-                  BOOST_LOG(warning) << "Failed to send clipboard change update to client " << session->client_name;
-                }
-              }
-            }
-#endif
+            maybe_send_clipboard_update(session);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/stream.cpp` around lines 2180 - 2252, The KITTY_WHILE_LOOP invocation
currently contains a platform-specific `#ifdef` _WIN32 block as part of its macro
argument (around the clipboard sync code using config::input.clipboard_sync,
session->control.clipboard and send_host_clipboard_snapshot), which triggers the
preprocessor/macro-parameter error; move the entire Windows-only clipboard logic
out of the macro argument by creating a helper function (e.g.,
handle_clipboard_sync_for_session or similar) implemented under `#ifdef` _WIN32
and call that helper from inside the KITTY_WHILE_LOOP body unconditionally (or
guarded by a runtime/platform check), ensuring the macro argument no longer
contains preprocessor directives while preserving the same checks:
config::input.clipboard_sync, session->control.clipboard.bound,
platf::clipboard::is_backend_available(),
platf::clipboard::current_sequence_number(),
session->control.clipboard.last_host_sequence and send_host_clipboard_snapshot
call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src_assets/common/assets/web/composables/useConfig.js`:
- Line 45: Update the default for the clipboard_sync setting from enabled to off
by changing the clipboard_sync entry in useConfig.js from 'enabled' to
'disabled' (or false) so the UI requires an explicit opt‑in; also align the
backend C++ config default for the same clipboard_sync setting and update the
localization strings that describe clipboard sync to indicate it is off by
default and must be explicitly enabled by the user.

In `@src/platform/windows/clipboard.cpp`:
- Around line 251-330: decode_dib_to_png currently trusts header fields and
allocates rgba without validating decoded dimensions/stride, which can lead to
integer overflow or decompression bombs; update decode_dib_to_png to validate
width and height are positive and below a sensible max (use the same
image_size_limit or a safe pixel-count cap), check that stride computation
((width * bit_count + 31) / 32) * 4 cannot overflow size_t, ensure pixel_offset
+ stride*height <= total_size using checked arithmetic, and only allocate the
rgba vector after those checks (and fail early if any multiplication would
overflow or exceed the pixel-size limit) so width/height/stride/pixel_bytes are
bounded before calling encode_png_from_rgba.
- Around line 596-625: The current write_item implementation empties the system
clipboard (EmptyClipboard()) before validating/creating the payload, which can
wipe the user's clipboard if payload creation
(write_png_image/write_unicode_text) or SetClipboardData fails; refactor
write_item so it first validates item.type and prepares the HGLOBAL/payload
(delegate to write_unicode_text or write_png_image but have them return an
allocated HGLOBAL or buffer on success), only then call clipboard_guard_t/
OpenClipboard(), EmptyClipboard(), and SetClipboardData(), and finally close;
ensure you do not call EmptyClipboard() unless payload creation succeeded, free
any allocated HGLOBAL on error, and keep LI_CLIPBOARD_ITEM_TYPE_NONE handling
unchanged.

In `@src/platform/windows/clipboard.h`:
- Around line 7-12: The header uses std::size_t for image_size_limit in the
platf::clipboard namespace but only includes <cstdint>, so explicitly add an
include for <cstddef> to this header; update the top-of-file includes to include
<cstddef> (so image_size_limit and any other uses of std::size_t are correctly
and clearly declared).

In `@src/stream.cpp`:
- Around line 733-743: The function clipboard_transfer_length_valid currently
allows unbounded lengths for LI_CLIPBOARD_ITEM_TYPE_TEXT and returns true for
unknown types; change it to enforce a protocol text size limit (add or use a
constant like LI_CLIPBOARD_MAX_TEXT_SIZE) and validate total_length <=
LI_CLIPBOARD_MAX_TEXT_SIZE in the LI_CLIPBOARD_ITEM_TYPE_TEXT case, and change
the default case to return false so unknown/unsupported item types are rejected;
keep existing checks for LI_CLIPBOARD_ITEM_TYPE_NONE and
LI_CLIPBOARD_ITEM_TYPE_IMAGE (using LI_CLIPBOARD_MAX_IMAGE_SIZE) and update any
related comments/tests accordingly.
- Around line 1740-1774: The code currently updates
session->control.clipboard.received_length using std::max which allows
missing/overlapping chunks to appear as "complete"; change the logic in the
LI_CLIPBOARD_MSG_ITEM_* handling to require strictly sequential chunks (i.e.,
before copying, check that chunk_offset ==
session->control.clipboard.received_length) and on mismatch log a warning and
call reset_clipboard_transfer(session); when valid, copy the payload and
increment received_length by chunk_length (received_length += chunk_length)
instead of using std::max; keep the existing ITEM_END checks but this sequential
enforcement ensures ITEM_END only succeeds for fully received, in-order
transfers (refer to chunk_offset, chunk_length,
session->control.clipboard.received_length, reset_clipboard_transfer, and
LI_CLIPBOARD_MSG_ITEM_END).

---

Outside diff comments:
In `@src/stream.cpp`:
- Around line 2180-2252: The KITTY_WHILE_LOOP invocation currently contains a
platform-specific `#ifdef` _WIN32 block as part of its macro argument (around the
clipboard sync code using config::input.clipboard_sync,
session->control.clipboard and send_host_clipboard_snapshot), which triggers the
preprocessor/macro-parameter error; move the entire Windows-only clipboard logic
out of the macro argument by creating a helper function (e.g.,
handle_clipboard_sync_for_session or similar) implemented under `#ifdef` _WIN32
and call that helper from inside the KITTY_WHILE_LOOP body unconditionally (or
guarded by a runtime/platform check), ensuring the macro argument no longer
contains preprocessor directives while preserving the same checks:
config::input.clipboard_sync, session->control.clipboard.bound,
platf::clipboard::is_backend_available(),
platf::clipboard::current_sequence_number(),
session->control.clipboard.last_host_sequence and send_host_clipboard_snapshot
call.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1449e244-9b12-4bf5-9569-9e933a0db50f

📥 Commits

Reviewing files that changed from the base of the PR and between 6083d69 and 5e9036e.

⛔ Files ignored due to path filters (1)
  • third-party/moonlight-common-c is excluded by !third-party/**
📒 Files selected for processing (12)
  • cmake/compile_definitions/windows.cmake
  • src/config.cpp
  • src/config.h
  • src/platform/common.h
  • src/platform/windows/clipboard.cpp
  • src/platform/windows/clipboard.h
  • src/platform/windows/input.cpp
  • src/stream.cpp
  • src_assets/common/assets/web/composables/useConfig.js
  • src_assets/common/assets/web/configs/tabs/Inputs.vue
  • src_assets/common/assets/web/public/assets/locale/en.json
  • src_assets/common/assets/web/public/assets/locale/zh.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Windows
  • GitHub Check: Agent
🧰 Additional context used
📓 Path-based instructions (4)
src_assets/**/*.{vue,js,html}

⚙️ CodeRabbit configuration file

src_assets/**/*.{vue,js,html}: 基于 Vue.js 的 Web 配置面板。审查 XSS/CSRF 安全性、 组件设计、状态管理和可访问性。

Files:

  • src_assets/common/assets/web/configs/tabs/Inputs.vue
  • src_assets/common/assets/web/composables/useConfig.js
src/**/*.{cpp,c,h}

⚙️ CodeRabbit configuration file

src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码,自托管游戏串流服务器。审查要点:内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。

Files:

  • src/config.h
  • src/platform/windows/input.cpp
  • src/platform/common.h
  • src/config.cpp
  • src/platform/windows/clipboard.h
  • src/platform/windows/clipboard.cpp
  • src/stream.cpp
cmake/**

⚙️ CodeRabbit configuration file

cmake/**: CMake 构建系统文件。审查跨平台兼容性、现代 CMake 实践。

Files:

  • cmake/compile_definitions/windows.cmake
src/platform/**

⚙️ CodeRabbit configuration file

src/platform/**: 平台抽象层代码(Windows/Linux/macOS)。确保各平台实现一致, 注意 Windows API 调用的错误处理和资源释放。

Files:

  • src/platform/windows/input.cpp
  • src/platform/common.h
  • src/platform/windows/clipboard.h
  • src/platform/windows/clipboard.cpp
🪛 Clang (14.0.6)
src/platform/windows/clipboard.h

[error] 7-7: 'cstdint' file not found

(clang-diagnostic-error)

src/platform/windows/clipboard.cpp

[error] 7-7: 'windows.h' file not found

(clang-diagnostic-error)

🪛 Cppcheck (2.20.0)
src/stream.cpp

[error] 2240-2240: failed to expand 'KITTY_WHILE_LOOP', it is invalid to use a preprocessor directive as macro parameter

(syntaxError)

🔇 Additional comments (3)
src/platform/common.h (1)

299-300: 修正注释中对协议常量的错误引用。

注释声称 clipboard_text = 0x04clipboard_image = 0x08 必须与 Limelight-internal.h 的 SS_FF_* 常量匹配,但这些常量在标准 moonlight-common-c 协议中不存在。剪贴板同步是 Sunshine 的专有协议扩展,而非 GameStream 标准功能。建议将注释改为:

// These are Sunshine-specific capability flags for clipboard sync (not part of standard GameStream)

或者删除对 Limelight-internal.h 的错误引用,仅保留说明这是协议扩展的注释。

src/config.cpp (2)

548-548: 剪贴板配置接入一致。

默认值和 apply_config() 解析键都使用 clipboard_sync,和 Web/UI 配置键保持一致。

Also applies to: 1375-1375


550-550: 此注释基于错误的假设,应当撤销。

amf_draw_mouse_cursor 字段及其默认值 false 是本 PR 新增的功能,而非对既有默认值的修改。检查提交历史可看出:

  • 该字段在 src/config.h 中新增(git show 输出中的 + 表示新增)
  • 对应的默认值在 src/config.cpp 中新增
  • src/platform/windows/display_vram.cpp 文件本身也是该 PR 的新增
  • 不存在需要保持向后兼容的"旧默认行为"

这是 AMF 捕获功能的直接组成部分,与剪贴板同步的 PR 目标完全一致。

mouse: 'enabled',
high_resolution_scrolling: 'enabled',
native_pen_touch: 'enabled',
clipboard_sync: 'enabled',
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

将剪贴板同步改为显式选择启用。

clipboard_sync 默认开启会让升级后的用户在兼容客户端会话中自动共享主机剪贴板;剪贴板常包含密码、令牌、邮箱或图片等敏感内容。建议默认关闭,并同步调整 src/config.cpp 的后端默认值和本地化文案。

建议调整默认值
-      clipboard_sync: 'enabled',
+      clipboard_sync: 'disabled',
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
clipboard_sync: 'enabled',
clipboard_sync: 'disabled',
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src_assets/common/assets/web/composables/useConfig.js` at line 45, Update the
default for the clipboard_sync setting from enabled to off by changing the
clipboard_sync entry in useConfig.js from 'enabled' to 'disabled' (or false) so
the UI requires an explicit opt‑in; also align the backend C++ config default
for the same clipboard_sync setting and update the localization strings that
describe clipboard sync to indicate it is off by default and must be explicitly
enabled by the user.

Comment thread src/platform/windows/clipboard.cpp
Comment thread src/platform/windows/clipboard.cpp
Comment thread src/platform/windows/clipboard.h
Comment thread src/stream.cpp
Comment thread src/stream.cpp
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a Sunshine protocol extension to support bidirectional clipboard synchronization (text + single image) with compatible Moonlight clients, including a Windows clipboard backend and a Web UI toggle to enable/disable the feature.

Changes:

  • Extend src/stream.cpp control-stream handling with clipboard bind/unbind, snapshot, and chunked item transfer.
  • Add a Windows clipboard backend (read/write + sequence tracking) and advertise clipboard capabilities to clients.
  • Add clipboard_sync configuration (default enabled) and expose it in the Web UI with EN/ZH strings.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src_assets/common/assets/web/public/assets/locale/zh.json Adds ZH strings for clipboard sync toggle and description.
src_assets/common/assets/web/public/assets/locale/en.json Adds EN strings for clipboard sync toggle and description.
src_assets/common/assets/web/configs/tabs/Inputs.vue Adds Windows-only clipboard sync switch in Inputs tab.
src_assets/common/assets/web/composables/useConfig.js Adds clipboard_sync default option for the Web UI config model.
src/stream.cpp Implements clipboard control message send/receive and host-side polling for clipboard changes.
src/platform/windows/input.cpp Includes clipboard backend and adds advertised clipboard capability flags.
src/platform/windows/clipboard.h Declares Windows clipboard backend API and item representation.
src/platform/windows/clipboard.cpp Implements Windows clipboard read/write and PNG/DIB handling.
src/platform/common.h Adds new platform capability bits for clipboard text/image sync.
src/config.h Adds clipboard_sync to input config struct.
src/config.cpp Sets default clipboard_sync to enabled and wires config parsing.
cmake/compile_definitions/windows.cmake Adds Windows clipboard backend sources to the build.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/stream.cpp Outdated
session->control.clipboard.received_length = 0;
session->control.clipboard.mime_type.clear();
session->control.clipboard.name.clear();
session->control.clipboard.data.clear();
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reset_clipboard_transfer() clears session->control.clipboard.data but doesn’t release capacity, so a single large clipboard transfer can permanently inflate per-session memory usage even after reset. Consider swapping with an empty vector or calling shrink_to_fit() (if acceptable) when resetting to avoid retaining large allocations.

Suggested change
session->control.clipboard.data.clear();
decltype(session->control.clipboard.data)().swap(session->control.clipboard.data);

Copilot uses AI. Check for mistakes.
Comment on lines +274 to +299
const int width = info.biWidth;
const int height = info.biHeight > 0 ? info.biHeight : -info.biHeight;
const bool top_down = info.biHeight < 0;
const int bit_count = info.biBitCount;
const DWORD compression = info.biCompression;

if (!((bit_count == 32 && (compression == BI_RGB || compression == BI_BITFIELDS)) ||
(bit_count == 24 && compression == BI_RGB))) {
GlobalUnlock(dib_handle);
return false;
}

std::size_t pixel_offset = info.biSize;
if (compression == BI_BITFIELDS) {
pixel_offset += 3 * sizeof(DWORD);
}

const auto stride = static_cast<std::size_t>(((width * bit_count + 31) / 32) * 4);
if (pixel_offset + stride * static_cast<std::size_t>(height) > total_size) {
GlobalUnlock(dib_handle);
return false;
}

const auto *pixels = reinterpret_cast<const std::uint8_t *>(header) + pixel_offset;
std::vector<std::uint8_t> rgba(static_cast<std::size_t>(width) * static_cast<std::size_t>(height) * 4);
bool has_non_zero_alpha = false;
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decode_dib_to_png() allocates an RGBA buffer sized widthheight4 based on clipboard-provided BITMAPINFOHEADER fields without any upper bounds or overflow checks. Malformed/hostile clipboard data can cause huge allocations or integer overflow. Add dimension caps and validate that widthheight4 fits in size_t and stays within a configured limit before allocating.

Copilot uses AI. Check for mistakes.
Comment on lines +461 to +487
int width = 0;
int height = 0;
int components = 0;
unsigned char *rgba =
stbi_load_from_memory(item.data.data(),
static_cast<int>(item.data.size()),
&width,
&height,
&components,
4);
if (rgba == nullptr || width <= 0 || height <= 0) {
if (reason) {
*reason = "Failed to decode PNG clipboard payload";
}
if (rgba != nullptr) {
stbi_image_free(rgba);
}
return false;
}

const std::size_t pixel_bytes =
static_cast<std::size_t>(width) * static_cast<std::size_t>(height) * 4;
const std::size_t dib_size = sizeof(BITMAPV5HEADER) + pixel_bytes;
HGLOBAL mem = GlobalAlloc(GMEM_MOVEABLE, dib_size);
if (mem == nullptr) {
stbi_image_free(rgba);
if (reason) {
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

write_png_image() trusts decoded PNG width/height when computing pixel_bytes and dib_size. widthheight4 can overflow size_t (or just be enormous), leading to undersized allocations and out-of-bounds writes in the pixel copy loop, or memory exhaustion. Add strict max dimension checks (and overflow-safe multiplication) before allocating/looping, and fail the write if the image exceeds limits.

Copilot uses AI. Check for mistakes.
Comment thread src/stream.cpp
Comment on lines +1576 to +1585
server->map(packetTypes[IDX_CLIPBOARD], [&, reset_clipboard_transfer, read_u16_le, read_u32_le, read_u64_le](session_t *session, const std::string_view &payload) {
BOOST_LOG(debug) << "type [IDX_CLIPBOARD]"sv;

#ifdef _WIN32
if (!config::input.clipboard_sync || !platf::clipboard::is_backend_available()) {
session->control.clipboard.bound = false;
reset_clipboard_transfer(session);
BOOST_LOG(debug) << "Ignoring clipboard control packet because clipboard sync is disabled"sv;
return;
}
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new clipboard control message parsing/sending logic is substantial but there are currently unit tests for src/stream.cpp (e.g., tests/unit/test_stream.cpp). Consider adding tests that cover clipboard ITEM_START/CHUNK/END parsing (bounds checks, unknown item types, max sizes) to prevent regressions and to validate the new safety constraints.

Copilot uses AI. Check for mistakes.
Comment thread src/stream.cpp
Comment on lines +733 to +743
bool
clipboard_transfer_length_valid(uint8_t item_type, std::uint32_t total_length) {
switch (item_type) {
case LI_CLIPBOARD_ITEM_TYPE_NONE:
return total_length == 0;
case LI_CLIPBOARD_ITEM_TYPE_IMAGE:
return total_length <= LI_CLIPBOARD_MAX_IMAGE_SIZE;
case LI_CLIPBOARD_ITEM_TYPE_TEXT:
default:
return true;
}
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clipboard_transfer_length_valid() returns true for LI_CLIPBOARD_ITEM_TYPE_TEXT and for any unknown item_type (default case). This allows a client to advertise an arbitrarily large total_length (or unknown type) and force session->control.clipboard.data.assign(total_length, …) to allocate unbounded memory, which is a DoS risk. Consider explicitly rejecting unknown item types and enforcing a reasonable max size for text (and/or a global max item size) before allocating.

Copilot uses AI. Check for mistakes.
Address the substantive review feedback for the new clipboard sync path without changing the product default.

This tightens transfer validation, enforces sequential chunk delivery, adds Windows image size and overflow checks, releases reset transfer buffers, and trims the Web UI clipboard copy to match the current product wording.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
src/stream.cpp (1)

1826-1849: 可移动 clipboard 缓冲,避免一次额外的大块拷贝。

item.data = session->control.clipboard.data; 在 1MB 文本/多 MB 图像场景下会产生一次不必要的深拷贝,之后紧跟的 reset_clipboard_transfer(session) 又会将原 data 清空。由于原 vector 即将被重置,可直接移动,避免在控制线程上持有关键锁期间做大块内存拷贝。

♻️ 建议修改
           platf::clipboard::item_t item;
           item.type = session->control.clipboard.item_type;
-          item.data = session->control.clipboard.data;
-          item.mime_type = session->control.clipboard.mime_type;
-          item.name = session->control.clipboard.name;
+          item.data = std::move(session->control.clipboard.data);
+          item.mime_type = std::move(session->control.clipboard.mime_type);
+          item.name = std::move(session->control.clipboard.name);
           item.content_hash = session->control.clipboard.content_hash;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/stream.cpp` around lines 1826 - 1849, The clipboard data is being
deep-copied into platf::clipboard::item_t via item.data =
session->control.clipboard.data even though reset_clipboard_transfer(session)
immediately clears the source; change the assignment to move the buffer (use
std::move on session->control.clipboard.data) so platf::clipboard::item_t takes
ownership and avoids the large copy, ensuring callers like
platf::clipboard::write_item still work with item.data and that
reset_clipboard_transfer(session) can safely operate on the moved-from source.
src/platform/common.h (1)

293-301: 注释描述范围过窄,与现有标志不一致。

第 293 行的新注释为 "These are Sunshine-specific capability flags for clipboard sync.",但此 namespace platform_caps 同时包含 pen_touch/controller_touch 等与剪贴板无关的 Sunshine 自定义能力标志。建议恢复为描述整个命名空间用途的通用注释,避免误导后续维护者。

♻️ 建议修改
-  // These are Sunshine-specific capability flags for clipboard sync.
+  // These are Sunshine-specific capability flags (not matching Limelight SS_FF_* constants).
   namespace platform_caps {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/platform/common.h` around lines 293 - 301, 注释只提到了剪贴板同步但 namespace
platform_caps 定义了多种 Sunshine
自定义能力标志(caps_t、pen_touch、controller_touch、clipboard_text、clipboard_image),应将注释改为描述整个命名空间用途的通用说明(例如表明这是
Sunshine 平台能力标志集合),以避免误导;在文件中找到 namespace platform_caps
和相关常量(caps_t、pen_touch、controller_touch、clipboard_text、clipboard_image)并替换当前狭义注释为一行或两行的通用注释,说明这些常量表示
Sunshine 的平台能力标志集合。
tests/unit/test_stream.cpp (1)

52-68: 单元测试分支覆盖不完整。

clipboard_transfer_length_valid 有 4 个分支,但只覆盖了 TEXTdefaultLI_CLIPBOARD_ITEM_TYPE_NONE(要求 total_length == 0)和 LI_CLIPBOARD_ITEM_TYPE_IMAGE(上限为 LI_CLIPBOARD_MAX_IMAGE_SIZE)边界未验证。clipboard_transfer_chunk_next_length 也缺少成功到达 total_length 的分片序列用例(例如最后一块恰好填满)。建议补充这些场景,保障回归检测。

♻️ 建议补充测试
 TEST(ClipboardTransferValidationTests, RejectsOversizedTextAndUnknownTypes) {
   constexpr std::uint32_t one_megabyte = 1024U * 1024U;

+  EXPECT_TRUE(stream::clipboard_transfer_length_valid(LI_CLIPBOARD_ITEM_TYPE_NONE, 0));
+  EXPECT_FALSE(stream::clipboard_transfer_length_valid(LI_CLIPBOARD_ITEM_TYPE_NONE, 1));
+  EXPECT_TRUE(stream::clipboard_transfer_length_valid(LI_CLIPBOARD_ITEM_TYPE_IMAGE, LI_CLIPBOARD_MAX_IMAGE_SIZE));
+  EXPECT_FALSE(stream::clipboard_transfer_length_valid(LI_CLIPBOARD_ITEM_TYPE_IMAGE, LI_CLIPBOARD_MAX_IMAGE_SIZE + 1));
   EXPECT_TRUE(stream::clipboard_transfer_length_valid(LI_CLIPBOARD_ITEM_TYPE_TEXT, one_megabyte));
   EXPECT_FALSE(stream::clipboard_transfer_length_valid(LI_CLIPBOARD_ITEM_TYPE_TEXT, one_megabyte + 1));
   EXPECT_FALSE(stream::clipboard_transfer_length_valid(0xFE, 16));
 }

 TEST(ClipboardTransferValidationTests, AcceptsSequentialChunksOnly) {
   std::uint32_t next_received_length = 0;

   EXPECT_TRUE(stream::clipboard_transfer_chunk_next_length(0, 10, 0, 4, next_received_length));
   EXPECT_EQ(next_received_length, 4U);
+  EXPECT_TRUE(stream::clipboard_transfer_chunk_next_length(4, 10, 4, 6, next_received_length));
+  EXPECT_EQ(next_received_length, 10U);
+  EXPECT_TRUE(stream::clipboard_transfer_chunk_next_length(10, 10, 10, 0, next_received_length));
+  EXPECT_EQ(next_received_length, 10U);

   EXPECT_FALSE(stream::clipboard_transfer_chunk_next_length(4, 10, 6, 2, next_received_length));
   EXPECT_FALSE(stream::clipboard_transfer_chunk_next_length(4, 10, 2, 2, next_received_length));
   EXPECT_FALSE(stream::clipboard_transfer_chunk_next_length(8, 10, 8, 3, next_received_length));
 }

As per coding guidelines, tests/**: 测试文件。验证测试覆盖率、边界情况和断言正确性。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/test_stream.cpp` around lines 52 - 68, Add unit tests to cover the
missing branches in clipboard_transfer_length_valid and the final-chunk success
case in clipboard_transfer_chunk_next_length: specifically, add assertions for
LI_CLIPBOARD_ITEM_TYPE_NONE that require total_length == 0 (valid for 0, invalid
for >0), and for LI_CLIPBOARD_ITEM_TYPE_IMAGE at the LI_CLIPBOARD_MAX_IMAGE_SIZE
boundary (valid at max, invalid at max+1); also add chunk-sequence tests for
clipboard_transfer_chunk_next_length that exercise a valid sequence reaching
total_length exactly (e.g., successive chunks that sum to total_length and the
final call returns true and sets next_received_length to the remaining bytes)
and a case where the final chunk exactly fills the remaining bytes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/platform/windows/clipboard.cpp`:
- Around line 411-438: The read_text_item function reads CF_UNICODETEXT via
GlobalLock but constructs std::wstring(text) from the pointer without bounding
by GlobalSize, which can read past the allocation; fix by using
GlobalSize(handle) to compute the buffer length in WCHARs (or
bytes/sizeof(wchar_t>), clamp to a safe length, and construct the std::wstring
from the pointer+length rather than relying on a NUL terminator; ensure you
still call GlobalUnlock(handle) on all failure paths and preserve existing
error/reason handling.

In `@src/stream.cpp`:
- Around line 1370-1408: The send_clipboard_payload function currently truncates
clipboard_payload.size() into header->payloadLength via static_cast<uint16_t>,
which can silently corrupt ITEM_START payloads; add a pre-check in
send_clipboard_payload to reject payloads larger than
std::numeric_limits<std::uint16_t>::max() (or a defined max like
LI_CLIPBOARD_MAX_CHUNK_SIZE if appropriate), log an error via BOOST_LOG(error)
indicating the payload is too large, and return -1 before building
plaintext/control_header_v2 so payloadLength cannot overflow; keep the existing
behavior for valid sizes and use the same symbols: send_clipboard_payload,
control_header_v2, header->payloadLength and IDX_CLIPBOARD.

---

Nitpick comments:
In `@src/platform/common.h`:
- Around line 293-301: 注释只提到了剪贴板同步但 namespace platform_caps 定义了多种 Sunshine
自定义能力标志(caps_t、pen_touch、controller_touch、clipboard_text、clipboard_image),应将注释改为描述整个命名空间用途的通用说明(例如表明这是
Sunshine 平台能力标志集合),以避免误导;在文件中找到 namespace platform_caps
和相关常量(caps_t、pen_touch、controller_touch、clipboard_text、clipboard_image)并替换当前狭义注释为一行或两行的通用注释,说明这些常量表示
Sunshine 的平台能力标志集合。

In `@src/stream.cpp`:
- Around line 1826-1849: The clipboard data is being deep-copied into
platf::clipboard::item_t via item.data = session->control.clipboard.data even
though reset_clipboard_transfer(session) immediately clears the source; change
the assignment to move the buffer (use std::move on
session->control.clipboard.data) so platf::clipboard::item_t takes ownership and
avoids the large copy, ensuring callers like platf::clipboard::write_item still
work with item.data and that reset_clipboard_transfer(session) can safely
operate on the moved-from source.

In `@tests/unit/test_stream.cpp`:
- Around line 52-68: Add unit tests to cover the missing branches in
clipboard_transfer_length_valid and the final-chunk success case in
clipboard_transfer_chunk_next_length: specifically, add assertions for
LI_CLIPBOARD_ITEM_TYPE_NONE that require total_length == 0 (valid for 0, invalid
for >0), and for LI_CLIPBOARD_ITEM_TYPE_IMAGE at the LI_CLIPBOARD_MAX_IMAGE_SIZE
boundary (valid at max, invalid at max+1); also add chunk-sequence tests for
clipboard_transfer_chunk_next_length that exercise a valid sequence reaching
total_length exactly (e.g., successive chunks that sum to total_length and the
final call returns true and sets next_received_length to the remaining bytes)
and a case where the final chunk exactly fills the remaining bytes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8bc87cf4-6169-4f3d-a167-4826bee0f5b3

📥 Commits

Reviewing files that changed from the base of the PR and between 5e9036e and 14b0061.

📒 Files selected for processing (7)
  • src/platform/common.h
  • src/platform/windows/clipboard.cpp
  • src/platform/windows/clipboard.h
  • src/stream.cpp
  • src_assets/common/assets/web/public/assets/locale/en.json
  • src_assets/common/assets/web/public/assets/locale/zh.json
  • tests/unit/test_stream.cpp
✅ Files skipped from review due to trivial changes (2)
  • src_assets/common/assets/web/public/assets/locale/zh.json
  • src_assets/common/assets/web/public/assets/locale/en.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (3)
tests/**

⚙️ CodeRabbit configuration file

tests/**: 测试文件。验证测试覆盖率、边界情况和断言正确性。

Files:

  • tests/unit/test_stream.cpp
src/**/*.{cpp,c,h}

⚙️ CodeRabbit configuration file

src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码,自托管游戏串流服务器。审查要点:内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。

Files:

  • src/platform/common.h
  • src/platform/windows/clipboard.cpp
  • src/stream.cpp
  • src/platform/windows/clipboard.h
src/platform/**

⚙️ CodeRabbit configuration file

src/platform/**: 平台抽象层代码(Windows/Linux/macOS)。确保各平台实现一致, 注意 Windows API 调用的错误处理和资源释放。

Files:

  • src/platform/common.h
  • src/platform/windows/clipboard.cpp
  • src/platform/windows/clipboard.h
🪛 Clang (14.0.6)
src/platform/windows/clipboard.cpp

[error] 7-7: 'windows.h' file not found

(clang-diagnostic-error)

src/platform/windows/clipboard.h

[error] 7-7: 'cstddef' file not found

(clang-diagnostic-error)

🔇 Additional comments (4)
src/platform/windows/clipboard.h (1)

1-37: LGTM。

头文件结构清晰,<cstddef> 已按历史评审意见显式引入;item_t 字段按值初始化,API 以 bool + 可选 reason 返回保持一致,符合平台抽象层约定。

src/stream.cpp (1)

733-763: 对历史严重问题的修复到位。

clipboard_transfer_length_valid 现在对 default 分支返回 false 并为 TEXT 设置 1MB 上限;clipboard_transfer_chunk_next_length 强制 chunk_offset == received_length 并校验 chunk_offset <= total_lengthchunk_length <= total_length - chunk_offset,成功杜绝了之前指出的"最大偏移判定完成"以及未知类型内存耗尽问题。

src/platform/windows/clipboard.cpp (2)

251-408: 历史关键问题已修复。

decoded_image_size_valid / dib_stride_valid / checked_mul / checked_add 组合使得 decode_dib_to_png 在分配 rgba 前完成了宽高、步长、像素缓冲区与头部范围的全链路溢出/上限校验;decode_dib_to_png 也正确处理了 top_down、32 位全零 alpha 归一化与 BI_BITFIELDS 遮罩偏移,消除了解压炸弹风险。


544-650: write_png_image 的延迟 EmptyClipboard 修复到位。

现在的顺序是先解码 PNG、校验解码尺寸、分配并填充 HGLOBAL,只在 SetClipboardData 之前调用 EmptyClipboard(),历史评审提出的"验证失败会清空用户剪贴板"问题已解决;辅助写入 PNG 自定义格式失败时也不会破坏主要的 CF_DIBV5 数据。

Comment thread src/platform/windows/clipboard.cpp
Comment thread src/stream.cpp
Bound CF_UNICODETEXT reads by GlobalSize and reject oversized control payloads before serializing clipboard headers.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
src/platform/windows/clipboard.cpp (1)

453-467: ⚠️ Potential issue | 🔴 Critical

在复制/转发/解码 PNG 前先做尺寸预检。

当前只限制压缩后的 4MB PNG:Line 453 会先把整个 HGLOBAL 复制进内存,Line 462-467 会把未校验解码尺寸的 PNG 发给客户端,Line 563 又会在 decoded_image_size_valid() 前先让 stb 完整解码。建议用 stbi_info_from_memory() 先读取宽高并套用 decoded_image_size_valid(),再复制/转发或调用 stbi_load_from_memory()。这与之前的“先校验解码后尺寸”问题同类。

🛡️ 建议修复方向
+    bool
+    png_decoded_size_valid(const std::vector<std::uint8_t> &png_bytes,
+                           std::size_t &pixel_bytes) {
+      int width = 0;
+      int height = 0;
+      int components = 0;
+      if (png_bytes.empty() ||
+          !stbi_info_from_memory(png_bytes.data(),
+                                 static_cast<int>(png_bytes.size()),
+                                 &width,
+                                 &height,
+                                 &components)) {
+        return false;
+      }
+
+      return decoded_image_size_valid(width, height, 4, pixel_bytes);
+    }
         if (png_bytes.size() > image_size_limit) {
           if (reason) {
             *reason = "PNG clipboard item exceeded image size limit";
           }
           item.type = LI_CLIPBOARD_ITEM_TYPE_NONE;
           return true;
         }
+
+        std::size_t pixel_bytes = 0;
+        if (!png_decoded_size_valid(png_bytes, pixel_bytes)) {
+          if (reason) {
+            *reason = "PNG clipboard item exceeded decoded image size limit";
+          }
+          item.type = LI_CLIPBOARD_ITEM_TYPE_NONE;
+          return true;
+        }
       if (item.data.size() > image_size_limit) {
         if (reason) {
           *reason = "PNG clipboard payload exceeded encoded image size limit";
         }
         return false;
       }
+
+      std::size_t expected_pixel_bytes = 0;
+      if (!png_decoded_size_valid(item.data, expected_pixel_bytes)) {
+        if (reason) {
+          *reason = "PNG clipboard payload exceeded decoded image size limit";
+        }
+        return false;
+      }
 
       int width = 0;
#!/bin/bash
# Verify the bundled stb_image header exposes stbi_info_from_memory for PNG metadata preflight.
fd -i '^stb_image\.h$' --exec rg -n -C2 '\bstbi_info_from_memory\s*\(' {}

As per coding guidelines, src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码,审查要点:内存安全、 线程安全、RAII 资源管理、安全漏洞。

Also applies to: 560-589

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/platform/windows/clipboard.cpp` around lines 453 - 467, 当前代码在把 PNG
HGLOBAL 内容读入 png_bytes 后直接接受并转发,未在解码前做尺寸预检。修改 read_hglobal_bytes → the PNG
handling block (where handle, png_bytes, item, LI_CLIPBOARD_ITEM_TYPE_IMAGE are
used):在设置 item.type 和将 png_bytes 移入 item.data 或在调用 stbi_load_from_memory() 之前,先用
stbi_info_from_memory(png_bytes.data(), png_bytes.size(), &w, &h, &comp) 获取宽高并调用
decoded_image_size_valid(w, h);若校验失败,设置 *reason(若非空)并把 item.type 设为
LI_CLIPBOARD_ITEM_TYPE_NONE 并返回 true;只有通过尺寸校验后才继续保留 png_bytes、设置
item.data、mime_type、content_hash(compute_item_hash)并返回。确保不在未校验尺寸前进行完整解码(stbi_load_from_memory)。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/platform/windows/clipboard.cpp`:
- Around line 428-442: Check GlobalSize(handle)/max_chars before allocating: if
the UTF-16 length in wchar_t (max_chars or measured length) would produce a
UTF-8 payload larger than the allowed protocol payload limit, bail out early and
set item.type = LI_CLIPBOARD_ITEM_TYPE_NONE (or otherwise return) instead of
constructing std::wstring/utf8 and filling item.data; do this check before
calling std::wstring text(wide, length) and before calling
wide_to_utf8/normalize_newlines_to_lf to avoid huge allocations and the
WideCharToMultiByte int-length path. Use the protocol payload max constant (e.g.
MAX_PAYLOAD_SIZE or define CLIPBOARD_PAYLOAD_MAX) and account for worst‑case
UTF‑8 expansion when comparing (e.g. up to 4 bytes per UTF‑16 code unit) so the
check references GlobalSize/ max_chars, the std::wstring constructor,
wide_to_utf8, normalize_newlines_to_lf and item.data assignment locations.
- Around line 331-343: Reject a biHeight equal to the minimum representable LONG
before negating to avoid signed overflow: check if info.biHeight ==
std::numeric_limits<LONG>::min() (or LONG_MIN) and if so call
GlobalUnlock(dib_handle) and return false; then compute height and top_down as
before using info.biHeight > 0 ? info.biHeight : -info.biHeight. Ensure <limits>
is included if using std::numeric_limits. This keeps BITMAPINFOHEADER,
info.biHeight, height, and top_down logic intact while preventing overflow.

---

Duplicate comments:
In `@src/platform/windows/clipboard.cpp`:
- Around line 453-467: 当前代码在把 PNG HGLOBAL 内容读入 png_bytes 后直接接受并转发,未在解码前做尺寸预检。修改
read_hglobal_bytes → the PNG handling block (where handle, png_bytes, item,
LI_CLIPBOARD_ITEM_TYPE_IMAGE are used):在设置 item.type 和将 png_bytes 移入 item.data
或在调用 stbi_load_from_memory() 之前,先用 stbi_info_from_memory(png_bytes.data(),
png_bytes.size(), &w, &h, &comp) 获取宽高并调用 decoded_image_size_valid(w, h);若校验失败,设置
*reason(若非空)并把 item.type 设为 LI_CLIPBOARD_ITEM_TYPE_NONE 并返回 true;只有通过尺寸校验后才继续保留
png_bytes、设置
item.data、mime_type、content_hash(compute_item_hash)并返回。确保不在未校验尺寸前进行完整解码(stbi_load_from_memory)。
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: aa673686-47cd-4b11-929f-41c8177fa592

📥 Commits

Reviewing files that changed from the base of the PR and between 14b0061 and 2b5d3d0.

📒 Files selected for processing (2)
  • src/platform/windows/clipboard.cpp
  • src/stream.cpp
✅ Files skipped from review due to trivial changes (1)
  • src/stream.cpp
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{cpp,c,h}

⚙️ CodeRabbit configuration file

src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码,自托管游戏串流服务器。审查要点:内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。

Files:

  • src/platform/windows/clipboard.cpp
src/platform/**

⚙️ CodeRabbit configuration file

src/platform/**: 平台抽象层代码(Windows/Linux/macOS)。确保各平台实现一致, 注意 Windows API 调用的错误处理和资源释放。

Files:

  • src/platform/windows/clipboard.cpp
🪛 Clang (14.0.6)
src/platform/windows/clipboard.cpp

[error] 7-7: 'windows.h' file not found

(clang-diagnostic-error)

🔇 Additional comments (1)
src/platform/windows/clipboard.cpp (1)

353-359: 移除此注释:代码偏移处理正确,无需修改。

通过 Windows 官方文档和标准 DIB 格式规范验证,当处理 CF_DIB(V5) 格式的 BI_BITFIELDS 压缩时,颜色 mask 在 header 之后是冗余重复的。虽然 BITMAPV5HEADER 在结构体内包含 mask 字段(bV5RedMask 等),但在打包的 DIB 内存布局中,这些 mask 会在 header 后重复出现(在 bmiColors 位置)。因此需要跳过这 12 字节才能到达真实像素数据。

  • BITMAPINFOHEADER (40 字节) + BI_BITFIELDS:header → 12 字节 mask → 像素
  • BITMAPV5HEADER (124 字节) + BI_BITFIELDS:header → 12 字节冗余 mask → 像素

当前代码正确计算了偏移量。建议的修复反而会导致 BITMAPV5HEADER 格式图像读取错误。

			> Likely an incorrect or invalid review comment.

Comment thread src/platform/windows/clipboard.cpp
Comment thread src/platform/windows/clipboard.cpp
Preflight decoded PNG sizes, bound CF_UNICODETEXT handling before conversion, avoid signed-height overflow in DIB parsing, move received clipboard buffers into the Windows backend, and extend the stream validation tests.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/platform/windows/clipboard.cpp (2)

567-611: write_unicode_text 缺少对 item.data.size() 的显式上限校验。

read_text_item 在 479-487 处使用 max_text_clipboard_bytes 限制读取;但写入路径未做对称防御——来自客户端的 item.data 直接进入 normalize_newlines_to_crlfutf8_to_wide,其中 MultiByteToWideChar 的长度参数被强制 static_cast<int>。当上游未校验或校验上限与此处不一致时,会出现 int 截断与大内存分配风险。

建议:

🛡️ 建议修复
     bool
     write_unicode_text(const item_t &item, std::string *reason) {
+      if (item.data.size() > max_text_clipboard_bytes) {
+        if (reason) {
+          *reason = "Text clipboard payload exceeded size limit";
+        }
+        return false;
+      }
       const std::wstring text = normalize_newlines_to_crlf(std::string_view {
         reinterpret_cast<const char *>(item.data.data()),
         item.data.size(),
       });

As per coding guidelines, src/**/*.{cpp,c,h}: 审查要点:内存安全、 线程安全、RAII 资源管理、安全漏洞。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/platform/windows/clipboard.cpp` around lines 567 - 611,
write_unicode_text currently passes item.data.size() directly into
normalize_newlines_to_crlf/utf8_to_wide which eventually static_casts to int for
MultiByteToWideChar; add an explicit bounds check that item.data.size() does not
exceed the configured max_text_clipboard_bytes and also ensure the byte count
fits into an int before conversion (e.g., if (item.data.size() >
max_text_clipboard_bytes || item.data.size() >
static_cast<size_t>(std::numeric_limits<int>::max())) { set reason and return
false; }), perform any necessary truncation or reject the request, and update
the error reason strings accordingly; reference write_unicode_text,
normalize_newlines_to_crlf, utf8_to_wide and the
item.data.size()/max_text_clipboard_bytes symbols when applying the change.

720-723: PNG 备份格式写入失败被静默吞掉。

write_hglobal_bytes(png_format, item.data) 的返回值未检查。尽管主格式 CF_DIBV5 已经写入成功,失败时至少应该通过 reason 提示或打日志,便于诊断客户端粘贴到 PNG-aware 应用时拿不到原始编码的问题。

♻️ 建议修复
       const UINT png_format = RegisterClipboardFormatW(L"PNG");
-      if (png_format != 0) {
-        write_hglobal_bytes(png_format, item.data);
-      }
+      if (png_format != 0 && !write_hglobal_bytes(png_format, item.data)) {
+        // 非致命:主格式 CF_DIBV5 已成功,这里仅记录调试信息。
+        BOOST_LOG(debug) << "Failed to set registered PNG clipboard format (fallback-only)";
+      }

(具体日志宏请替换为本项目实际使用者。)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/platform/windows/clipboard.cpp` around lines 720 - 723, 在写入 PNG 备份格式时未检查
write_hglobal_bytes(png_format, item.data) 返回值,导致写入失败被静默吞掉;在处理
RegisterClipboardFormatW(L"PNG") 并调用 write_hglobal_bytes
时应检查其返回结果,若为失败则通过项目通用的日志/错误通道记录包含 png_format、item.data 相关上下文及具体失败原因(或将 reason
变量设置为描述性错误信息),并保持对主格式 CF_DIBV5 的已有处理不变以便诊断为什么粘贴到 PNG-aware 应用时无法获得原始编码。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/platform/windows/clipboard.cpp`:
- Around line 40-49: The clipboard code currently calls OpenClipboard(nullptr)
in clipboard_guard_t(), which breaks EmptyClipboard()/SetClipboardData() in
service/Session0 scenarios; update the implementation to create or reuse a
message-only hidden window (CreateWindowEx with HWND_MESSAGE) and pass its HWND
to OpenClipboard instead of nullptr, ensuring the window is created once and
destroyed on shutdown; modify clipboard_guard_t (and its retry loop using
clipboard_retry_count and clipboard_retry_delay) to accept or obtain that HWND,
and update write_unicode_text and write_png_image to rely on this HWND-backed
clipboard_opening so EmptyClipboard/SetClipboardData succeed in service
environments.

---

Nitpick comments:
In `@src/platform/windows/clipboard.cpp`:
- Around line 567-611: write_unicode_text currently passes item.data.size()
directly into normalize_newlines_to_crlf/utf8_to_wide which eventually
static_casts to int for MultiByteToWideChar; add an explicit bounds check that
item.data.size() does not exceed the configured max_text_clipboard_bytes and
also ensure the byte count fits into an int before conversion (e.g., if
(item.data.size() > max_text_clipboard_bytes || item.data.size() >
static_cast<size_t>(std::numeric_limits<int>::max())) { set reason and return
false; }), perform any necessary truncation or reject the request, and update
the error reason strings accordingly; reference write_unicode_text,
normalize_newlines_to_crlf, utf8_to_wide and the
item.data.size()/max_text_clipboard_bytes symbols when applying the change.
- Around line 720-723: 在写入 PNG 备份格式时未检查 write_hglobal_bytes(png_format,
item.data) 返回值,导致写入失败被静默吞掉;在处理 RegisterClipboardFormatW(L"PNG") 并调用
write_hglobal_bytes 时应检查其返回结果,若为失败则通过项目通用的日志/错误通道记录包含 png_format、item.data
相关上下文及具体失败原因(或将 reason 变量设置为描述性错误信息),并保持对主格式 CF_DIBV5 的已有处理不变以便诊断为什么粘贴到
PNG-aware 应用时无法获得原始编码。
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 284ebffc-4a7d-439f-84f6-b67777252e81

📥 Commits

Reviewing files that changed from the base of the PR and between 2b5d3d0 and 53c999a.

📒 Files selected for processing (4)
  • src/platform/common.h
  • src/platform/windows/clipboard.cpp
  • src/stream.cpp
  • tests/unit/test_stream.cpp
✅ Files skipped from review due to trivial changes (3)
  • src/platform/common.h
  • src/stream.cpp
  • tests/unit/test_stream.cpp
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{cpp,c,h}

⚙️ CodeRabbit configuration file

src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码,自托管游戏串流服务器。审查要点:内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。

Files:

  • src/platform/windows/clipboard.cpp
src/platform/**

⚙️ CodeRabbit configuration file

src/platform/**: 平台抽象层代码(Windows/Linux/macOS)。确保各平台实现一致, 注意 Windows API 调用的错误处理和资源释放。

Files:

  • src/platform/windows/clipboard.cpp
🪛 Clang (14.0.6)
src/platform/windows/clipboard.cpp

[error] 7-7: 'windows.h' file not found

(clang-diagnostic-error)

Comment thread src/platform/windows/clipboard.cpp Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
src/platform/windows/clipboard.cpp (2)

793-810: 建议显式 #include "src/config.h"

Line 799 直接引用 config::input.clipboard_sync,但本文件的包含列表(line 23-28)没有显式引入 src/config.h,目前编译通过依赖 src/platform/common.h 等头文件的传递包含。一旦上游头整理/瘦身(很常见的重构),此处会在未触及本文件时突然断编。

 `#include` "clipboard.h"

 `#include` "src/platform/common.h"
+#include "src/config.h"
 `#include` "src/logging.h"

As per coding guidelines, src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码,审查要点:内存安全、 线程安全、RAII 资源管理、安全漏洞。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/platform/windows/clipboard.cpp` around lines 793 - 810, The code reads
config::input.clipboard_sync inside supported_capabilities() but the file relies
on transitive includes; explicitly add a direct include for the config header to
avoid fragile compilation: add an `#include` "src/config.h" to the top of
src/platform/windows/clipboard.cpp ( alongside the other includes ) so
supported_capabilities() can reliably reference config::input.clipboard_sync and
prevent breakage if upstream headers are refactored.

430-487: 局限:忽略 BI_BITFIELDS 实际掩码并对 32bpp 全零 alpha 强制覆盖。

两处是已知限制,当前实现对“常见剪贴板源(BGRA + 忽略 alpha)”是合理折衷,但值得确认:

  1. Line 430-434 接受 BI_BITFIELDS,但后续通道交换(line 468-471)固定按 BGRA 顺序读取,未使用 bV5RedMask / bV5GreenMask / bV5BlueMask / bV5AlphaMaskBITMAPINFOHEADER 之后的 3 个掩码 DWORD。若源使用非标准通道顺序(例如 RGBA 而非 BGRA),输出颜色会错位。若仅打算兼容 Windows 原生 BGRA 剪贴板,建议至少对非标准掩码做拒绝而不是静默出错。
  2. Lines 483-487 在 32bpp 所有像素 alpha 均为 0 时统一覆盖为 0xFF。这对“旧应用把 alpha 当填充字节忽略”这种常见情况是必要的,但会错误地把“整图完全透明”的合法图片变成不透明。建议在日志里记录一下该分支的命中,便于后续出问题时定位。

不阻塞合入;仅留作后续改进。

As per coding guidelines, src/platform/**: 平台抽象层代码(Windows/Linux/macOS)。确保各平台实现一致, 注意 Windows API 调用的错误处理和资源释放。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/platform/windows/clipboard.cpp` around lines 430 - 487, The code accepts
BI_BITFIELDS but then unconditionally treats 32bpp pixels as BGRA when
converting (see bit_count, compression and the per-pixel loop reading src[x*4 +
...]); update the BI_BITFIELDS branch to read the three/four DWORD masks
(bV5RedMask/bV5GreenMask/bV5BlueMask[/bV5AlphaMask] or the three DWORDs
following BITMAPINFOHEADER) and either (a) validate they match the expected BGRA
masks and proceed, or (b) reject/return false for non-standard masks to avoid
silent color channel swaps; additionally, when forcing alpha to 0xFF for a 32bpp
image with has_non_zero_alpha==false, emit a diagnostic log entry (use the
existing logging mechanism) before overwriting rgba bytes so it's traceable
(refer to has_non_zero_alpha, rgba and the final masking loop).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/platform/windows/clipboard.cpp`:
- Around line 436-455: The BI_BITFIELDS branch unconditionally adds
3*sizeof(DWORD) to pixel_offset causing BITMAPV4HEADER/BITMAPV5HEADER (biSize
108/124) to be mis-parsed; update the condition that adds the mask DWORDs so the
extra 3*DWORD skip is only applied when info.biSize equals the BITMAPINFOHEADER
size (i.e. sizeof(BITMAPINFOHEADER) / biSize == 40) and leave V4/V5 headers
alone; ensure you still call GlobalUnlock(dib_handle) and return false on
checked_add failure and keep the subsequent validations (dib_stride_valid,
checked_mul, checked_add, decoded_image_size_valid) unchanged.
- Around line 852-870: The switch handling LI_CLIPBOARD_ITEM_TYPE_NONE currently
calls EmptyClipboard() and allows clients to send NONE, which bypasses
clipboard_transfer_length_valid() and breaks echo suppression; to fix this, add
a validation in the client-transfer handling (the code path that processes
CLIPBOARD_MSG_CLIENT_CLIPBOARD_DATA_START / uses
clipboard_transfer_length_valid()) to reject LI_CLIPBOARD_ITEM_TYPE_NONE unless
the SNAPSHOT flag is set, and also change the echo-suppression assignment (the
line setting suppress_next_host_echo from item.content_hash) to guard against
NONE—e.g., only set suppress_next_host_echo when item.type !=
LI_CLIPBOARD_ITEM_TYPE_NONE && item.content_hash != 0—so NONE does not disable
suppression; ensure the switch (write_unicode_text / write_png_image /
EmptyClipboard) behavior remains unchanged except for the added validation
upstream.

---

Nitpick comments:
In `@src/platform/windows/clipboard.cpp`:
- Around line 793-810: The code reads config::input.clipboard_sync inside
supported_capabilities() but the file relies on transitive includes; explicitly
add a direct include for the config header to avoid fragile compilation: add an
`#include` "src/config.h" to the top of src/platform/windows/clipboard.cpp (
alongside the other includes ) so supported_capabilities() can reliably
reference config::input.clipboard_sync and prevent breakage if upstream headers
are refactored.
- Around line 430-487: The code accepts BI_BITFIELDS but then unconditionally
treats 32bpp pixels as BGRA when converting (see bit_count, compression and the
per-pixel loop reading src[x*4 + ...]); update the BI_BITFIELDS branch to read
the three/four DWORD masks (bV5RedMask/bV5GreenMask/bV5BlueMask[/bV5AlphaMask]
or the three DWORDs following BITMAPINFOHEADER) and either (a) validate they
match the expected BGRA masks and proceed, or (b) reject/return false for
non-standard masks to avoid silent color channel swaps; additionally, when
forcing alpha to 0xFF for a 32bpp image with has_non_zero_alpha==false, emit a
diagnostic log entry (use the existing logging mechanism) before overwriting
rgba bytes so it's traceable (refer to has_non_zero_alpha, rgba and the final
masking loop).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 39b03076-b693-42fb-8d61-b85e84f83061

📥 Commits

Reviewing files that changed from the base of the PR and between 53c999a and 452dfa9.

📒 Files selected for processing (1)
  • src/platform/windows/clipboard.cpp
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{cpp,c,h}

⚙️ CodeRabbit configuration file

src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码,自托管游戏串流服务器。审查要点:内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。

Files:

  • src/platform/windows/clipboard.cpp
src/platform/**

⚙️ CodeRabbit configuration file

src/platform/**: 平台抽象层代码(Windows/Linux/macOS)。确保各平台实现一致, 注意 Windows API 调用的错误处理和资源释放。

Files:

  • src/platform/windows/clipboard.cpp
🪛 Clang (14.0.6)
src/platform/windows/clipboard.cpp

[error] 7-7: 'windows.h' file not found

(clang-diagnostic-error)

🔇 Additional comments (1)
src/platform/windows/clipboard.cpp (1)

40-114: LGTM — owner window + RAII 组合正确处理了服务场景。

ensure_clipboard_owner_window() 使用互斥保护、按 ERROR_CLASS_ALREADY_EXISTS 幂等注册类、懒创建 message-only 窗口并经 IsWindow 复用;clipboard_guard_t 以重试循环打开剪贴板并在析构时关闭,彻底解决了此前把 nullptr 传给 OpenClipboard 导致 SetClipboardData 在服务环境下失败的问题。读路径使用 nullptr owner、写路径使用 owner 窗口,划分清晰。

Comment on lines +436 to +455
std::size_t pixel_offset = info.biSize;
if (compression == BI_BITFIELDS) {
if (!checked_add(pixel_offset, 3 * sizeof(DWORD), pixel_offset)) {
GlobalUnlock(dib_handle);
return false;
}
}

std::size_t stride = 0;
std::size_t pixel_data_bytes = 0;
std::size_t required_size = 0;
std::size_t rgba_bytes = 0;
if (!dib_stride_valid(width, bit_count, stride) ||
!checked_mul(stride, static_cast<std::size_t>(height), pixel_data_bytes) ||
!checked_add(pixel_offset, pixel_data_bytes, required_size) ||
required_size > total_size ||
!decoded_image_size_valid(width, height, 4, rgba_bytes)) {
GlobalUnlock(dib_handle);
return false;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

For Windows CF_DIBV5 clipboard format with BITMAPV5HEADER and BI_BITFIELDS compression, are the color bit masks stored in the bV5RedMask/bV5GreenMask/bV5BlueMask/bV5AlphaMask header fields, or as three additional DWORDs following the header like BITMAPINFOHEADER?

💡 Result:

For Windows CF_DIBV5 clipboard format with BITMAPV5HEADER and BI_BITFIELDS compression, the color bit masks are stored in the header fields bV5RedMask, bV5GreenMask, bV5BlueMask, and bV5AlphaMask. They are not stored as three additional DWORDs following the header, unlike BITMAPINFOHEADER where the three DWORD masks follow the header in the bmiColors array (or immediately after for packed DIBs). This is confirmed by the official Microsoft documentation for BITMAPV5HEADER, which explicitly states that for BI_BITFIELDS, "the color masks for the red, green, and blue components of each pixel are specified in the bV5RedMask, bV5GreenMask, and bV5BlueMask members." The structure definition includes these fields after bV5ClrImportant. In contrast, for BITMAPINFOHEADER (used in CF_DIB), when biCompression is BI_BITFIELDS, "the bmiColors member contains three DWORD color masks" following the 40-byte header. Clipboard code examples for CF_DIBV5 calculate the offset to bitmap bits as header size + color table size, and only add extra DWORDs (12 bytes for RGB) if compression is BI_BITFIELDS when using BITMAPINFOHEADER or equivalent smaller headers, not for the full BITMAPV5HEADER (bV5Size=124). For BITMAPV5HEADER with BI_BITFIELDS, no extra masks are added because they are already in the header; the color table size (bV5ClrUsed) is typically 0 for such high-bpp bitmaps without a palette.

Citations:


BI_BITFIELDS 时像素偏移计算错误,导致 CF_DIBV5 格式无法解码

BITMAPV4HEADER(biSize=108)和 BITMAPV5HEADER(biSize=124)的颜色掩码已包含在头部字段内(bV5RedMaskbV5GreenMaskbV5BlueMaskbV5AlphaMask),而不是像 BITMAPINFOHEADER(biSize=40)那样在头部后跟随 3 个 DWORD。当前代码对所有 BI_BITFIELDS 格式都无条件地追加 3 * sizeof(DWORD),导致 V4/V5 头的 pixel_offset 增加 12 字节,在第 451 行的大小校验处失败。现代 Windows 剪贴板实现(Chrome、Edge 等浏览器、截图工具)常使用 CF_DIBV5 + BI_BITFIELDS + 32bpp 格式,此修复前会被全部拒绝。

仅当头部为 BITMAPINFOHEADER 时才需跳过紧跟的掩码 DWORD:

修复建议
       std::size_t pixel_offset = info.biSize;
-      if (compression == BI_BITFIELDS) {
+      if (compression == BI_BITFIELDS && info.biSize == sizeof(BITMAPINFOHEADER)) {
         if (!checked_add(pixel_offset, 3 * sizeof(DWORD), pixel_offset)) {
           GlobalUnlock(dib_handle);
           return false;
         }
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/platform/windows/clipboard.cpp` around lines 436 - 455, The BI_BITFIELDS
branch unconditionally adds 3*sizeof(DWORD) to pixel_offset causing
BITMAPV4HEADER/BITMAPV5HEADER (biSize 108/124) to be mis-parsed; update the
condition that adds the mask DWORDs so the extra 3*DWORD skip is only applied
when info.biSize equals the BITMAPINFOHEADER size (i.e. sizeof(BITMAPINFOHEADER)
/ biSize == 40) and leave V4/V5 headers alone; ensure you still call
GlobalUnlock(dib_handle) and return false on checked_add failure and keep the
subsequent validations (dib_stride_valid, checked_mul, checked_add,
decoded_image_size_valid) unchanged.

Comment on lines +852 to +870
switch (item.type) {
case LI_CLIPBOARD_ITEM_TYPE_TEXT:
return write_unicode_text(item, reason);
case LI_CLIPBOARD_ITEM_TYPE_IMAGE:
return write_png_image(item, reason);
case LI_CLIPBOARD_ITEM_TYPE_NONE:
if (!EmptyClipboard()) {
if (reason) {
*reason = "EmptyClipboard failed";
}
return false;
}
return true;
default:
if (reason) {
*reason = "Unsupported clipboard item type";
}
return false;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all producers of LI_CLIPBOARD_ITEM_TYPE_NONE being routed to write_item, to determine intended semantics.
rg -nP -C3 'LI_CLIPBOARD_ITEM_TYPE_NONE'
rg -nP -C5 '\bwrite_item\s*\('
rg -nP -C5 'suppress_next_host_echo|suppressed_host_hash|last_sent_hash'

Repository: AlkaidLab/foundation-sunshine

Length of output: 14756


🏁 Script executed:

# Search for client-side clipboard code and how NONE is generated/sent
find . -type f \( -name "*.c" -o -name "*.cpp" -o -name "*.h" \) | xargs grep -l "clipboard" | grep -v windows | head -20

Repository: AlkaidLab/foundation-sunshine

Length of output: 175


🏁 Script executed:

# Check stream.cpp more carefully for when write_item is actually called with NONE
rg -nP -B10 -A5 'write_item\(item' src/stream.cpp

Repository: AlkaidLab/foundation-sunshine

Length of output: 1111


🏁 Script executed:

# Look at the complete context of receive_clipboard_transfer to understand the flow
rg -nP -B20 -A30 'receive_clipboard_transfer|CLIPBOARD_MSG_CLIENT_FORMAT_DATA_RESPONSE' src/stream.cpp | head -200

Repository: AlkaidLab/foundation-sunshine

Length of output: 55


🏁 Script executed:

# Look for the code that sets session->control.clipboard.item_type from the transfer message
rg -nP -B5 -A10 'clipboard.item_type\s*=' src/stream.cpp | head -150

Repository: AlkaidLab/foundation-sunshine

Length of output: 1809


🏁 Script executed:

# Find where clipboard transfer data is received and parsed
rg -nP -B10 -A20 'CLIPBOARD_MSG_CLIENT_CLIPBOARD_DATA|receive.*clipboard.*transfer|clipboard.*transfer.*data' src/stream.cpp | head -200

Repository: AlkaidLab/foundation-sunshine

Length of output: 55


🏁 Script executed:

# Check if there's a Linux/macOS equivalent for comparison
fd -t f clipboard | grep -E '\.(cpp|h)$' | xargs rg -l 'write_item|read_current'

Repository: AlkaidLab/foundation-sunshine

Length of output: 141


🏁 Script executed:

# Find where item_type is extracted/parsed from the incoming clipboard message
rg -nP -B30 'session->control.clipboard.item_type = item_type' src/stream.cpp | head -100

Repository: AlkaidLab/foundation-sunshine

Length of output: 1719


🏁 Script executed:

# Look for CLIPBOARD_MSG_CLIENT_CLIPBOARD_DATA_START or similar message handling
rg -nP -B5 -A30 'CLIPBOARD_MSG_CLIENT_CLIPBOARD_DATA_START|unpack.*clipboard|parse.*clipboard' src/stream.cpp

Repository: AlkaidLab/foundation-sunshine

Length of output: 55


🏁 Script executed:

# Check the complete write_item function to understand the NONE case better
sed -n '843,875p' src/platform/windows/clipboard.cpp

Repository: AlkaidLab/foundation-sunshine

Length of output: 891


🏁 Script executed:

# Check the complete context around line 1516 where NONE is handled
sed -n '1510,1550p' src/stream.cpp

Repository: AlkaidLab/foundation-sunshine

Length of output: 1662


🏁 Script executed:

# Look for any conditions/flags that might prevent write_item from being called with NONE
sed -n '1820,1860p' src/stream.cpp

Repository: AlkaidLab/foundation-sunshine

Length of output: 2191


🏁 Script executed:

# Check if there are any guards preventing ITEM_START from being accepted with type=NONE
rg -nP -B10 'CLIPBOARD_MSG_CLIENT_CLIPBOARD_DATA_START' src/stream.cpp | head -50

Repository: AlkaidLab/foundation-sunshine

Length of output: 55


🏁 Script executed:

# Check if there's any client-side code or documentation about when NONE is sent
find . -type f \( -name "*.md" -o -name "*.txt" -o -name "*.rst" \) | xargs grep -i "clipboard.*none\|NONE.*clipboard" 2>/dev/null

Repository: AlkaidLab/foundation-sunshine

Length of output: 57


🏁 Script executed:

# Look for any protocol documentation or comments about clipboard types
rg -nP -B5 -A5 'LI_CLIPBOARD_ITEM_TYPE.*TEXT|LI_CLIPBOARD_ITEM_TYPE.*IMAGE|LI_CLIPBOARD_ITEM_TYPE.*NONE' include/ src/ | grep -E '(//|/\*|define|enum)' | head -50

Repository: AlkaidLab/foundation-sunshine

Length of output: 275


🏁 Script executed:

# Check the header file that defines clipboard types
find . -name "*.h" -type f | xargs grep -l 'LI_CLIPBOARD_ITEM_TYPE' | head -5

Repository: AlkaidLab/foundation-sunshine

Length of output: 55


🏁 Script executed:

# Verify the structure of how OpenClipboard/EmptyClipboard are used
sed -n '843,880p' src/platform/windows/clipboard.cpp

Repository: AlkaidLab/foundation-sunshine

Length of output: 891


🏁 Script executed:

# Search for any documentation or comments about clipboard item types in protocol headers
find . -name "*.h" -type f -print0 | xargs -0 rg -l 'CLIPBOARD' 2>/dev/null | head -10

Repository: AlkaidLab/foundation-sunshine

Length of output: 55


🏁 Script executed:

# Look for where LI_CLIPBOARD_ITEM_TYPE is defined
rg -n 'enum.*CLIPBOARD_ITEM_TYPE|define.*LI_CLIPBOARD_ITEM_TYPE' --type-list

Repository: AlkaidLab/foundation-sunshine

Length of output: 463


🏁 Script executed:

# Check if there's any guard that prevents client from sending NONE
sed -n '1700,1750p' src/stream.cpp

Repository: AlkaidLab/foundation-sunshine

Length of output: 2566


🏁 Script executed:

# Verify the echo suppression logic when content_hash is 0
sed -n '1840,1850p' src/stream.cpp

Repository: AlkaidLab/foundation-sunshine

Length of output: 710


🏁 Script executed:

# Check if there are any comments or documentation about the protocol/semantics
rg -nP -B3 -A3 'NONE|clipboard.*semantic|clipboard.*protocol|clipboard.*clear' src/stream.cpp | head -100

Repository: AlkaidLab/foundation-sunshine

Length of output: 2257


🏁 Script executed:

# Look for any test cases that might clarify the intended behavior
cat tests/unit/test_stream.cpp | grep -A20 -B5 'NONE\|clipboard'

Repository: AlkaidLab/foundation-sunshine

Length of output: 2953


🏁 Script executed:

# Check the complete clipboard_guard_t to understand resource management
rg -nP -B5 -A15 'class clipboard_guard|struct clipboard_guard' src/platform/windows/clipboard.cpp

Repository: AlkaidLab/foundation-sunshine

Length of output: 707


🏁 Script executed:

# Verify if read_current_item can actually set NONE when reading host clipboard
sed -n '480,590p' src/platform/windows/clipboard.cpp

Repository: AlkaidLab/foundation-sunshine

Length of output: 3840


🏁 Script executed:

# Check if there are any explicit checks that prevent NONE from being sent by client
sed -n '1700,1790p' src/stream.cpp | grep -A50 'CLIPBOARD_MSG_CLIENT_CLIPBOARD_DATA_START' | head -60

Repository: AlkaidLab/foundation-sunshine

Length of output: 55


🏁 Script executed:

# Look for where the client actually initiates sending NONE (there might be client code)
find . -type f -name "*.cpp" -o -name "*.c" | xargs rg -l 'client.*clipboard|clipboard.*client' | grep -v windows | grep -v test

Repository: AlkaidLab/foundation-sunshine

Length of output: 90


🏁 Script executed:

# Check the complete picture of the echo suppression to ensure it handles NONE correctly
sed -n '1835,1870p' src/stream.cpp

Repository: AlkaidLab/foundation-sunshine

Length of output: 1744


🏁 Script executed:

# Look for any other places where write_item might be called
rg -nP 'write_item' src/

Repository: AlkaidLab/foundation-sunshine

Length of output: 352


确认问题有效,并补充发现 echo suppression 存在缺陷

收到 LI_CLIPBOARD_ITEM_TYPE_NONE 就调用 EmptyClipboard() 的问题确实存在。除了原有的顾虑外,还发现以下缺陷:

  1. 无防护措施:客户端可通过 CLIPBOARD_MSG_CLIENT_CLIPBOARD_DATA_START 发送任意 type,包括 NONE。校验函数 clipboard_transfer_length_valid() 仅验证 total_length == 0,不限制 NONE 的发送。

  2. Echo suppression 机制对 NONE 失效

    • Line 1843:suppress_next_host_echo = item.content_hash != 0;
    • NONE 的 content_hash 为 0,导致 suppress 为 false
    • 结果:清空剪贴板后,host 立即检测到空剪贴板并回发 NONE,可能造成循环或不预期行为
  3. 建议修复

    • 若 NONE 不应由客户端主动发送,在 line 1722 后添加校验,拒绝非 SNAPSHOT 标志的 NONE
    • 若 NONE 允许清空,则需修正 line 1843 为 suppress_next_host_echo = (item.type != LI_CLIPBOARD_ITEM_TYPE_NONE) && (item.content_hash != 0);,或在 write_item 内部对 NONE 特殊处理
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/platform/windows/clipboard.cpp` around lines 852 - 870, The switch
handling LI_CLIPBOARD_ITEM_TYPE_NONE currently calls EmptyClipboard() and allows
clients to send NONE, which bypasses clipboard_transfer_length_valid() and
breaks echo suppression; to fix this, add a validation in the client-transfer
handling (the code path that processes CLIPBOARD_MSG_CLIENT_CLIPBOARD_DATA_START
/ uses clipboard_transfer_length_valid()) to reject LI_CLIPBOARD_ITEM_TYPE_NONE
unless the SNAPSHOT flag is set, and also change the echo-suppression assignment
(the line setting suppress_next_host_echo from item.content_hash) to guard
against NONE—e.g., only set suppress_next_host_echo when item.type !=
LI_CLIPBOARD_ITEM_TYPE_NONE && item.content_hash != 0—so NONE does not disable
suppression; ensure the switch (write_unicode_text / write_png_image /
EmptyClipboard) behavior remains unchanged except for the added validation
upstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants