Copilot commented on code in PR #7973:
URL: https://github.com/apache/incubator-seata/pull/7973#discussion_r2928403472
##########
json-common/src/main/java/org/apache/seata/common/json/impl/FastjsonJsonSerializer.java:
##########
@@ -132,13 +138,54 @@ public <T> T parseObject(String text, Class<T> type,
boolean ignoreAutoType) {
if ("[]".equals(text)) {
return (T) new java.util.ArrayList<>();
}
+
+ // Check allowlist when AutoType is enabled
+ if (!ignoreAutoType && useAutoType(text)) {
+ checkAutoTypeClasses(text);
+ }
+
if (ignoreAutoType) {
return JSON.parseObject(text, type,
READER_FEATURES_IGNORE_AUTO_TYPE);
} else {
return JSON.parseObject(text, type,
READER_FEATURES_SUPPORT_AUTO_TYPE);
}
+ } catch (SecurityException e) {
+ throw e;
} catch (Exception e) {
throw new JsonParseException("FastJSON deserialize error", e);
}
}
+
+ /**
+ * Check all @type classes in JSON against allowlist
+ */
+ private void checkAutoTypeClasses(String json) {
+ int index = 0;
+ while ((index = json.indexOf("\"@type\"", index)) >= 0) {
+ String className = extractTypeValue(json, index);
+ if (className != null) {
+ JsonAllowlistManager.getInstance().checkClass(className);
+ }
+ index++;
+ }
+ }
+
+ /**
+ * Extract @type value from JSON string
+ */
+ private String extractTypeValue(String json, int typeIndex) {
+ int colonIndex = json.indexOf(':', typeIndex);
+ if (colonIndex < 0) {
+ return null;
+ }
+ int startQuote = json.indexOf('"', colonIndex);
+ if (startQuote < 0) {
+ return null;
+ }
+ int endQuote = json.indexOf('"', startQuote + 1);
+ if (endQuote < 0) {
+ return null;
+ }
+ return json.substring(startQuote + 1, endQuote);
+ }
Review Comment:
Like the Jackson implementation, this
`checkAutoTypeClasses`/`extractTypeValue` approach is based on raw
`String#indexOf` and is not JSON-token-aware. It can incorrectly interpret
occurrences of "@type" inside string values (or non-string `@type` values) and
block parsing with a `SecurityException` even though Fastjson wouldn’t treat
those as AutoType metadata. Suggest parsing in a safe mode that treats `@type`
as a normal key (e.g., disable special key detect) and then traversing the
parsed structure to validate only real `@type` fields with string values.
##########
json-common/src/main/java/org/apache/seata/common/json/impl/JacksonJsonSerializer.java:
##########
@@ -147,13 +153,54 @@ public <T> T parseObject(String json, Class<T> type,
boolean ignoreAutoType) {
if ("[]".equals(json)) {
return (T) new ArrayList<>(0);
}
+
+ // Check allowlist when AutoType is enabled
+ if (!ignoreAutoType && useAutoType(json)) {
+ checkAutoTypeClasses(json);
+ }
+
if (ignoreAutoType) {
return defaultObjectMapper.readValue(json, type);
} else {
return objectMapperWithAutoType.readValue(json, type);
}
+ } catch (SecurityException e) {
+ throw e;
} catch (IOException e) {
throw new JsonParseException("Jackson deserialize error", e);
}
}
+
+ /**
+ * Check all @type classes in JSON against allowlist
+ */
+ private void checkAutoTypeClasses(String json) {
+ int index = 0;
+ while ((index = json.indexOf("\"@type\"", index)) >= 0) {
+ String className = extractTypeValue(json, index);
+ if (className != null) {
+ JsonAllowlistManager.getInstance().checkClass(className);
+ }
+ index++;
+ }
+ }
+
+ /**
+ * Extract @type value from JSON string
+ */
+ private String extractTypeValue(String json, int typeIndex) {
+ int colonIndex = json.indexOf(':', typeIndex);
+ if (colonIndex < 0) {
+ return null;
+ }
+ int startQuote = json.indexOf('"', colonIndex);
+ if (startQuote < 0) {
+ return null;
+ }
+ int endQuote = json.indexOf('"', startQuote + 1);
+ if (endQuote < 0) {
+ return null;
+ }
+ return json.substring(startQuote + 1, endQuote);
+ }
Review Comment:
`checkAutoTypeClasses` scans the raw JSON text for the substring "\"@type\""
and then extracts the next quoted string after the next ':' to decide what to
validate. This is not JSON-token-aware, so it can mis-detect "@type"
occurrences inside string values (or when `@type` is not a string value) and
throw `SecurityException` for benign payloads. Consider switching to a proper
JSON token scan (e.g., Jackson streaming parser) that only treats a field name
token equal to `@type` as type metadata, and only validates when the
corresponding value token is a string.
##########
seata-spring-autoconfigure/seata-spring-autoconfigure-client/src/main/java/org/apache/seata/spring/boot/autoconfigure/properties/SeataJsonProperties.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * 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.spring.boot.autoconfigure.properties;
+
+import org.apache.seata.common.json.JsonAllowlistManager;
+import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.stereotype.Component;
+
+import javax.annotation.PostConstruct;
+
+import static
org.apache.seata.spring.boot.autoconfigure.StarterConstants.JSON_PREFIX;
+
+/**
+ * Seata JSON configuration properties
+ */
+@Component
+@ConfigurationProperties(prefix = JSON_PREFIX)
+public class SeataJsonProperties {
+
+ /**
+ * JSON deserialization allowlist, comma-separated
+ * Entries ending with '.' are prefix matches, otherwise exact matches
+ * Example: com.company.model.,com.company.dto.,com.company.SomeClass
+ */
+ private String allowlist;
+
+ public String getAllowlist() {
+ return allowlist;
+ }
+
+ public SeataJsonProperties setAllowlist(String allowlist) {
+ this.allowlist = allowlist;
+ return this;
+ }
+
+ @PostConstruct
+ public void init() {
+ if (allowlist != null && !allowlist.isEmpty()) {
+ JsonAllowlistManager.getInstance().loadUserAllowlist(allowlist);
+ }
Review Comment:
`init()` only calls `loadUserAllowlist` when `allowlist` is non-empty. Since
`JsonAllowlistManager` is a JVM-wide singleton, this can leave stale user
allowlist entries in place across Spring context refreshes / test suites when
the property becomes unset/empty. Consider always delegating to
`loadUserAllowlist(allowlist)` (it already clears on null/empty) so an
empty/missing config reliably resets to the built-in allowlist only.
--
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]
