Conversation
Summary by CodeRabbit发布说明
工作总览该拉取请求改进了三个文件中的错误处理和日志记录机制。更新了状态文件操作以使用显式文件流和错误检查，增强了 Windows 平台的字符编码转换和命令执行的诊断日志，并添加了 UTF-8 转换失败时的 ANSI 代码页回退机制。 变更
代码审查工作量评估🎯 3 (中等) | ⏱️ ~20 分钟 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (1)
src/platform/windows/misc.cpp (1)
2033-2046: 语义变化确实存在，但应评估实际影响。
from_utf8()的 CP_ACP 回退行为改变了函数的错误处理语义：原本 UTF-8 解码失败会返回空字符串，现在会回退到 ANSI 代码页解码并返回结果。根据代码分析，这似乎是有意设计的，用于提高在非 UTF-8 系统（如中文 Windows GBK 环境）上的鲁棒性。实际影响评估：
音频设备匹配场景（audio.cpp 的
is_sink_available()等）：from_utf8()返回的 ANSI 解码字符串被传入match_all_fields()用于设备查找。虽然语义改变了，但回退通常是安全的——如果设备名称在 UTF-8 和 ANSI 中都能正确解码，匹配行为实际上改善了。隐默行为变化：调用方无法从 API 推断出是否发生了回退。虽然函数内有警告日志，但函数签名未体现此行为。
建议：
- 如果 CP_ACP 回退是增强特性，应在函数文档中明确说明此行为
- 如果某些调用方需要严格 UTF-8 验证，应考虑提供选项或使用专用的严格转换函数
- 当前代码工作正常，但文档化会提高代码可维护性
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/windows/misc.cpp` around lines 2033 - 2046, The review points out that from_utf8() now falls back to CP_ACP (ANSI) on decode failure which changes semantics (silent fallback) and can surprise callers like audio.cpp::is_sink_available() and match_all_fields(); update the codebase by documenting this fallback in from_utf8()'s API comment and README, and add a strict alternative or flag (e.g., from_utf8_strict or a parameter on from_utf8) that returns an empty string/error on non-UTF-8 input so callers that require strict validation can opt in; reference the functions from_utf8, from_utf8_strict (new) and callers such as match_all_fields and is_sink_available when adding docs and usage notes.
🤖 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/misc.cpp`:
- Around line 2019-2031: The diagnostic log currently writes potentially
sensitive data (hex_dump, string.substr) at error level using BOOST_LOG(error);
change this to avoid leaking PII by either lowering the log severity (e.g., use
BOOST_LOG(debug) instead of BOOST_LOG(error)) or gate the detailed output behind
a debug/diagnostic flag (check a runtime isDebug/isDiagnosticEnabled boolean) so
that hex_dump, dump_len and string preview are only emitted when debugging is
enabled; keep the surrounding message (winerr, string.size()) but ensure
detailed hex_dump and substring are suppressed at error level.
---
Nitpick comments:
In `@src/platform/windows/misc.cpp`:
- Around line 2033-2046: The review points out that from_utf8() now falls back
to CP_ACP (ANSI) on decode failure which changes semantics (silent fallback) and
can surprise callers like audio.cpp::is_sink_available() and match_all_fields();
update the codebase by documenting this fallback in from_utf8()'s API comment
and README, and add a strict alternative or flag (e.g., from_utf8_strict or a
parameter on from_utf8) that returns an empty string/error on non-UTF-8 input so
callers that require strict validation can opt in; reference the functions
from_utf8, from_utf8_strict (new) and callers such as match_all_fields and
is_sink_available when adding docs and usage notes.
🪄 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: cfe28e99-4653-4c03-bd0d-f7368345b7c0
📒 Files selected for processing (3)
src/nvhttp.cppsrc/platform/windows/input.cppsrc/platform/windows/misc.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/input.cppsrc/platform/windows/misc.cppsrc/nvhttp.cpp
src/platform/**
⚙️ CodeRabbit configuration file
src/platform/**: 平台抽象层代码（Windows/Linux/macOS）。确保各平台实现一致， 注意 Windows API 调用的错误处理和资源释放。
Files:
src/platform/windows/input.cppsrc/platform/windows/misc.cpp
🔇 Additional comments (4)
src/platform/windows/misc.cpp (1)
1039-1039: 使用wstring()直接获取宽字符路径是正确的改进。相比之前通过
from_utf8(working_dir.string())转换，直接调用working_dir.wstring()可以正确处理 Windows 原生路径编码（如中文 Windows 系统上的 GBK 编码路径），避免 UTF-8 转换失败的问题。src/platform/windows/input.cpp (1)
1290-1293: 增强的错误日志记录有助于诊断 UTF-8 转换问题。现在记录了
GetLastError()和输入大小，这与misc.cpp中的诊断改进保持一致。需注意：根据
src/input.cpp:856-865，passthrough()在数据包处理循环中调用此函数，没有频率限制。如果客户端持续发送无效 UTF-8 数据，可能导致日志泛滥。如果这成为问题，可以考虑添加速率限制或使用
debug级别日志。src/nvhttp.cpp (2)
318-323: 显式流打开失败处理做得很好对
ifstream/ofstream增加is_open()检查并在失败时明确日志和早返回，这一改动提升了状态读写失败时的可诊断性与稳定性。Also applies to: 348-353, 372-378
315-316: [Your rewritten review comment text here]
[Exactly ONE classification tag]
|
|
||
| // Log hex dump of the first bytes to help diagnose the invalid string | ||
| std::ostringstream hex_dump; | ||
| auto dump_len = std::min<size_t>(string.size(), 64); | ||
| for (size_t i = 0; i < dump_len; i++) { | ||
| hex_dump << std::hex << std::setfill('0') << std::setw(2) | ||
| << (static_cast<unsigned>(string[i]) & 0xFF); | ||
| if (i + 1 < dump_len) hex_dump << ' '; | ||
| } | ||
| BOOST_LOG(error) << "Failed to get UTF-16 buffer size: "sv << winerr | ||
| << " (len=" << string.size() | ||
| << ", hex=[" << hex_dump.str() << "]" | ||
| << ", str=\"" << string.substr(0, 64) << "\")"; |
There was a problem hiding this comment.
诊断日志可能泄露敏感信息。
记录输入字符串的十六进制转储和预览对于调试编码问题很有帮助，但需要注意：如果输入包含敏感数据（如路径中的用户名、密码或其他 PII），这些信息可能会被写入日志文件。
建议将日志级别从 error 降为 debug，或者仅在调试模式下输出完整内容：
💡 建议的改进
- BOOST_LOG(error) << "Failed to get UTF-16 buffer size: "sv << winerr
+ BOOST_LOG(debug) << "Failed to get UTF-16 buffer size: "sv << winerr
<< " (len=" << string.size()
<< ", hex=[" << hex_dump.str() << "]"
<< ", str=\"" << string.substr(0, 64) << "\")";📝 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.
| // Log hex dump of the first bytes to help diagnose the invalid string | |
| std::ostringstream hex_dump; | |
| auto dump_len = std::min<size_t>(string.size(), 64); | |
| for (size_t i = 0; i < dump_len; i++) { | |
| hex_dump << std::hex << std::setfill('0') << std::setw(2) | |
| << (static_cast<unsigned>(string[i]) & 0xFF); | |
| if (i + 1 < dump_len) hex_dump << ' '; | |
| } | |
| BOOST_LOG(error) << "Failed to get UTF-16 buffer size: "sv << winerr | |
| << " (len=" << string.size() | |
| << ", hex=[" << hex_dump.str() << "]" | |
| << ", str=\"" << string.substr(0, 64) << "\")"; | |
| // Log hex dump of the first bytes to help diagnose the invalid string | |
| std::ostringstream hex_dump; | |
| auto dump_len = std::min<size_t>(string.size(), 64); | |
| for (size_t i = 0; i < dump_len; i++) { | |
| hex_dump << std::hex << std::setfill('0') << std::setw(2) | |
| << (static_cast<unsigned>(string[i]) & 0xFF); | |
| if (i + 1 < dump_len) hex_dump << ' '; | |
| } | |
| BOOST_LOG(debug) << "Failed to get UTF-16 buffer size: "sv << winerr | |
| << " (len=" << string.size() | |
| << ", hex=[" << hex_dump.str() << "]" | |
| << ", str=\"" << string.substr(0, 64) << "\")"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/platform/windows/misc.cpp` around lines 2019 - 2031, The diagnostic log
currently writes potentially sensitive data (hex_dump, string.substr) at error
level using BOOST_LOG(error); change this to avoid leaking PII by either
lowering the log severity (e.g., use BOOST_LOG(debug) instead of
BOOST_LOG(error)) or gate the detailed output behind a debug/diagnostic flag
(check a runtime isDebug/isDiagnosticEnabled boolean) so that hex_dump, dump_len
and string preview are only emitted when debugging is enabled; keep the
surrounding message (winerr, string.size()) but ensure detailed hex_dump and
substring are suppressed at error level.
1 participant
sunshine安装在中文路径，会导致无法配对成功。
考虑到一些权限问题，我还是觉得应该建议安装在默认的C盘路径