Copilot commented on code in PR #8163:
URL: https://github.com/apache/incubator-seata/pull/8163#discussion_r3567832036
##########
extensions/rpc/seata-http/src/main/java/org/apache/seata/integration/http/AbstractHttpExecutor.java:
##########
@@ -179,14 +168,38 @@ private <K> K wrapHttpExecute(
protected abstract <K> K convertResult(HttpResponse response, Class<K>
clazz);
public static Map<String, String> convertParamOfBean(Object sourceParam) {
- return CollectionUtils.toStringMap(JSON.parseObject(
- JSON.toJSONString(
- sourceParam, SerializerFeature.WriteNullStringAsEmpty,
SerializerFeature.WriteMapNullValue),
- Map.class,
- LOCAL_CONFIG));
+ Map<String, Object> parameters =
JsonUtil.parseObject(JsonUtil.toJSONString(sourceParam), Map.class);
+ normalizeNullStringParameters(sourceParam, parameters);
+ return CollectionUtils.toStringMap(parameters);
}
public static <T> Map<String, String> convertParamOfJsonString(String
jsonStr, Class<T> returnType) {
- return convertParamOfBean(JSON.parseObject(jsonStr, returnType,
LOCAL_CONFIG));
+ return convertParamOfBean(JsonUtil.parseObject(jsonStr, returnType));
+ }
Review Comment:
convertParamOfJsonString() parses an arbitrary JSON string into a Java type
using JsonUtil.parseObject(jsonStr, returnType) without disabling auto-type.
This is user-controlled input, so it should parse with ignoreAutoType=true to
avoid unexpected polymorphic deserialization.
##########
json-common/json-common-core/src/main/java/org/apache/seata/common/json/JsonUtil.java:
##########
@@ -42,19 +43,28 @@ public final class JsonUtil {
JsonSerializerFactory.getSerializer(CONFIG_JSON_SERIALIZER_NAME);
static String resolveJsonSerializerName(Configuration configuration) {
- String serializerType =
configuration.getConfig(ConfigurationKeys.JSON_SERIALIZER_TYPE);
- if (StringUtils.isNotBlank(serializerType)) {
- return serializerType;
- }
-
- String deprecatedSerializerType =
+ String deprecatedTccSerializerType =
configuration.getConfig(ConfigurationKeys.TCC_BUSINESS_ACTION_CONTEXT_JSON_PARSER_NAME);
- if (StringUtils.isNotBlank(deprecatedSerializerType)) {
+ if (StringUtils.isNotBlank(deprecatedTccSerializerType)) {
LOGGER.warn(
"The config '{}' is deprecated since 2.7.0 and will be
removed in a future version. Please use '{}' instead.",
ConfigurationKeys.TCC_BUSINESS_ACTION_CONTEXT_JSON_PARSER_NAME,
ConfigurationKeys.JSON_SERIALIZER_TYPE);
- return deprecatedSerializerType;
+ return deprecatedTccSerializerType;
+ }
+
+ String deprecatedSagaSerializerType =
configuration.getConfig(ConfigurationKeys.CLIENT_SAGA_JSON_PARSER);
+ if (StringUtils.isNotBlank(deprecatedSagaSerializerType)) {
+ LOGGER.warn(
+ "The config '{}' is deprecated and will be removed in a
future version. Please use '{}' instead.",
+ ConfigurationKeys.CLIENT_SAGA_JSON_PARSER,
+ ConfigurationKeys.JSON_SERIALIZER_TYPE);
+ return deprecatedSagaSerializerType;
+ }
Review Comment:
The deprecation warnings say to switch to JSON_SERIALIZER_TYPE, but this
method returns the deprecated TCC/Saga keys even when the canonical key is set.
That makes migration confusing because users can set the new key and still be
overridden by the deprecated one. Consider updating the warning text to
explicitly state that the deprecated key takes precedence and should be removed
to let JSON_SERIALIZER_TYPE take effect.
##########
extensions/rpc/seata-http/src/main/java/org/apache/seata/integration/http/AbstractHttpExecutor.java:
##########
@@ -91,16 +87,9 @@ private <T> StringEntity execute(String host, String path, T
paramObject) {
String content;
if (paramObject instanceof String) {
String sParam = (String) paramObject;
- JSONObject jsonObject = null;
try {
- Object obj = JSON.parse(sParam, LOCAL_CONFIG);
- if (obj instanceof JSONObject) {
- jsonObject = (JSONObject) obj;
- } else {
- jsonObject = (JSONObject) JSON.toJSON(obj);
- }
- content = jsonObject.toJSONString();
- } catch (JSONException e) {
+ content =
JsonUtil.toJSONString(JsonUtil.parseObject(sParam, Object.class));
+ } catch (JsonParseException e) {
Review Comment:
execute() now parses a caller-provided JSON string via
JsonUtil.parseObject(sParam, Object.class) without disabling auto-type. With a
Fastjson-based serializer type, this can reintroduce unsafe polymorphic
deserialization behavior compared to the previous safe-mode ParserConfig. Since
this input is untrusted, parse with ignoreAutoType=true.
##########
discovery/seata-discovery-raft/src/main/java/org/apache/seata/discovery/registry/raft/RaftRegistryServiceImpl.java:
##########
@@ -795,7 +792,7 @@ private static void acquireClusterMetaData(String
clusterName, String group) thr
}
if (StringUtils.isNotBlank(response)) {
try {
- MetadataResponse metadataResponse =
OBJECT_MAPPER.readValue(response, MetadataResponse.class);
+ MetadataResponse metadataResponse =
JsonUtil.parseObject(response, MetadataResponse.class);
if
(CollectionUtils.isEmpty(metadataResponse.getNodes())) {
Review Comment:
acquireClusterMetaData() now parses HTTP response JSON via
JsonUtil.parseObject(response, MetadataResponse.class) without disabling
auto-type. If the configured serializer type supports polymorphic
deserialization, this makes the endpoint response a potential gadget entry
point. Parse with ignoreAutoType=true here (the metadata schema should not
require @type).
##########
discovery/seata-discovery-raft/src/main/java/org/apache/seata/discovery/registry/raft/RaftRegistryServiceImpl.java:
##########
@@ -831,13 +828,13 @@ private static void refreshToken(String tcAddress) throws
RetryableException {
if (httpResponse.code() == HttpStatus.SC_OK) {
if (httpResponse.body() != null) {
response = httpResponse.body().string();
- JsonNode jsonNode = OBJECT_MAPPER.readTree(response);
- String codeStatus = jsonNode.get("code").asText();
+ Map<String, Object> responseMap =
JsonUtil.parseObject(response, Map.class);
+ String codeStatus =
String.valueOf(responseMap.get("code"));
if (!StringUtils.equals(codeStatus, "200")) {
throw new AuthenticationFailedException(
"Authentication failed! you should
configure the correct username and password.");
}
- jwtToken = jsonNode.get("data").asText();
+ jwtToken = String.valueOf(responseMap.get("data"));
tokenTimeStamp = System.currentTimeMillis();
Review Comment:
refreshToken() parses the login response into a Map via
JsonUtil.parseObject(response, Map.class) without disabling auto-type. Even
though you only read two fields, this is still untrusted JSON from the network;
parsing with ignoreAutoType=true reduces exposure to polymorphic
deserialization issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
