Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,13 @@ private void notEntitled(String message, Class<?> callerClass) {
var exception = new NotEntitledException(message);
// Don't emit a log for muted classes, e.g. classes containing self tests
if (mutedClasses.contains(callerClass) == false) {
logger.warn(message, exception);
var moduleName = callerClass.getModule().getName();
var notEntitledLogger = LogManager.getLogger(PolicyManager.class + "." + ((moduleName == null) ? "ALL_UNNAMED" : moduleName));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we use ALL-UNNAMED (with a dash) in the policy files

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the module name and not the component name? (plugin name, "server" or "apm-agent"?)
Wouldn't we have clash for ALL-UNNAMED from different plugins if we need to adjust log level for one non-modular plugin?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The component name is also reasonable, but the finer grained we can make this the less changing this logging could affect other log messages.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having both as agreed is the best 👍 Thanks!

String frameInfoSuffix = StackWalker.getInstance(RETAIN_CLASS_REFERENCE)
.walk(this::findRequestingFrame)
.map(StackFrame::toString)
.orElse("");
notEntitledLogger.info(message + frameInfoSuffix);
}
throw exception;
}
Expand Down Expand Up @@ -658,19 +664,18 @@ Class<?> requestingClass(Class<?> callerClass) {
return callerClass;
}
Optional<Class<?>> result = StackWalker.getInstance(RETAIN_CLASS_REFERENCE)
.walk(frames -> findRequestingClass(frames.map(StackFrame::getDeclaringClass)));
.walk(frames -> findRequestingFrame(frames).map(StackFrame::getDeclaringClass));
return result.orElse(null);
}

/**
* Given a stream of classes corresponding to the frames from a {@link StackWalker},
* returns the module whose entitlements should be checked.
* Given a stream of {@link StackFrame}s, identify the one whose entitlements should be checked.
*
* @throws NullPointerException if the requesting module is {@code null}
*/
Optional<Class<?>> findRequestingClass(Stream<Class<?>> classes) {
return classes.filter(c -> c.getModule() != entitlementsModule) // Ignore the entitlements library
.skip(1) // Skip the sensitive caller method
Optional<StackFrame> findRequestingFrame(Stream<StackFrame> frames) {
return frames.filter(f -> f.getDeclaringClass().getModule() != entitlementsModule) // ignore entitlements library
.skip(1) // Skip the sensitive caller method
.findFirst();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.junit.BeforeClass;

import java.io.IOException;
import java.lang.StackWalker.StackFrame;
import java.lang.module.Configuration;
import java.lang.module.ModuleFinder;
import java.net.URL;
Expand Down Expand Up @@ -320,18 +321,21 @@ public void testRequestingModuleWithStackWalk() throws IOException, ClassNotFoun
assertEquals(
"Skip entitlement library and the instrumented method",
requestingClass,
policyManager.findRequestingClass(Stream.of(entitlementsClass, instrumentedClass, requestingClass, ignorableClass)).orElse(null)
policyManager.findRequestingFrame(
Stream.of(entitlementsClass, instrumentedClass, requestingClass, ignorableClass).map(MockFrame::new)
).map(StackFrame::getDeclaringClass).orElse(null)
);
assertEquals(
"Skip multiple library frames",
requestingClass,
policyManager.findRequestingClass(Stream.of(entitlementsClass, entitlementsClass, instrumentedClass, requestingClass))
.orElse(null)
policyManager.findRequestingFrame(
Stream.of(entitlementsClass, entitlementsClass, instrumentedClass, requestingClass).map(MockFrame::new)
).map(StackFrame::getDeclaringClass).orElse(null)
);
assertThrows(
"Non-modular caller frames are not supported",
NullPointerException.class,
() -> policyManager.findRequestingClass(Stream.of(entitlementsClass, null))
() -> policyManager.findRequestingFrame(Stream.of(entitlementsClass, null).map(MockFrame::new))
);
}

Expand Down Expand Up @@ -657,4 +661,47 @@ private static ModuleLayer createLayerForJar(Path jar, String moduleName) {
);
return moduleController.layer();
}

record MockFrame(Class<?> declaringClass) implements StackFrame {
@Override
public String getClassName() {
return getDeclaringClass().getName();
}

@Override
public String getMethodName() {
throw new UnsupportedOperationException();
}

@Override
public Class<?> getDeclaringClass() {
return declaringClass;
}

@Override
public int getByteCodeIndex() {
throw new UnsupportedOperationException();
}

@Override
public String getFileName() {
throw new UnsupportedOperationException();
}

@Override
public int getLineNumber() {
throw new UnsupportedOperationException();
}

@Override
public boolean isNativeMethod() {
throw new UnsupportedOperationException();
}

@Override
public StackTraceElement toStackTraceElement() {
throw new UnsupportedOperationException();
}
}

}