jbonofre commented on code in PR #2870:
URL: https://github.com/apache/karaf/pull/2870#discussion_r3975477753
##########
services/interceptor/impl/src/main/java/org/apache/karaf/service/interceptor/impl/runtime/proxy/AsmProxyFactory.java:
##########
@@ -114,19 +113,22 @@ private boolean hasSameSignature(Method a, Method b) {
private void createConstructor(final ClassWriter cw, final String
proxyClassFileName, final Class<?> classToProxy,
final String classFileName) {
- Constructor superDefaultCt;
- String parentClassFileName = classFileName;
- String descriptor = "()V";
-
- try {
- if (classToProxy.isInterface()) {
- parentClassFileName = Type.getInternalName(Object.class);
- superDefaultCt = Object.class.getConstructor(null);
- descriptor = Type.getConstructorDescriptor(superDefaultCt);
+ // the proxy extends the proxied class, or Object when proxying an
interface; either way
+ // the super constructor it invokes is the no-arg one
+ final String parentClassFileName;
+ if (classToProxy.isInterface()) {
+ parentClassFileName = Type.getInternalName(Object.class);
+ } else {
+ parentClassFileName = classFileName;
+ try {
+ classToProxy.getDeclaredConstructor();
Review Comment:
`getDeclaredConstructor()` also matches a private or package-private no-arg
constructor, but the `INVOKESPECIAL` we generate below can't call either one.
So far those classes the check passes and we still fail late, which is exactly
what this guard is meant to prevent.
Package-private doesn't help here either: the proxy is define by a fresh
`ProxyFactory.ProxyClassLoader`, so it ends up in a different runtime package
than the proxied class even though the package name matches.
`protected` does work (super-constructor access from a direct subclass is
legal across packages), so the condition we actually want is "public or
protected", not "declared". Concrete case that regresses today: a service impl
registered via `bundleContext.registerService(...)` with a package-private
no-arg constructor (a pretty normal idiom) gets an
`IllegalStateException`/`IllegalAccessError` instead of the clear
`IllegalArgumentException`.
The simplest fix is `getConstructor()`, which is public-only and also avoids
needing `accessDeclaredMembers` under a security manager:
```java
classToProxy.getConstructor();
```
If you want to keep `protected` proxyable, keep `getDeclaredConstructor()`
and reject on the modifiers:
```java
final Constructor<?> superCt = classToProxy.getDeclaredConstructor();
final int modifiers = superCt.getModifiers();
if (!Modifier.isPublic(modifiers) && !Modifier.isProtected(modifiers)) {
throw new IllegalArgumentException("Cannot proxy " +
classToProxy.getName() + ", its no-arg constructor is not accessible from the
generated proxy");
}
```
Maybe worth a test alongside `proxyWithoutNoArgConstructor` covering a class
whose only no-arg constructor is package-private.
--
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]
