Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 71 additions & 46 deletions src/nvhttp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define BOOST_BIND_GLOBAL_PLACEHOLDERS

// standard includes
#include <algorithm>
#include <chrono>
#include <filesystem>
#include <map>
Expand Down Expand Up @@ -1102,6 +1103,7 @@ namespace nvhttp {
tree.put("root.currentgame", current_appid);
tree.put("root.state", current_appid > 0 ? "SUNSHINE_SERVER_BUSY" : "SUNSHINE_SERVER_FREE");
tree.put("root.appListEtag", proc::proc.get_apps_etag());
tree.put("root.DesktopSpecialAppSupport", 1);

// AI capability: inform client if AI proxy is available
tree.put("root.AiCapability", confighttp::isAiEnabled() ? 1 : 0);
Expand Down Expand Up @@ -1777,13 +1779,24 @@ namespace nvhttp {
}
}

void
launch(bool &host_audio, resp_https_t response, req_https_t request) {
print_req<SunshineHTTPS>(request);
static bool
has_required_launch_params(const args_t &args, bool require_appid) {
return args.find("rikey"s) != std::end(args)
&& args.find("rikeyid"s) != std::end(args)
&& args.find("localAudioPlayMode"s) != std::end(args)
&& (!require_appid || args.find("appid"s) != std::end(args));
}

print_request_ip<SunshineHTTPS>(request, "Launch request");
static bool
has_required_resume_params(const args_t &args) {
return args.find("rikey"s) != std::end(args)
&& args.find("rikeyid"s) != std::end(args);
}

void
launch_app(bool &host_audio, resp_https_t response, req_https_t request, const args_t &args, int appid, const char *result_node_name) {
pt::ptree tree;
const auto result_node = "root."s + result_node_name;
bool need_to_restore_display_state { false };
auto g = util::fail_guard([&]() {
std::ostringstream data;
Expand All @@ -1801,24 +1814,9 @@ namespace nvhttp {
}
});

auto args = request->parse_query_string();
if (
args.find("rikey"s) == std::end(args) ||
args.find("rikeyid"s) == std::end(args) ||
args.find("localAudioPlayMode"s) == std::end(args) ||
args.find("appid"s) == std::end(args)) {
tree.put("root.resume", 0);
tree.put("root.<xmlattr>.status_code", 400);
tree.put("root.<xmlattr>.status_message", "Missing a required launch parameter");

return;
}

auto appid = util::from_view(get_arg(args, "appid"));

auto current_appid = proc::proc.running();
if (current_appid > 0) {
tree.put("root.resume", 0);
tree.put(result_node, 0);
tree.put("root.<xmlattr>.status_code", 400);
tree.put("root.<xmlattr>.status_message", "An app is already running on this host");

Expand All @@ -1828,15 +1826,18 @@ namespace nvhttp {
// Early validation of AppID to prevent starting VDD or other expensive operations
// if the requested app does not exist.
if (proc::proc.get_app_name(appid).empty()) {
tree.put("root.resume", 0);
tree.put(result_node, 0);
tree.put("root.<xmlattr>.status_code", 404);
tree.put("root.<xmlattr>.status_message", "App not found");
BOOST_LOG(error) << "Launch couldn't find app with ID ["sv << appid << ']';
return;
}

host_audio = util::from_view(get_arg(args, "localAudioPlayMode"));
if (args.find("localAudioPlayMode"s) != std::end(args)) {
host_audio = util::from_view(get_arg(args, "localAudioPlayMode"));
}
const auto launch_session = make_launch_session(host_audio, args);
launch_session->appid = appid;

// 获取客户端证书UUID(稳定的客户端标识符)
std::string client_cert_uuid = get_client_cert_uuid_from_request(request);
Expand All @@ -1860,7 +1861,7 @@ namespace nvhttp {
if (video::probe_encoders()) {
tree.put("root.<xmlattr>.status_code", 503);
tree.put("root.<xmlattr>.status_message", "Failed to initialize video capture/encoding. Is a display connected and turned on?");
tree.put("root.gamesession", 0);
tree.put(result_node, 0);

return;
}
Expand All @@ -1872,27 +1873,25 @@ namespace nvhttp {

tree.put("root.<xmlattr>.status_code", 403);
tree.put("root.<xmlattr>.status_message", "Encryption is mandatory for this host but unsupported by the client");
tree.put("root.gamesession", 0);
tree.put(result_node, 0);

return;
}

if (appid > 0) {
auto err = proc::proc.execute(appid, launch_session);
if (err) {
tree.put("root.<xmlattr>.status_code", err);
tree.put("root.<xmlattr>.status_message", "Failed to start the specified application");
tree.put("root.gamesession", 0);
auto err = proc::proc.execute(appid, launch_session);
if (err) {
tree.put("root.<xmlattr>.status_code", err);
tree.put("root.<xmlattr>.status_message", "Failed to start the specified application");
tree.put(result_node, 0);
Comment on lines +1881 to +1885
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 | ⚡ Quick win

内部执行失败不要直接回填为负的 status_code

proc::proc.execute() 这里除了 404 还会返回 -1。直接写进 XML 会产生无效状态码,客户端侧的失败分支会变得不可预测。更稳妥的做法是只透传明确的协议状态(比如 404),其余内部错误统一映射到 500/503,原始返回值继续只打日志。

🛠️ 建议修改
-    auto err = proc::proc.execute(appid, launch_session);
+    const auto err = proc::proc.execute(appid, launch_session);
     if (err) {
-      tree.put("root.<xmlattr>.status_code", err);
+      tree.put("root.<xmlattr>.status_code", err == 404 ? 404 : 500);
       tree.put("root.<xmlattr>.status_message", "Failed to start the specified application");
       tree.put(result_node, 0);

       return;
     }
📝 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
auto err = proc::proc.execute(appid, launch_session);
if (err) {
tree.put("root.<xmlattr>.status_code", err);
tree.put("root.<xmlattr>.status_message", "Failed to start the specified application");
tree.put(result_node, 0);
const auto err = proc::proc.execute(appid, launch_session);
if (err) {
tree.put("root.<xmlattr>.status_code", err == 404 ? 404 : 500);
tree.put("root.<xmlattr>.status_message", "Failed to start the specified application");
tree.put(result_node, 0);
🤖 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 1881 - 1885, proc::proc.execute 返回的 err 不应直接写入
XML 为负值:在处理其返回值(变量 err)时,如果 err == 404 则原样透传到
tree.put("root.<xmlattr>.status_code"),否则把内部错误统一映射为 HTTP 5xx(例如 500 或 503)写入
status_code 并设置合适的 status_message,同时将原始 err 值记录到日志而不是写入 XML;修改涉及的符号有
proc::proc.execute、err、tree.put("root.<xmlattr>.status_code")、tree.put("root.<xmlattr>.status_message")
和 result_node,保持对 result_node 的原有赋值逻辑(tree.put(result_node, 0))不变。


return;
}
return;
}

tree.put("root.<xmlattr>.status_code", 200);
tree.put("root.sessionUrl0", launch_session->rtsp_url_scheme +
net::addr_to_url_escaped_string(request->local_endpoint().address()) + ':' +
std::to_string(net::map_port(rtsp_stream::RTSP_SETUP_PORT)));
tree.put("root.gamesession", 1);
tree.put(result_node, 1);

rtsp_stream::launch_session_raise(launch_session);

Expand All @@ -1916,6 +1915,35 @@ namespace nvhttp {
need_to_restore_display_state = false;
}

void
launch(bool &host_audio, resp_https_t response, req_https_t request) {
print_req<SunshineHTTPS>(request);

print_request_ip<SunshineHTTPS>(request, "Launch request");

auto args = request->parse_query_string();
if (!has_required_launch_params(args, true)) {
pt::ptree tree;
auto g = util::fail_guard([&]() {
std::ostringstream data;

pt::write_xml(data, tree);
response->write(data.str());
response->close_connection_after_response = true;
});

tree.put("root.gamesession", 0);
tree.put("root.<xmlattr>.status_code", 400);
tree.put("root.<xmlattr>.status_message", "Missing a required launch parameter");

return;
}

auto appid = util::from_view(get_arg(args, "appid"));

launch_app(host_audio, response, request, args, appid, "gamesession");
}

void
resume(bool &host_audio, resp_https_t response, req_https_t request) {
print_req<SunshineHTTPS>(request);
Expand All @@ -1941,26 +1969,22 @@ namespace nvhttp {
response->close_connection_after_response = true;
});

auto current_appid = proc::proc.running();
if (current_appid == 0) {
tree.put("root.resume", 0);
tree.put("root.<xmlattr>.status_code", 503);
tree.put("root.<xmlattr>.status_message", "No running app to resume");

return;
}

auto args = request->parse_query_string();
if (
args.find("rikey"s) == std::end(args) ||
args.find("rikeyid"s) == std::end(args)) {
if (!has_required_resume_params(args)) {
tree.put("root.resume", 0);
tree.put("root.<xmlattr>.status_code", 400);
tree.put("root.<xmlattr>.status_message", "Missing a required resume parameter");

return;
}

auto current_appid = proc::proc.running();
if (current_appid == 0) {
g.disable();
launch_app(host_audio, response, request, args, proc::DESKTOP_APP_ID, "resume");
return;
}

// Newer Moonlight clients send localAudioPlayMode on /resume too,
// so we should use it if it's present in the args and there are
// no active sessions we could be interfering with.
Expand All @@ -1969,6 +1993,7 @@ namespace nvhttp {
host_audio = util::from_view(get_arg(args, "localAudioPlayMode"));
}
const auto launch_session = make_launch_session(host_audio, args);
launch_session->appid = current_appid;

// Get client certificate UUID (stable client identifier) and store it in env
std::string client_cert_uuid = get_client_cert_uuid_from_request(request);
Expand Down Expand Up @@ -2000,7 +2025,7 @@ namespace nvhttp {

tree.put("root.<xmlattr>.status_code", 403);
tree.put("root.<xmlattr>.status_message", "Encryption is mandatory for this host but unsupported by the client");
tree.put("root.gamesession", 0);
tree.put("root.resume", 0);

return;
}
Expand Down
74 changes: 56 additions & 18 deletions src/process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,21 @@ namespace proc {
}
};

namespace {
ctx_t
make_desktop_app() {
ctx_t app;
app.name = std::string(DESKTOP_APP_NAME);
app.image_path = std::string(DESKTOP_APP_IMAGE_PATH);
app.id = std::to_string(DESKTOP_APP_ID);
app.auto_detach = true;
app.wait_all = true;
app.mouse_mode = 0;
app.exit_timeout = std::chrono::seconds { 5 };
return app;
Comment on lines +65 to +76
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 | ⚡ Quick win

ctx_t 显式值初始化,避免 Desktop 路径带出未定义状态。

这里的 ctx_t app; 只填了部分字段,elevated 这类标量成员仍然是未初始化值。当前 Desktop 主要走 placebo 分支,但后面只要有代码读取这些字段就是 UB;直接改成值初始化最稳妥。

🛠️ 建议修改
-      ctx_t app;
+      ctx_t app {};
       app.name = std::string(DESKTOP_APP_NAME);
       app.image_path = std::string(DESKTOP_APP_IMAGE_PATH);
       app.id = std::to_string(DESKTOP_APP_ID);
       app.auto_detach = true;
       app.wait_all = true;
       app.mouse_mode = 0;
       app.exit_timeout = std::chrono::seconds { 5 };
       return app;

As per coding guidelines,"Sunshine 核心 C++ 源码,自托管游戏串流服务器。审查要点:内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。"

🧰 Tools
🪛 Cppcheck (2.20.0)

[error] 76-76: Uninitialized variable

(uninitvar)

🤖 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/process.cpp` around lines 65 - 76, The function make_desktop_app
constructs a ctx_t but currently uses uninitialized scalar members (e.g.
elevated) by declaring "ctx_t app;" — change this to value-initialize the struct
(e.g. use ctx_t app{} or equivalent value initialization) so all members start
with deterministic defaults before you assign name, image_path, id, auto_detach,
wait_all, mouse_mode, and exit_timeout in make_desktop_app; this prevents UB
from reading unset fields later.

}
} // namespace

std::unique_ptr<platf::deinit_t>
init() {
return std::make_unique<deinit_t>();
Expand Down Expand Up @@ -156,17 +171,23 @@ namespace proc {
// Ensure starting from a clean slate
terminate();

auto iter = std::find_if(_apps.begin(), _apps.end(), [&app_id](const auto app) {
return app.id == std::to_string(app_id);
});
_app_id = app_id;
if (app_id == DESKTOP_APP_ID) {
_app = make_desktop_app();
}
else {
auto iter = std::find_if(_apps.begin(), _apps.end(), [&app_id](const auto app) {
return app.id == std::to_string(app_id);
});

if (iter == _apps.end()) {
BOOST_LOG(error) << "Couldn't find app with ID ["sv << app_id << ']';
return 404;
if (iter == _apps.end()) {
BOOST_LOG(error) << "Couldn't find app with ID ["sv << app_id << ']';
return 404;
}

_app = *iter;
}

_app_id = app_id;
_app = *iter;
_app_prep_begin = std::begin(_app.prep_cmds);
_app_prep_it = _app_prep_begin;

Expand Down Expand Up @@ -409,7 +430,8 @@ namespace proc {
for (const auto &app : apps) {
combined_info += app.id + app.name;
}

combined_info += std::to_string(DESKTOP_APP_ID) + std::string(DESKTOP_APP_NAME);

// Use CRC32 for the tag, same as used elsewhere
auto crc = calculate_crc32(combined_info);
_apps_etag = std::to_string(crc);
Expand Down Expand Up @@ -441,6 +463,10 @@ namespace proc {
// Returns http content-type header compatible image type.
std::string
proc_t::get_app_image(int app_id) {
if (app_id == DESKTOP_APP_ID) {
return validate_app_image_path(std::string(DESKTOP_APP_IMAGE_PATH));
}

auto iter = std::find_if(_apps.begin(), _apps.end(), [&app_id](const auto app) {
return app.id == std::to_string(app_id);
});
Expand All @@ -451,6 +477,10 @@ namespace proc {

std::string
proc_t::get_app_name(int app_id) {
if (app_id == DESKTOP_APP_ID) {
return std::string(DESKTOP_APP_NAME);
}

auto iter = std::find_if(_apps.begin(), _apps.end(), [&app_id](const auto app) {
return app.id == std::to_string(app_id);
});
Expand All @@ -459,6 +489,10 @@ namespace proc {

std::string
proc_t::get_app_cmd(int app_id) {
if (app_id == DESKTOP_APP_ID) {
return std::string();
}

auto iter = std::find_if(_apps.begin(), _apps.end(), [&app_id](const auto app) {
return app.id == std::to_string(app_id);
});
Expand Down Expand Up @@ -885,15 +919,19 @@ namespace proc {
ctx.mouse_mode = mouse_mode.value_or(0);
ctx.exit_timeout = std::chrono::seconds { exit_timeout.value_or(5) };

auto possible_ids = calculate_app_id(name, ctx.image_path, i++);
if (ids.count(std::get<0>(possible_ids)) == 0) {
// Avoid using index to generate id if possible
ctx.id = std::get<0>(possible_ids);
}
else {
// Fallback to include index on collision
ctx.id = std::get<1>(possible_ids);
}
std::tuple<std::string, std::string> possible_ids;
do {
possible_ids = calculate_app_id(name, ctx.image_path, i++);
if (ids.count(std::get<0>(possible_ids)) == 0) {
// Avoid using index to generate id if possible
ctx.id = std::get<0>(possible_ids);
}
else {
// Fallback to include index on collision
ctx.id = std::get<1>(possible_ids);
}
} while (ids.count(ctx.id) != 0 || ctx.id == std::to_string(DESKTOP_APP_ID));
Comment on lines +922 to +933
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 | ⚡ Quick win

保留 ID 命中时,这个重试循环会卡死。

如果 std::get<0>(possible_ids) 恰好等于 DESKTOP_APP_ID,这里每次都会再次选中这个保留值,因为分支里只把“已存在的 ID”当成冲突,没有把“保留的 Desktop ID”也当成冲突处理。这样 while 条件会一直成立,parse() 会在这条应用记录上无限循环。

🛠️ 建议修改
-        std::tuple<std::string, std::string> possible_ids;
+        const auto reserved_id = std::to_string(DESKTOP_APP_ID);
+        std::tuple<std::string, std::string> possible_ids;
         do {
           possible_ids = calculate_app_id(name, ctx.image_path, i++);
-          if (ids.count(std::get<0>(possible_ids)) == 0) {
+          if (std::get<0>(possible_ids) != reserved_id &&
+              ids.count(std::get<0>(possible_ids)) == 0) {
             // Avoid using index to generate id if possible
             ctx.id = std::get<0>(possible_ids);
           }
           else {
             // Fallback to include index on collision
             ctx.id = std::get<1>(possible_ids);
           }
-        } while (ids.count(ctx.id) != 0 || ctx.id == std::to_string(DESKTOP_APP_ID));
+        } while (ids.count(ctx.id) != 0 || ctx.id == reserved_id);
🤖 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/process.cpp` around lines 922 - 933, The retry loop can infinite-loop
when the non-indexed candidate equals the reserved DESKTOP_APP_ID; modify the
selection logic in the loop around calculate_app_id(name, ctx.image_path, i++)
so that you treat the reserved value as a collision: either (A) when
std::get<0>(possible_ids) == std::to_string(DESKTOP_APP_ID) force selection of
std::get<1>(possible_ids) (the indexed fallback) instead of assigning ctx.id =
std::get<0>(possible_ids), or (B) include the reserved string in the condition
that checks ids.count(...) to reject that candidate immediately; ensure ctx.id
is never set to the reserved DESKTOP_APP_ID and the loop condition
(ids.count(ctx.id) != 0 || ctx.id == std::to_string(DESKTOP_APP_ID)) can
terminate.


ids.insert(ctx.id);

ctx.name = std::move(name);
Expand Down
6 changes: 6 additions & 0 deletions src/process.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
#define __kernel_entry
#endif

#include <limits>
#include <optional>
#include <string_view>
#include <unordered_map>

#include <boost/process/v1.hpp>
Expand All @@ -21,6 +23,10 @@
namespace proc {
using file_t = util::safe_ptr_v2<FILE, int, fclose>;

inline constexpr int DESKTOP_APP_ID = std::numeric_limits<int>::max();
inline constexpr std::string_view DESKTOP_APP_NAME = "Desktop";
inline constexpr std::string_view DESKTOP_APP_IMAGE_PATH = "desktop";

typedef config::prep_cmd_t cmd_t;
struct scmd_t {
scmd_t(std::string &&id, std::string &&name, std::string &&do_cmd, bool &&elevated):
Expand Down
Loading