[+] SanitizeUserData #13
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Here's the translation of the review guide to Chinese: 审阅者指南 by Sourcery此拉取请求引入了一个新系统，用于对从服务器接收的用户数据进行消毒。这是为了防止由无效数据导致的崩溃。更改包括挂接网络数据包处理，以在游戏使用数据之前拦截和修改响应。此外，还添加了一个新的配置选项，用于启用或禁用此功能。 新用户数据消毒流程的序列图sequenceDiagram
participant Client
participant NetPacketHook
participant SanitizeUserData
participant Server
Client->>Server: 发送API请求
Server-->>NetPacketHook: 带有用户数据的响应
activate NetPacketHook
NetPacketHook->>SanitizeUserData: OnNetPacketComplete(api, request, response)
activate SanitizeUserData
Note over SanitizeUserData: 根据API类型消毒用户数据
SanitizeUserData-->>NetPacketHook: 返回修改后的响应
deactivate SanitizeUserData
NetPacketHook-->>Client: 消毒后的用户数据
deactivate NetPacketHook
数据消毒组件的类图classDiagram
class NetPacketHook {
+OnNetPacketComplete: event NetPacketCompleteHook
+PreProcImpl(Packet)
}
class SanitizeUserData {
+OnBeforePatch()
-OnNetPacketComplete(string, Variant, Variant)
-OnUserDataResponse(ProxyObject, ProxyObject)
-OnUserItemResponse(ProxyObject, ProxyObject)
-OnUserFavoriteResponse(ProxyObject, ProxyObject)
-FilterListResponse(string, ProxyObject, string, Func)
-SanitizeItemIdField(ProxyObject, string, string)
}
class JsonHelper {
+TryToInt32(Variant, out int)
+TryToInt64(Variant, out long)
+DeepEqual(Variant, Variant)
}
NetPacketHook --> SanitizeUserData: 通知
SanitizeUserData --> JsonHelper: 使用
数据消毒流程图graph TD
A[接收服务器响应] --> B{API是否在处理程序映射中？}
B -->|是| C[获取API类型的处理程序]
B -->|否| D[返回原始响应]
C --> E[提取请求和响应对象]
E --> F[应用数据消毒]
F --> G{数据是否被修改？}
G -->|是| H[返回修改后的响应]
G -->|否| I[返回原始响应]
subgraph 消毒类型
F --> J[验证物品ID]
F --> K[过滤无效条目]
F --> L[消毒枚举值]
F --> M[设置默认值]
end
文件级更改
提示和命令与 Sourcery 交互
自定义您的体验访问您的仪表板以：
获取帮助Original review guide in EnglishReviewer's Guide by SourceryThis pull request introduces a new system for sanitizing user data received from the server. This is done to prevent crashes caused by invalid data. The changes include hooking into the network packet processing to intercept and modify the responses before they are used by the game. Additionally, a new config option was added to enable or disable this feature. Sequence diagram for the new user data sanitization flowsequenceDiagram
participant Client
participant NetPacketHook
participant SanitizeUserData
participant Server
Client->>Server: Send API request
Server-->>NetPacketHook: Response with user data
activate NetPacketHook
NetPacketHook->>SanitizeUserData: OnNetPacketComplete(api, request, response)
activate SanitizeUserData
Note over SanitizeUserData: Sanitize user data based on API type
SanitizeUserData-->>NetPacketHook: Return modified response
deactivate SanitizeUserData
NetPacketHook-->>Client: Sanitized user data
deactivate NetPacketHook
Class diagram for the new data sanitization componentsclassDiagram
class NetPacketHook {
+OnNetPacketComplete: event NetPacketCompleteHook
+PreProcImpl(Packet)
}
class SanitizeUserData {
+OnBeforePatch()
-OnNetPacketComplete(string, Variant, Variant)
-OnUserDataResponse(ProxyObject, ProxyObject)
-OnUserItemResponse(ProxyObject, ProxyObject)
-OnUserFavoriteResponse(ProxyObject, ProxyObject)
-FilterListResponse(string, ProxyObject, string, Func)
-SanitizeItemIdField(ProxyObject, string, string)
}
class JsonHelper {
+TryToInt32(Variant, out int)
+TryToInt64(Variant, out long)
+DeepEqual(Variant, Variant)
}
NetPacketHook --> SanitizeUserData: notifies
SanitizeUserData --> JsonHelper: uses
Flow diagram for data sanitization processgraph TD
A[Receive Server Response] --> B{Is API in Handler Map?}
B -->|Yes| C[Get Handler for API Type]
B -->|No| D[Return Original Response]
C --> E[Extract Request & Response Objects]
E --> F[Apply Data Sanitization]
F --> G{Data Modified?}
G -->|Yes| H[Return Modified Response]
G -->|No| I[Return Original Response]
subgraph Sanitization Types
F --> J[Validate Item IDs]
F --> K[Filter Invalid Entries]
F --> L[Sanitize Enum Values]
F --> M[Set Default Values]
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
嘿 @Menci - 我已经审查了你的更改 - 以下是一些反馈：
整体评论：
- NetPacketHook.PreProcImpl 中的 'modified' 标志已设置，但在数据包修改逻辑中从未使用，这可能会阻止响应修改生效
以下是我在审查期间查看的内容
- 🟢 一般问题：一切看起来都很好
- 🟢 安全性：一切看起来都很好
- 🟢 测试：一切看起来都很好
- 🟢 复杂性：一切看起来都很好
- 🟢 文档：一切看起来都很好
帮助我变得更有用！请在每条评论上点击 👍 或 👎，我将使用这些反馈来改进你的评论。
Original comment in English
Hey @Menci - I've reviewed your changes - here's some feedback:
Overall Comments:
- The 'modified' flag in NetPacketHook.PreProcImpl is set but never used in the packet modification logic, which could prevent response modifications from taking effect
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Menci
force-pushed
the
sanitize-user-data
branch
from
acb54c2 to
60f3397
Compare
|
@sourcery-ai review |
There was a problem hiding this comment.
嘿 @Menci - 我已经审查了你的更改，看起来非常棒！
以下是我在审查期间查看的内容
- 🟡 一般性问题：发现2个问题
- 🟢 安全性：一切看起来都很好
- 🟢 测试：一切看起来都很好
- 🟢 复杂性：一切看起来都很好
- 🟢 文档：一切看起来都很好
帮助我变得更有用！请在每条评论上点击 👍 或 👎，我将使用这些反馈来改进你的评论。
Original comment in English
Hey @Menci - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+343
to
+349
| private static System.Type ResolveEnumType(string enumName) | ||
| { | ||
| return AppDomain.CurrentDomain | ||
| .GetAssemblies() | ||
| .SelectMany(assembly => assembly.GetTypes()) | ||
| .FirstOrDefault(type => type.FullName == enumName && type.IsEnum); | ||
| } |
There was a problem hiding this comment.
建议（性能）： 考虑缓存已解析的枚举类型，以避免重复扫描程序集
使用静态字典缓存枚举类型查找的结果，以避免重复扫描所有程序集。
Suggested change
| private static System.Type ResolveEnumType(string enumName) | |
| { | |
| return AppDomain.CurrentDomain | |
| .GetAssemblies() | |
| .SelectMany(assembly => assembly.GetTypes()) | |
| .FirstOrDefault(type => type.FullName == enumName && type.IsEnum); | |
| } | |
| private static readonly Dictionary<string, System.Type> _enumTypeCache = new Dictionary<string, System.Type>(); | |
| private static System.Type ResolveEnumType(string enumName) | |
| { | |
| // 检查类型是否已在缓存中 | |
| if (_enumTypeCache.TryGetValue(enumName, out var cachedType)) | |
| { | |
| return cachedType; | |
| } | |
| // 如果不在缓存中，则解析并添加到缓存 | |
| var resolvedType = AppDomain.CurrentDomain | |
| .GetAssemblies() | |
| .SelectMany(assembly => assembly.GetTypes()) | |
| .FirstOrDefault(type => type.FullName == enumName && type.IsEnum); | |
| if (resolvedType != null) | |
| { | |
| _enumTypeCache[enumName] = resolvedType; | |
| } | |
| return resolvedType; | |
| } |
Original comment in English
suggestion (performance): Consider caching resolved enum types to avoid repeated assembly scanning
Use a static Dictionary to cache the results of enum type lookups to avoid scanning all assemblies repeatedly.
Suggested change
| private static System.Type ResolveEnumType(string enumName) | |
| { | |
| return AppDomain.CurrentDomain | |
| .GetAssemblies() | |
| .SelectMany(assembly => assembly.GetTypes()) | |
| .FirstOrDefault(type => type.FullName == enumName && type.IsEnum); | |
| } | |
| private static readonly Dictionary<string, System.Type> _enumTypeCache = new Dictionary<string, System.Type>(); | |
| private static System.Type ResolveEnumType(string enumName) | |
| { | |
| // Check if the type is already in cache | |
| if (_enumTypeCache.TryGetValue(enumName, out var cachedType)) | |
| { | |
| return cachedType; | |
| } | |
| // If not in cache, resolve it and add to cache | |
| var resolvedType = AppDomain.CurrentDomain | |
| .GetAssemblies() | |
| .SelectMany(assembly => assembly.GetTypes()) | |
| .FirstOrDefault(type => type.FullName == enumName && type.IsEnum); | |
| if (resolvedType != null) | |
| { | |
| _enumTypeCache[enumName] = resolvedType; | |
| } | |
| return resolvedType; | |
| } |
Comment on lines
+66
to
+68
| catch (Exception e) | ||
| { | ||
| MelonLogger.Error($"[NetPacketExtension] Failed to process NetPacket: {e}"); |
There was a problem hiding this comment.
建议： 在错误日志中包含更多上下文
在错误日志中添加API名称和其他相关上下文，以帮助调试生产问题。
Suggested change
| catch (Exception e) | |
| { | |
| MelonLogger.Error($"[NetPacketExtension] Failed to process NetPacket: {e}"); | |
| catch (Exception e) | |
| { | |
| MelonLogger.Error($"[NetPacketExtension] 处理 API '{api}' 的 NetPacket 失败（响应长度：{decodedResponse?.Length ?? 0} 字节）：{e}"); |
Original comment in English
suggestion: Include more context in error logging
Add API name and other relevant context to the error log to aid in debugging production issues.
Suggested change
| catch (Exception e) | |
| { | |
| MelonLogger.Error($"[NetPacketExtension] Failed to process NetPacket: {e}"); | |
| catch (Exception e) | |
| { | |
| MelonLogger.Error($"[NetPacketExtension] Failed to process NetPacket for API '{api}' (response length: {decodedResponse?.Length ?? 0} bytes): {e}"); |
clansty
approved these changes
Jan 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
对从服务器接收的用户数据进行消毒，以防止由无效数据导致的崩溃。
新功能：
Bug 修复：
增强：
NetPacketExtension.OnNetPacketResponse替换为NetPacketHook.OnNetPacketComplete，以允许对数据包响应进行修改。Original summary in English
Summary by Sourcery
Sanitize user data received from the server to prevent crashes caused by invalid data.
New Features:
Bug Fixes:
Enhancements:
NetPacketExtension.OnNetPacketResponsewithNetPacketHook.OnNetPacketCompleteto allow modifications to the packet response.新功能：
测试：
Original summary in English
Summary by Sourcery
对从服务器接收的用户数据进行消毒，以防止由无效数据导致的崩溃。
新功能：
Bug 修复：
增强：
NetPacketExtension.OnNetPacketResponse替换为NetPacketHook.OnNetPacketComplete，以允许对数据包响应进行修改。Original summary in English
Summary by Sourcery
Sanitize user data received from the server to prevent crashes caused by invalid data.
New Features:
Bug Fixes:
Enhancements:
NetPacketExtension.OnNetPacketResponsewithNetPacketHook.OnNetPacketCompleteto allow modifications to the packet response.