LuciferYang opened a new issue, #7697:
URL: https://github.com/apache/kyuubi/issues/7697
### Description
`DynConstructors.builder()` creates a `Builder` whose `baseClass` is null,
and the zero-argument overload forwards that field:
```java
public Builder hiddenImpl(Class<?>... types) {
hiddenImpl(baseClass, types);
return this;
}
```
`hiddenImpl(Class<T> targetClass, Class<?>... types)` then calls
`targetClass.getDeclaredConstructor(types)` and catches only
`SecurityException` and `NoSuchMethodException`, so the null dereference leaves
the builder chain as a bare NPE:
```
java.lang.NullPointerException: Cannot invoke
"java.lang.Class.getDeclaredConstructor(java.lang.Class[])" because
"targetClass" is null
```
The message does not mention the missing base class.
A loose `Class` argument goes elsewhere: `builder().hiddenImpl(Foo.class)`
binds `hiddenImpl(Class, Class...)` with `Foo` as the target class, and a miss
there is recorded against `Foo` rather than against the base class. What does
reach it is a call with no arguments, or one passing an explicit `Class<?>[]`,
which is also what a Scala `: _*` splat compiles to and is the call shape
`ReflectUtils` already uses against the `DynMethods` builder. So
`builder(Foo.class).hiddenImpl(new Class<?>[] {String.class})` does find
`Foo(String)`, while the same call on a class-less builder throws.
No caller in Kyuubi reaches it. The zero-argument `hiddenImpl()` has no use
in the repository, and the twenty-odd production sites that call
`DynConstructors.builder()` without a class all continue with one of the `impl`
overloads, naming either a class or a class name. So this is a latent defect in
a retained upstream API rather than a failure anyone is hitting.
### Affected code
`DynConstructors.Builder.hiddenImpl(Class<?>... types)`. The `String` and
`Class<T>` overloads name their own target class. Handing an explicit null to
those, as in `hiddenImpl((Class<?>) null, String.class)`, also throws NPE, but
that is a caller supplying null rather than the builder forwarding its own
unset field.
### Affects Version(s)
master (1.13.0-SNAPSHOT), 1.12, 1.11, 1.10, 1.9. All carry the overload
unchanged.
### Upstream
iceberg-common deprecated this overload in 1.6.0, with "This varargs method
conflicts with `hiddenImpl(Class, Class...)`. Use `builder(Class)` instead.",
and removed it in 1.7.0 (commit `d17a7f189`, apache/iceberg#10818, the same
commit that removed the `ctorImpl` overloads behind #7688). The class-less
`builder()` stayed; only the overload that read `baseClass` went away. There is
no newer upstream release to sync a fix from, and dropping the overload is
upstream's own answer to the ambiguity that produces this NPE. Note that
dropping it here would not be free: a caller passing an explicit array, or a
Scala splat, binds to this overload today.
### Are you willing to submit PR?
Yes.
--
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]
