uuuyuqi opened a new issue, #16270: URL: https://github.com/apache/dubbo/issues/16270
## Pre-check - [x] I am sure that all the content I provide is in English. ## Search before asking - [x] I had searched in the [issues](https://github.com/apache/dubbo/issues?q=is%3Aissue) and found no similar issues. ## Apache Dubbo Component Java SDK (apache/dubbo) ## Dubbo Version Reproduced on Dubbo Java 3.2.16 (Spring Boot 3.2.4, OpenJDK 17.0.10, macOS 14, hessian2 default serialization). I also inspected the same code on `upstream/3.3` (HEAD `3dbba260ca`) and `upstream/3.4`: the relevant lines in `PojoUtils.realize1` and `DefaultSerializeClassChecker.loadClass` are identical, so the gap exists there as well — though I have only run the live reproduction on 3.2.16. ## Steps to reproduce this issue Define a service whose parameter type is a concrete POJO class that does **not** implement `Serializable`: ```java // API public interface GreetingService { String processRiskDto(RiskDto arg); } // DTO — intentionally NOT Serializable public class RiskDto { private String label; private Integer sequence; // standard getters / setters / no-arg constructor } ``` Provider implementation just echoes the arg. On the consumer side, invoke it generically through `GenericService`. We compare two payloads: ```java @DubboReference(interfaceClass = GreetingService.class, generic = true) private GenericService genericGreetingService; // Case A: Map without "class" key Map<String, Object> a = new LinkedHashMap<>(); a.put("label", "no-class-key"); a.put("sequence", 100); genericGreetingService.$invoke( "processRiskDto", new String[]{"com.example.api.dto.RiskDto"}, new Object[]{a}); // Case B: Map with "class" key Map<String, Object> b = new LinkedHashMap<>(); b.put("class", "com.example.api.dto.RiskDto"); b.put("label", "with-class-key"); b.put("sequence", 200); genericGreetingService.$invoke( "processRiskDto", new String[]{"com.example.api.dto.RiskDto"}, new Object[]{b}); ``` Default protocol/serialization (`dubbo` + `hessian2`), default `dubbo.application.checkSerializable` (true). ### Observed behavior - **Case A (no `class` key) — succeeds.** Provider's `processRiskDto` is invoked with a fully populated `RiskDto` instance, even though `RiskDto` does not implement `Serializable`. - **Case B (with `class` key) — fails** with the expected: ``` Caused by: java.lang.IllegalArgumentException: [Serialization Security] Serialized class com.example.api.dto.RiskDto has not implement Serializable interface. Current mode is strict check, will disallow to deserialize it by default. at org.apache.dubbo.common.utils.DefaultSerializeClassChecker.loadClass(DefaultSerializeClassChecker.java:114) at org.apache.dubbo.common.utils.PojoUtils.realize1(PojoUtils.java:460) at org.apache.dubbo.common.utils.PojoUtils.realize0(PojoUtils.java:348) at org.apache.dubbo.common.utils.PojoUtils.realize(PojoUtils.java:250) at org.apache.dubbo.common.utils.PojoUtils.realize(PojoUtils.java:130) at org.apache.dubbo.rpc.filter.GenericFilter.invoke(GenericFilter.java:120) ``` I also verified this on the provider side with Arthas: when Case A is invoked, `DefaultSerializeClassChecker.loadClass` is **never called** for `RiskDto`, while `GreetingServiceImpl.processRiskDto` is invoked and receives a real `RiskDto` instance. ## What you expected to happen For a strong-typed call (or for a generic call **with** a `class` key), Dubbo enforces the Serializable contract on user POJOs (per `DefaultSerializeClassChecker.loadClass` and `Hessian2SerializerFactory.checkSerializable`). The same DTO type behaves inconsistently depending on whether the caller happens to include a `class` entry in the generic Map: | Call style | Serializable enforced? | |---|---| | Strong-typed RPC | Yes (Hessian2 path) | | Generic call, Map **with** `class` key | Yes (`loadClass` path) | | Generic call, Map **without** `class` key | **No** | The third row looks surprising: the same non-`Serializable` DTO is rejected in two paths and silently accepted in the third. I would expect the Serializable contract to be a property of the type, not of whether the caller chose to include a `class` field. The main question I would like the maintainers to clarify is: **is this difference intentional, or is it an oversight in the check coverage?** ## Anything else ### Code analysis The check is wired only into the "load class by name" code path. In `PojoUtils.realize1`: ```java // dubbo-common/.../PojoUtils.java if (pojo instanceof Map<?, ?> && type != null) { Object className = ((Map<Object, Object>) pojo).get("class"); if (className instanceof String) { // ← gate if (!CLASS_NOT_FOUND_CACHE.containsKey(className)) { try { type = DefaultSerializeClassChecker.getInstance() .loadClass(ClassUtils.getClassLoader(), (String) className); // ← only check site } catch (ClassNotFoundException e) { CLASS_NOT_FOUND_CACHE.put((String) className, NOT_FOUND_VALUE); } } } // ... // Eventually reaches: dest = newInstance(type); // line 557 / 560 — plain reflection, no check } ``` `DefaultSerializeClassChecker.loadClass` (line 105) is the only place the Serializable check happens on this path. When the Map has no `class` entry, the `if` at line 456 short-circuits, the loader is never invoked, and `newInstance(type)` reaches `cls.getDeclaredConstructor().newInstance()` directly. The other Serializable check, `Hessian2SerializerFactory.checkSerializable` (line 74), is also bypassed in this case because in pure generic invocation the wire only carries `HashMap` + primitives — Hessian2 never sees the user POJO class on the wire and therefore never registers a serializer for it. ### Why this might be unintentional A few observations that suggest this is a coverage gap rather than a deliberate exemption: 1. The first version of the check (PR #11430, Feb 2023) placed `validateClass` **inside** the `if (className instanceof String)` block. Subsequent refactors in PR #11419 kept the same gating, suggesting the author was thinking "validate when loading by string name" rather than "validate every type that gets instantiated". 2. `DefaultSerializeClassChecker`'s class JavaDoc references Fastjson2's `ContextAutoTypeBeforeHandler` — which is an autotype-string security filter. The Serializable check appears to be a secondary concern bolted onto that filter. 3. The comment in `Hessian2SerializerFactory.checkSerializable` describes a "two-checker" model where Hessian2 and `DefaultSerializeClassChecker` jointly enforce the contract. The generic-no-`class`-key path slips between both checkers. 4. There is no unit test that pins the current behavior. In `PojoUtilsTest`, every test DTO `implements Serializable`, so the no-`class`-key + non-Serializable combination is not covered either way. ### Question for the maintainers Could you clarify whether the current behavior is intentional? Either answer is useful: - If **intentional** (e.g. types resolved from method signatures are considered trusted and exempt from this check), it would be helpful to document that, so users do not unknowingly rely on the inconsistency. - If **unintentional**, I am happy to follow up with a fix and tests. ## Are you willing to submit a pull request to fix on your own? - [x] Yes I am willing to submit a pull request on my own! ## Code of Conduct - [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct) -- 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]
