Copilot commented on code in PR #8046: URL: https://github.com/apache/incubator-seata/pull/8046#discussion_r3050477052
########## json-common/json-common-jackson3/src/main/java/org/apache/seata/common/json/impl/Jackson3JsonSerializer.java: ########## @@ -0,0 +1,179 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.common.json.impl; + +import org.apache.seata.common.exception.JsonParseException; +import org.apache.seata.common.json.JsonAllowlistManager; +import org.apache.seata.common.json.JsonSerializer; +import org.apache.seata.common.loader.LoadLevel; +import tools.jackson.core.JacksonException; +import tools.jackson.databind.DatabindContext; +import tools.jackson.databind.DefaultTyping; +import tools.jackson.databind.DeserializationFeature; +import tools.jackson.databind.JavaType; +import tools.jackson.databind.ObjectMapper; +import tools.jackson.databind.json.JsonMapper; +import tools.jackson.databind.jsontype.PolymorphicTypeValidator; + +import java.lang.reflect.Type; +import java.util.ArrayList; +import java.util.List; + +/** + * Jackson 3.x implementation of JsonSerializer + */ +@LoadLevel(name = Jackson3JsonSerializer.NAME) +public class Jackson3JsonSerializer implements JsonSerializer { + + public static final String NAME = "jackson3"; + + private final ObjectMapper defaultObjectMapper; + + private final ObjectMapper objectMapperWithAutoType; + + public Jackson3JsonSerializer() { + this.defaultObjectMapper = JsonMapper.builder() + .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + .build(); + + AllowlistTypeValidator validator = new AllowlistTypeValidator(); + this.objectMapperWithAutoType = JsonMapper.builder() + .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + .activateDefaultTypingAsProperty(validator, DefaultTyping.NON_FINAL, "@type") + .build(); + } + + @Override + public String toJSONString(Object object) { + try { + return defaultObjectMapper.writeValueAsString(object); + } catch (JacksonException e) { + throw new JsonParseException("Jackson3 serialize error", e); + } + } + + @Override + public <T> T parseObject(String text, Class<T> clazz) { + if (text == null || clazz == null) { + return null; + } + try { + return defaultObjectMapper.readValue(text, clazz); + } catch (JacksonException e) { + throw new JsonParseException("Jackson3 deserialize error", e); + } + } + + @Override + public <T> T parseObjectWithType(String text, Type type) { + if (text == null || type == null) { + return null; + } + try { + return objectMapperWithAutoType.readValue(text, objectMapperWithAutoType.constructType(type)); + } catch (SecurityException e) { + throw e; + } catch (JacksonException e) { + rethrowIfSecurityException(e); + throw new JsonParseException("Jackson3 deserialize error", e); + } + } + + @Override + public boolean useAutoType(String json) { + return json != null && json.contains("\"@type\""); + } + + @Override + public String toJSONString(Object o, boolean prettyPrint) { + return toJSONString(o, false, prettyPrint); + } + + @Override + public String toJSONString(Object o, boolean ignoreAutoType, boolean prettyPrint) { + try { + if (o instanceof List && ((List<?>) o).isEmpty()) { + return "[]"; + } + ObjectMapper mapper = ignoreAutoType ? defaultObjectMapper : objectMapperWithAutoType; + if (prettyPrint) { + return mapper.writerWithDefaultPrettyPrinter().writeValueAsString(o); + } else { + return mapper.writeValueAsString(o); + } + } catch (JacksonException e) { + throw new JsonParseException("Jackson3 serialize error", e); + } + } + + @Override + public <T> T parseObject(String json, Class<T> type, boolean ignoreAutoType) { + if (json == null || type == null) { + return null; + } + try { + if ("[]".equals(json)) { + return (T) new ArrayList<>(0); + } Review Comment: `parseObject(String, Class, boolean)` returns an empty `ArrayList` when `json` is `"[]"` even if the requested `type` is not a collection. This can return an invalid runtime type and cause `ClassCastException` later. Only apply the `"[]"` shortcut when the target `type` is a `Collection`/`List`, otherwise deserialize normally. ########## json-common/json-common-jackson3/pom.xml: ########## @@ -0,0 +1,65 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +--> +<project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <parent> + <groupId>org.apache.seata</groupId> + <artifactId>json-common</artifactId> + <version>${revision}</version> + </parent> + <modelVersion>4.0.0</modelVersion> + <artifactId>json-common-jackson3</artifactId> + <packaging>jar</packaging> + <name>json-common-jackson3 ${project.version}</name> + <description>Jackson 3 JSON serializer for Seata (requires JDK 17+)</description> + + <properties> + <java.version>17</java.version> + <maven.compiler.source>17</maven.compiler.source> + <maven.compiler.target>17</maven.compiler.target> + </properties> + + <dependencies> + <dependency> + <groupId>${project.groupId}</groupId> + <artifactId>json-common-core</artifactId> + <version>${project.version}</version> + </dependency> + <dependency> + <groupId>tools.jackson.core</groupId> + <artifactId>jackson-databind</artifactId> + <scope>provided</scope> + <exclusions> + <exclusion> + <groupId>com.fasterxml.jackson.core</groupId> + <artifactId>jackson-annotations</artifactId> + </exclusion> + </exclusions> + </dependency> + <dependency> + <groupId>com.fasterxml.jackson.core</groupId> + <artifactId>jackson-annotations</artifactId> + <version>2.21</version> + <scope>provided</scope> + </dependency> Review Comment: `json-common-jackson3` declares `com.fasterxml.jackson.core:jackson-annotations` with a hard-coded version (`2.21`) and an exclusion that doesn’t match the `tools.jackson.*` coordinates used by Jackson 3. This bypasses the repo’s Jackson BOM-managed versions (see `dependencies/pom.xml` importing `com.fasterxml.jackson:jackson-bom` via `${jackson.version}`) and risks dependency/version conflicts. Prefer relying on the BOM (omit the explicit version) and only add exclusions/dependencies that match the actual transitive graph for `tools.jackson.core:jackson-databind`. ########## json-common/json-common-core/src/main/java/org/apache/seata/common/json/impl/Fastjson2JsonSerializer.java: ########## @@ -0,0 +1,174 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.seata.common.json.impl; + +import com.alibaba.fastjson2.JSON; +import com.alibaba.fastjson2.JSONReader; +import com.alibaba.fastjson2.JSONWriter; +import com.alibaba.fastjson2.filter.ContextAutoTypeBeforeHandler; +import com.alibaba.fastjson2.util.TypeUtils; +import org.apache.seata.common.exception.JsonParseException; +import org.apache.seata.common.json.JsonAllowlistManager; +import org.apache.seata.common.json.JsonSerializer; +import org.apache.seata.common.loader.LoadLevel; + +import java.lang.reflect.Type; + +/** + * Fastjson2 implementation of JsonSerializer + */ +@LoadLevel(name = Fastjson2JsonSerializer.NAME) +public class Fastjson2JsonSerializer implements JsonSerializer { + + public static final String NAME = "fastjson2"; + + private static final JSONWriter.Feature[] SERIALIZER_FEATURES = + new JSONWriter.Feature[] {JSONWriter.Feature.WriteClassName}; + + private static final JSONWriter.Feature[] SERIALIZER_FEATURES_PRETTY = + new JSONWriter.Feature[] {JSONWriter.Feature.WriteClassName, JSONWriter.Feature.PrettyFormat}; + + private static final JSONWriter.Feature[] FEATURES_PRETTY = + new JSONWriter.Feature[] {JSONWriter.Feature.PrettyFormat}; + + private static final AllowlistAutoTypeHandler ALLOWLIST_HANDLER = + new AllowlistAutoTypeHandler("org.apache.seata.", "io.seata."); + + @Override + public String toJSONString(Object object) { + try { + return JSON.toJSONString(object); + } catch (Exception e) { + throw new JsonParseException("Fastjson2 serialize error", e); + } + } + + @Override + public <T> T parseObject(String text, Class<T> clazz) { + if (text == null || clazz == null) { + return null; + } + try { + return JSON.parseObject(text, clazz); + } catch (Exception e) { + throw new JsonParseException("Fastjson2 deserialize error", e); + } + } + + @Override + public <T> T parseObjectWithType(String text, Type type) { + if (text == null || type == null) { + return null; + } + try { + return JSON.parseObject(text, type, ALLOWLIST_HANDLER, JSONReader.Feature.SupportAutoType); + } catch (SecurityException e) { + throw e; + } catch (Exception e) { + rethrowIfSecurityException(e); + throw new JsonParseException("Fastjson2 deserialize error", e); + } + } + + @Override + public boolean useAutoType(String json) { + return json != null && json.contains("\"@type\""); + } + + @Override + public String toJSONString(Object object, boolean prettyPrint) { + return toJSONString(object, false, prettyPrint); + } + + @Override + public String toJSONString(Object object, boolean ignoreAutoType, boolean prettyPrint) { + try { + if (prettyPrint) { + if (ignoreAutoType) { + return JSON.toJSONString(object, FEATURES_PRETTY); + } else { + return JSON.toJSONString(object, SERIALIZER_FEATURES_PRETTY); + } + } else { + if (ignoreAutoType) { + return JSON.toJSONString(object); + } else { + return JSON.toJSONString(object, SERIALIZER_FEATURES); + } + } + } catch (Exception e) { + throw new JsonParseException("Fastjson2 serialize error", e); + } + } + + @Override + public <T> T parseObject(String text, Class<T> type, boolean ignoreAutoType) { + if (text == null || type == null) { + return null; + } + try { + if ("[]".equals(text)) { Review Comment: `parseObject(String, Class, boolean)` returns `new ArrayList<>()` whenever input is exactly `"[]"`, regardless of the requested `type`. For non-collection target types this produces an invalid return value and can cause `ClassCastException`. Consider only returning an empty list when `type` is a `Collection`/`List`, otherwise parse normally. ```suggestion if ("[]".equals(text) && java.util.Collection.class.isAssignableFrom(type)) { ``` -- 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]
