-
Notifications
You must be signed in to change notification settings - Fork 329
[Dependency] Upgrade fastjson to fastjson2 (v2.0.60) #394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Migrate from fastjson 1.2.83 to fastjson2 2.0.60 for improved performance and security. - Add fastjson2 dependency version management in framework/dependency/pom.xml - Update fit-value-fastjson plugin to use fastjson2 artifact - Update JSONPath import from com.alibaba.fastjson to com.alibaba.fastjson2 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
修改 HttpToolTest 测试类,将 Tool.execute() 的字符串参数改为使用 ObjectSerializer 反序列化后的 Map 对象,避免依赖 fastjson 的自动 JSON 解析行为。 同时在 ValueFetcher 和 ValueSetter 的 JavaDoc 中明确说明:当参数为 String 时, 默认按普通字符串处理,不会自动解析 JSON 文本。如需从 JSON 字符串中取值或设置属性, 调用方应先使用 ObjectSerializer 将其反序列化为对象。 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
📊 专业 PR 审查报告审查方式: 5 个专业代理并行深度审查 (code-reviewer, pr-test-analyzer, comment-analyzer, silent-failure-hunter, code-simplifier) 🔴 关键问题(必须修复)① 静默失败风险 - FastJsonValueHandler 缺少错误处理 (严重性: CRITICAL)问题位置: 问题描述:
影响:
修复建议: @Override
public Object fetch(Object object, String propertyPath) {
if (object == null) {
return null;
}
if (StringUtils.isBlank(propertyPath)) {
return object;
}
try {
String parsedPath = this.getParsedPath(propertyPath);
return JSONPath.eval(object, parsedPath);
} catch (com.alibaba.fastjson2.JSONException e) {
throw new IllegalArgumentException(
StringUtils.format("Invalid JSONPath expression: {0}", propertyPath), e);
}
}
@Override
public Object set(Object object, String propertyPath, Object value) {
if (object == null) {
return null;
}
if (StringUtils.isBlank(propertyPath)) {
return value;
}
try {
String parsedPath = this.getParsedPath(propertyPath);
boolean success = JSONPath.set(object, parsedPath, value);
if (!success) {
throw new IllegalStateException(
StringUtils.format("Failed to set value at JSONPath: {0}", propertyPath));
}
return object;
} catch (com.alibaba.fastjson2.JSONException e) {
throw new IllegalArgumentException(
StringUtils.format("Invalid JSONPath expression: {0}", propertyPath), e);
}
}② 缺少关键测试 - JSON 字符串自动解析防护测试 (严重性: 9/10)问题位置: 问题描述:
缺失的测试 (添加到 @Test
@DisplayName("当 object 为 JSON 字符串时,不应自动解析")
void shouldNotAutoParseJsonString() {
ValueFetcher fetcher = new FastJsonValueHandler();
String jsonString = "{\"key\":\"value\"}";
// 应当作普通字符串处理,不解析
Object result = fetcher.fetch(jsonString, "key");
assertThat(result).isNull(); // 证明未自动解析
// 获取空路径应返回字符串本身
Object selfResult = fetcher.fetch(jsonString, "");
assertThat(selfResult).isEqualTo(jsonString);
}类似测试添加到 @Test
@DisplayName("当 object 为 JSON 字符串时,不应自动解析")
void shouldNotAutoParseJsonStringInSetter() {
ValueSetter setter = new FastJsonValueHandler();
String jsonString = "{\"key\":\"value\"}";
// 不应修改字符串
Object result = setter.set(jsonString, "key", "newValue");
assertThat(result).isEqualTo(jsonString); // 未变化
}🟠 重要问题(强烈建议修复)③ 测试覆盖不足 - 缺少 Tool.execute() JSON 字符串拒绝测试 (严重性: 8/10)问题位置: 问题描述: 建议添加测试: @Test
@DisplayName("当直接传入 JSON 字符串时,应无法正确处理参数")
void shouldNotAcceptJsonStringDirectly() {
Tool.Info info = readToolInfo("basic-auth.json");
Tool tool = createTool(info);
// 文档化迁移行为
assertThatThrownBy(() -> tool.execute("{\"name\":\"testuser\", \"pwd\":\"testpass\"}"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Expected Map but got String");
}④ 测试代码错误处理缺失 (严重性: HIGH)问题位置: 问题描述: 修复建议: private static Map<String, Object> deserializeJsonObject(String json) {
ObjectSerializer jsonSerializer = new JacksonObjectSerializer(null, null, null, true);
Type objectType = TypeUtils.parameterized(Map.class, new Type[] {String.class, Object.class});
try {
return jsonSerializer.deserialize(json, objectType);
} catch (SerializationException e) {
throw new IllegalStateException(
StringUtils.format("Failed to deserialize test JSON: {0}", json), e);
}
}🟡 建议改进(可选)⑤ 文档改进 - 添加版本上下文问题位置: 建议: JavaDoc 中添加 "自 fastjson2 升级后" 的上下文说明: /**
* <p><strong>注意(自 fastjson2 升级后):</strong>当 {@code object} 为 {@link String} 时,
* 默认按普通字符串处理,不会自动解析 JSON 文本...
*/⑥ 代码简化 - 提取共享的 ObjectSerializer问题位置: 两处创建相同的 private static final ObjectSerializer JSON_SERIALIZER =
new JacksonObjectSerializer(null, null, null, true);⑦ 代码简化 - 移除不必要的 else问题位置: 移除 ✅ 积极发现
📋 行动计划🔴 合并前必须完成
🟠 强烈建议
🟡 可选优化
📊 审查统计
🎯 最终建议状态: 合并条件:
理由:
预计修复时间: 2-3 小时(添加错误处理 + 2 个测试) 修复后,这将是一个高质量的依赖升级 PR!👍 🤖 此审查报告由 Claude Code PR Review Toolkit 自动生成(5 个专业审查代理) 更正说明: 此评论修正了之前版本中使用 |
第5点可以不处理 |
🔗 相关问题 / Related Issue
📋 变更类型 / Type of Change
📝 变更目的 / Purpose of the Change
升级 fastjson 到 fastjson2 (v2.0.60),解决安全漏洞并获取性能改进。fastjson2 是阿里巴巴官方推荐的升级版本,提供更好的性能和安全性。
📋 主要变更 / Brief Changelog
framework/dependency/pom.xml中的 fastjson 依赖替换为 fastjson2 (v2.0.60)fit-value-fastjson插件中的包引用从com.alibaba.fastjson到com.alibaba.fastjson2HttpToolTest测试类中的 JSON 字符串处理方式,使用ObjectSerializer显式反序列化ValueFetcher和ValueSetterAPI 文档,明确 String 参数的处理规则🧪 验证变更 / Verifying this Change
测试步骤 / Test Steps
./.claude/run-test.shmvn clean install执行成功fit-value-fastjson插件测试覆盖 / Test Coverage
📸 截图 / Screenshots
测试流程执行结果:
✅ 贡献者检查清单 / Contributor Checklist
基本要求 / Basic Requirements:
代码质量 / Code Quality:
测试要求 / Testing Requirements:
mvn -B clean package -Dmaven.test.skip=true/ Basic checks passmvn clean install/ Unit tests pass文档和兼容性 / Documentation and Compatibility:
📋 附加信息 / Additional Notes
升级说明:
com.alibaba.fastjson→com.alibaba.fastjson2重要变更:
ValueFetcher和ValueSetter接口的行为保持不变,但现在在 JavaDoc 中明确说明:当参数为 String 时,默认按普通字符串处理,不会自动解析 JSON 文本ObjectSerializer将其反序列化为对象审查者注意事项 / Reviewer Notes:
这是一个重要的依赖升级,建议审查者:
fit-value-fastjson插件功能无异常HttpToolTest测试类的修改是否合理🤖 Generated with Claude Code