dongjoon-hyun commented on PR #57768: URL: https://github.com/apache/spark/pull/57768#issuecomment-5199219898
The `LinkageError` hardening looks incomplete to me. The follow-up note above says both call sites catch it now, but the catches guard only the calls made directly at those sites — the reflective calls reached *through* `nameableSupertype` are still outside both of them: - In `canNarrowSafely`, `val target = nameableSupertype(cls)` and `isPubliclyNameable(target)` run **before** the `try`. `nameableSupertype`'s loop condition calls `c.getCanonicalName`, and `isPubliclyNameable` walks `getEnclosingClass` — both can throw `NoClassDefFoundError` when an enclosing class is not loadable (the same partial/shaded-jar scenario this PR defends against). An `Error` escaping here propagates through `referencesUnnarrowableClass` -> `active()` -> `CodeGenerator.compile` and bypasses the `NonFatal`-based codegen fallbacks. - In `rewriteQualifiedName`, `sourceNameOf(nameableSupertype(cls))` leaves the `nameableSupertype` call unprotected; `sourceNameOf` guards only its own `cls.getCanonicalName`. Before this refactor the whole sequence (`Class.forName` + `nameableSupertype` + `getCanonicalName`) sat inside `resolveSourceName`'s catch, so this path has narrower protection than it had on master. Suggestion: move the first three lines of `canNarrowSafely` into its `try`, and wrap the narrow-and-name step in `rewriteQualifiedName` with the same catch semantics (degrade to the binary name / "cannot narrow"). -- 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]
