fix: expose clipboard blob routes on nvhttp#643
Conversation
Summary by CodeRabbit发布说明
Walkthrough重构 clipboard blob POST/GET 端点：新增 ChangesBlob 端点响应构造重构
Sequence Diagram(s)sequenceDiagram
participant Client
participant nvhttp as nvhttp.Handler
participant process_upload as clipboard_http::process_blob_upload
participant make_upload as clipboard_http::make_blob_upload_response
participant process_get as clipboard_http::process_blob_get
participant make_get as clipboard_http::make_blob_get_response
participant clipboard_sync as config::input.clipboard_sync
participant blob_store as clipboard_blob_store
Client->>nvhttp: POST /api/v1/clipboard/blob (headers, body)
nvhttp->>process_upload: forward Request
process_upload->>make_upload: headers, body
make_upload->>clipboard_sync: check enabled
make_upload->>blob_store: put(mime, bytes)
blob_store-->>make_upload: result (id/size/expires_in) or error
make_upload-->>process_upload: blob_response_t
process_upload-->>nvhttp: status/body/headers
nvhttp-->>Client: HTTP response
Client->>nvhttp: GET /api/v1/clipboard/blob/<id>
nvhttp->>process_get: forward Request
process_get->>make_get: id
make_get->>clipboard_sync: check enabled
make_get->>blob_store: get(id, consume=false)
blob_store-->>make_get: (found|not found, mime, bytes)
make_get-->>process_get: blob_response_t
process_get-->>nvhttp: status/body/headers
nvhttp-->>Client: HTTP response
预估审查工作量🎯 4 (Complex) | ⏱️ ~45 minutes 相关 PR
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/clipboard_http.cpp`:
- Around line 316-319: Check Content-Length in the request headers before
consuming req->content: validate header via req->header (reject missing/invalid
with bad_content_length, reject > allowed limit with payload_too_large) and only
then read the body (avoiding ss << req->content.rdbuf() for oversized streams);
call make_blob_upload_response after that validation and adjust the NVHTTP
mirror route logic (the same pre-check around the code handling lines ~357-369)
so large uploads are rejected early and memory isn't fully allocated upfront.
🪄 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: 945bd3e5-75bf-4924-8c54-82bc49b19a4e
📒 Files selected for processing (3)
src/clipboard_http.cppsrc/clipboard_http.hsrc/nvhttp.cpp
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/clipboard_http.hsrc/nvhttp.cppsrc/clipboard_http.cpp
- factor blob upload/get response building out of confighttp handlers - reject missing, invalid, or oversized blob Content-Length before reading request bodies - mirror clipboard blob POST/GET endpoints onto nvhttp - let paired Moonlight clients fetch large clipboard blobs via cert auth - keep local GUI agent clipboard flow unchanged
qiin2333
force-pushed
the
fix/clipboard-blob-nvhttp
branch
from
7af464b to
5d7d4c7
Compare
- clipboard_blob_store::make_id now uses OpenSSL RAND_bytes (122 bits CSPRNG + RFC 4122 v4 layout) instead of std::mt19937_64. The id is effectively a bearer capability for any HTTPS-authenticated client that learns it via a KIND_REF wire frame, so cryptographic strength is required. - Add inline templated process_blob_upload / process_blob_get helpers that encapsulate preflight + body read + path_match extraction. confighttp and nvhttp now share one definition each, eliminating duplicated lambda bodies while still working with nvhttp's bespoke SunshineHTTPS request type.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/nvhttp.cpp (1)
2430-2441: 💤 Low value镜像路由的设计合理，但建议补充
print_req以保持日志一致性新增的 blob 路由依赖 nvhttp 现有的客户端证书验证（
https_server.verify）作为认证手段，而 confighttp 那边使用auth_fn走 basic auth，这种分工与 PR 描述一致。process_blob_upload内部已先按Content-Length预检，再读 body，因此不会被超大上传压垮内存。不过该 lambda 与 nvhttp 中其他 handler 不同，没有调用
print_req<SunshineHTTPS>(request)，会导致 debug/verbose 日志里缺少这两条端点的请求记录，调试 Moonlight 客户端的 blob 走向时不便。建议补一行：♻️ 建议补充日志
https_server.resource["^/api/v1/clipboard/blob$"]["POST"] = [](resp_https_t response, req_https_t request) { + print_req<SunshineHTTPS>(request); auto out = clipboard_http::process_blob_upload(request); response->write(out.status, out.body, out.headers); }; https_server.resource["^/api/v1/clipboard/blob/([A-Za-z0-9_\\-]{1,128})$"]["GET"] = [](resp_https_t response, req_https_t request) { + print_req<SunshineHTTPS>(request); auto out = clipboard_http::process_blob_get(request); response->write(out.status, out.body, out.headers); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/nvhttp.cpp` around lines 2430 - 2441, The two new nvhttp route handlers for "/api/v1/clipboard/blob" and "/api/v1/clipboard/blob/([A-Za-z0-9_\\-]{1,128})" should call the existing request-logging helper to keep logs consistent; inside each lambda (the POST handler that calls clipboard_http::process_blob_upload and the GET handler that calls clipboard_http::process_blob_get) add a call to print_req<SunshineHTTPS>(request) before invoking the process_* functions so those requests appear in debug/verbose logs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/nvhttp.cpp`:
- Around line 2430-2441: The two new nvhttp route handlers for
"/api/v1/clipboard/blob" and "/api/v1/clipboard/blob/([A-Za-z0-9_\\-]{1,128})"
should call the existing request-logging helper to keep logs consistent; inside
each lambda (the POST handler that calls clipboard_http::process_blob_upload and
the GET handler that calls clipboard_http::process_blob_get) add a call to
print_req<SunshineHTTPS>(request) before invoking the process_* functions so
those requests appear in debug/verbose logs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 080f78b0-9972-4e86-b2c5-88ab6e13ced6
📒 Files selected for processing (4)
src/clipboard_blob_store.cppsrc/clipboard_http.cppsrc/clipboard_http.hsrc/nvhttp.cpp
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/clipboard_blob_store.cppsrc/clipboard_http.hsrc/nvhttp.cppsrc/clipboard_http.cpp
🔇 Additional comments (5)
src/clipboard_http.h (1)
89-97: 预检在前、读取在后的设计正确，符合内存安全要求
process_blob_upload在调用make_blob_upload_preflight_response通过Content-Length拦截过大请求后再读取 body，避免了为超限请求分配内存；同时通过模板适配SimpleWeb::HTTPS与SunshineHTTPS两种 Request 类型，确实做到了 confighttp 与 nvhttp 共用同一份逻辑。src/clipboard_blob_store.cpp (1)
33-63: CSPRNG 替换 mt19937 的实现正确，注释也明确说明了为何必须是密码学安全 RNG
raw[6] & 0x0F | 0x40与raw[8] & 0x3F | 0x80正确设置了 RFC 4122 v4 版本/变体位，kSlots中的 16 个位置也对应 UUID 字符串中除连字符外的字节偏移。RAND_bytes失败时抛出std::runtime_error，由put()内std::lock_guard的 RAII 析构释放g_mu，符合异常安全。鉴于注释说明 OpenSSL RNG 不可用时 TLS 栈本身也已经无法工作，硬失败是合理选择。src/clipboard_http.cpp (3)
305-346: helper 拆分与处理器委托清晰，已正确解决先前关于先读后检的反馈
json_blob_response/parse_content_length抽离合理：parse_content_length用std::all_of(isdigit)先排除+/空白/负号，再用std::stoull配合consumed == value.size()校验整段消耗，并以try/catch(...)兜住std::out_of_range，对溢出和畸形输入都是稳健的。handle_blob_upload现在只做 auth + 委托给process_blob_upload，由后者先跑预检再读 body，与之前 review 中提到的「先读后检会让超大上传先占用内存」一致。
349-424: 💤 Low value
Content-Length必填会拒绝分块传输上传，建议确认 Moonlight/curl 调用方都会发送Content-Length
make_blob_upload_preflight_response在没有Content-Lengthheader 时直接返回bad_content_length，这对正常 Moonlight 客户端没问题，但任何使用Transfer-Encoding: chunked的客户端（一些 HTTP 库默认就是 chunked）都会被拒。如果产品形态确认 nvhttp 这条新路由的调用方都会显式带Content-Length，这层严格性是可接受的安全权衡；如果未来要兼容 chunked 上传，则需在读取 body 后再二次检查实际字节数。另外一个小观察：在
make_blob_upload_response中预检会在clipboard_sync开关之前执行（Line 353-360），因此clipboard_sync关闭时仍会先暴露bad_content_length这种字段层面的错误信息——影响很小，但如果想严格做信息隐藏，可考虑把clipboard_sync检查前移。
426-462: GET 路径的安全防护到位
id.size() != 36校验配合clipboard_blob_store::make_id产出的 UUID-v4 形状（36 字符）一致，避免路由正则[A-Za-z0-9_\-]{1,128}被滥用。Content-Type回显的是 upload 时已通过is_valid_mime严格校验过的值，再叠加X-Content-Type-Options: nosniff与Cache-Control: no-store，防御 UA sniff 与中间缓存都已覆盖；consume=false也保留了短时重试能力，TTL 兜底回收。
1 participant
Summary
POST /api/v1/clipboard/blobandGET /api/v1/clipboard/blob/<id>blob transport onnvhttpfor paired Moonlight clientscapability,item, andeventsonconfighttp; the local GUI agent loopback flow is unchangedProblem
The existing clipboard design intentionally routes real clipboard handling through the user-session GUI agent. The C++ service is an opaque byte bridge because it runs outside the interactive user session and should not parse clipboard payloads or touch the OS clipboard directly.
That design works for normal text and small image payloads, but large payloads use the REF + blob path: a small
KIND_REFframe is sent over the encrypted control stream while the actual bytes are stored in the short-lived clipboard blob store.Before this patch, the blob HTTP endpoints were only available through
confighttp, which is the right endpoint for the local GUI agent (https://127.0.0.1:<web-ui-port>), but it is not a natural endpoint for paired remote Moonlight clients. Those clients already authenticate tonvhttpwith their pairing certificate and should not need Web UI credentials just to resolve a clipboard blob reference.Solution
This patch does not move clipboard processing into
nvhttpand does not remove or replace theconfighttpGUI-agent endpoints.Instead, it keeps the original architecture intact and mirrors only the blob object transport onto
nvhttp:confighttpforcapability,item,events, and blob access on loopbacknvhttpwhen they send or receiveKIND_REFframesThe shared helper also validates
Content-Lengthbefore reading upload bodies, so missing/invalid lengths are rejected withbad_content_lengthand oversized uploads are rejected withpayload_too_largebefore allocating the full body.Testing
src/clipboard_http.h,src/clipboard_http.cpp, andsrc/nvhttp.cppsunshinesuccessfully withcmake --build build --target sunshine -j4/mingw64/binlocallyNotes
I intentionally did not remove any
confighttpclipboard routes. They are the canonical local GUI-agent transport and are still required for the user-session clipboard bridge. This PR only adds a paired-client blob transport path for REF payloads.