Fix crashes on any invalid json parsed by unity and bump dependencies #40
Conversation
WalkthroughUpdated qpm.json to bump beatsaverplusplus to ^0.2.3. Overhauled qpm.shared.json restored dependency graph with multiple renames, additions, version changes, and build option adjustments. Added try-catch and logging around JSON deserialization in MultiplayerStatusModelHooks.cpp, returning nullptr on exception. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Game
participant Hook as MultiplayerStatusModelHooks
participant JsonUtil as JsonUtility
Game->>Hook: Parse MultiplayerStatusData (value, info)
alt CUSTOM type
Hook->>JsonUtil: FromJson(CUSTOM)
JsonUtil-->>Hook: statusData
Hook-->>Game: statusData
else Non-CUSTOM type
rect rgba(230,240,255,0.6)
note right of Hook: New behavior: try/catch around FromJson
Hook->>JsonUtil: FromJson(non-CUSTOM)
JsonUtil-->>Hook: statusData
Hook-->>Game: statusData
end
opt Exception thrown
Hook-->>Hook: Log warning + print 40-frame backtrace
Hook-->>Game: nullptr
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Hooks/MultiplayerStatusModelHooks.cpp (1)
23-24: Null-deref risk in DEBUG prints (method_inst->type_argc).Both sites access method_inst->type_argc without null-check; will segfault when method_inst is null.
Apply:
- DEBUG("JsonConvert_DeserializeObject_MultiplayerStatusData check: ptr method_inst='{}', type_argc='{}', ptr type_argv='{}'", fmt::ptr(method_inst), method_inst->type_argc, fmt::ptr(method_inst ? method_inst->type_argv : nullptr)); + { + const auto argc = method_inst ? method_inst->type_argc : 0u; + DEBUG("JsonConvert_DeserializeObject_MultiplayerStatusData check: ptr method_inst='{}', type_argc='{}', ptr type_argv='{}'", + fmt::ptr(method_inst), argc, fmt::ptr(method_inst ? method_inst->type_argv : nullptr)); + }- DEBUG("JsonUtility_FromJson_MultiplayerStatusData check: ptr method_inst='{}', type_argc='{}', ptr type_argv='{}'", fmt::ptr(method_inst), method_inst->type_argc, fmt::ptr(method_inst ? method_inst->type_argv : nullptr)); + { + const auto argc = method_inst ? method_inst->type_argc : 0u; + DEBUG("JsonUtility_FromJson_MultiplayerStatusData check: ptr method_inst='{}', type_argc='{}', ptr type_argv='{}'", + fmt::ptr(method_inst), argc, fmt::ptr(method_inst ? method_inst->type_argv : nullptr)); + }Also applies to: 50-51
🧹 Nitpick comments (1)
src/Hooks/MultiplayerStatusModelHooks.cpp (1)
30-33: Mirror the try/catch for the Newtonsoft.Json path too.Invalid JSON via JsonConvert.DeserializeObject can crash similarly. Wrap the orig call like the Unity path.
- // call orig here, remember to pass the info parameter to your orig call! - return JsonConvert_DeserializeObject_MultiplayerStatusData(value, info); + try { + return JsonConvert_DeserializeObject_MultiplayerStatusData(value, info); + } catch (const std::exception& e) { + WARNING("JsonConvert_DeserializeObject_MultiplayerStatusData exception: {}", e.what()); + Paper::Logger::Backtrace(40); + return nullptr; + } catch (...) { + WARNING("JsonConvert_DeserializeObject_MultiplayerStatusData exception caught; returning nullptr"); + Paper::Logger::Backtrace(40); + return nullptr; + }Please confirm upstream call sites tolerate nullptr for both FromJson and DeserializeObject paths (no immediate deref).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
qpm.json(1 hunks)qpm.shared.json(5 hunks)src/Hooks/MultiplayerStatusModelHooks.cpp(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Hooks/MultiplayerStatusModelHooks.cpp (2)
src/Models/MpStatusData.cpp (1)
New(92-159)shared/Models/MpStatusData.hpp (1)
DECLARE_CLASS_CODEGEN(6-52)
⏰ 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: build
🔇 Additional comments (4)
qpm.json (1)
121-123: Approve bump of beatsaverplusplus to ^0.2.3 — qpm.json and qpm.shared.json are in sync and restore produced version 0.2.3; no further action needed.src/Hooks/MultiplayerStatusModelHooks.cpp (1)
58-66: Add empty-input guard and log std::exception::what()
- Early-return if
valueis null or empty- Catch
std::exceptionfirst to loge.what(), then fallback tocatch(...)
Verify whether IL2CPP-managed exceptions fromUnityEngine.JsonUtilitypropagate into native C++ catches in your hook environment; if not, switch to an il2cpp-aware invoke API to report errors instead of throwing through C++.qpm.shared.json (2)
122-124: Top-level beatsaverplusplus ^0.2.3 — in sync with qpm.json and restored.Looks consistent with restored version 0.2.3 below.
136-144: Restored dependencies and include paths verified. All top-level dependencies have matching restored entries; bs-cordl uses “include”, fmt uses “fmt/include/”, and paper2_scotland2 uses “shared/utfcpp/source” as expected.
2 participants
Summary by CodeRabbit
Bug Fixes
Chores