vrjdev commented on PR #58016: URL: https://github.com/apache/spark/pull/58016#issuecomment-5687468168
> Hi @vrjdev, I verified the mechanics against the code and the JDK behavior: > > * The reject-path tests are reliable - the `IllegalArgumentException` is thrown while reading the class descriptor in `readObject()`, before any object body is reconstructed. > * The three `...IsCurrentlyAllowed` tests correctly use `ObjectStreamClass.lookupAny(...)` > (which returns a descriptor even for non-serializable classes) plus a direct `resolveClass` call - the right technique, since Field`/`MethodHandle`/`WeakReference`can't be round-tripped. The empty-header stream in`newFilteredStream()`is sufficient because this`resolveClass`override reads nothing from the stream, only inspects`desc`. > * The comment framing the `java.lang.*` subpackage match as a pre-existing intent/impl gap from [SPARK-20922](https://issues.apache.org/jira/browse/SPARK-20922) (not introduced here) is accurate, as is the note that the JVM refuses to define any `java.`-prefixed class (so `java.langfoo.Bar` can't be spoofed the same way). > > For context, `LauncherServerSuite.testStreamFiltering()` already exercises the filter end-to-end over a socket, but it never calls `resolveClass` directly and can't reach the `java.lang.*` subpackage branch - so this direct coverage genuinely adds value rather than duplicating it. > > Also, PR description should be updated. Thanks for the detailed verification, @sarutak. Updated the PR description to reflect the current 10 tests and filed a follow-up ticket per your other comment. -- 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]
