Agent should not open java.lang package to unnamed module of the ap…

…plication class loader (#1334)

Co-authored-by: Evgeny Mandrikov <mandrikov@gmail.com>
Co-authored-by: Marc R. Hoffmann <hoffmann@mountainminds.com>

org.jacoco.agent.rt.test/src/org/jacoco/agent/rt/internal/AgentModuleTest.java

@@ -0,0 +1,77 @@
+/*******************************************************************************
+ * Copyright (c) 2009, 2023 Mountainminds GmbH & Co. KG and Contributors
+ * This program and the accompanying materials are made available under
+ * the terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *    Evgeny Mandrikov - initial API and implementation
+ *
+ *******************************************************************************/
+package org.jacoco.agent.rt.internal;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertSame;
+
+import org.jacoco.core.test.validation.JavaVersion;
+import org.junit.Test;
+
+/**
+ * Unit tests for {@link AgentModule}.
+ */
+public class AgentModuleTest {
+
+	@Test
+	public void isSupported_should_return_false_before_Java9() {
+		Boolean expected = Boolean
+				.valueOf(!JavaVersion.current().isBefore("9"));
+		Boolean supported = Boolean.valueOf(AgentModule.isSupported());
+		assertEquals(expected, supported);
+	}
+
+	@Test
+	public void should_only_load_classes_in_scope() throws Exception {
+		AgentModule am = new AgentModule();
+		Class<? extends Target> targetclass = am
+				.loadClassInModule(TargetImpl.class);
+		Target t = targetclass.getDeclaredConstructor().newInstance();
+
+		assertNotSame(this.getClass().getClassLoader(),
+				t.getClass().getClassLoader());
+		assertSame(t.getClass().getClassLoader(),
+				t.getInnerClassInstance().getClass().getClassLoader());
+		assertNotSame(this.getClass().getClassLoader(),
+				t.getInnerClassInstance().getClass().getClassLoader());
+		assertSame(this.getClass().getClassLoader(),
+				t.getOtherClassInstance().getClass().getClassLoader());
+	}
+
+	public interface Target {
+
+		Object getInnerClassInstance();
+
+		Object getOtherClassInstance();
+
+	}
+
+	public static class TargetImpl implements Target {
+
+		static class Inner {
+		}
+
+		public Object getInnerClassInstance() {
+			return new Inner();
+		}
+
+		public Object getOtherClassInstance() {
+			return new Other();
+		}
+	}
+
+	public static class Other {
+	}
+
+}

org.jacoco.agent.rt/src/org/jacoco/agent/rt/internal/AgentModule.java

@@ -0,0 +1,154 @@
+/*******************************************************************************
+ * Copyright (c) 2009, 2023 Mountainminds GmbH & Co. KG and Contributors
+ * This program and the accompanying materials are made available under
+ * the terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *    Evgeny Mandrikov - initial API and implementation
+ *    Marc R. Hoffmann - move to separate class
+ *
+ *******************************************************************************/
+package org.jacoco.agent.rt.internal;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.instrument.Instrumentation;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import org.jacoco.core.internal.InputStreams;
+
+/**
+ * An isolated class loader and distinct module to encapsulate JaCoCo runtime
+ * classes. This isolated environment allows to specifically open JDK packages
+ * to the agent runtime without changing package accessibility for the
+ * application under test.
+ * <p>
+ * The implementation uses the property that the <a href=
+ * "https://docs.oracle.com/javase/specs/jvms/se11/html/jvms-5.html#jvms-5.3.6">
+ * unnamed module is distinct from all run-time modules (including unnamed
+ * modules) bound to other class loaders</a>.
+ */
+public class AgentModule {
+
+	/**
+	 * Checks whether Java modules are supported by the current Java runtime.
+	 *
+	 * @return <code>true</code> is modules are supported
+	 */
+	public static boolean isSupported() {
+		try {
+			getModuleClass();
+		} catch (final ClassNotFoundException e) {
+			return false;
+		}
+		return true;
+	}
+
+	private final Set<String> scope = new HashSet<String>();
+	private final ClassLoader classLoader;
+
+	/**
+	 * Creates a new isolated module.
+	 *
+	 * @throws Exception
+	 *             if it cannot be created
+	 */
+	public AgentModule() throws Exception {
+		classLoader = new ClassLoader() {
+			@Override
+			protected Class<?> loadClass(final String name,
+					final boolean resolve) throws ClassNotFoundException {
+				if (!scope.contains(name)) {
+					return super.loadClass(name, resolve);
+				}
+				final InputStream resourceAsStream = getResourceAsStream(
+						name.replace('.', '/') + ".class");
+				final byte[] bytes;
+				try {
+					bytes = InputStreams.readFully(resourceAsStream);
+				} catch (final IOException e) {
+					throw new RuntimeException(e);
+				}
+				return defineClass(name, bytes, 0, bytes.length);
+			}
+		};
+	}
+
+	/**
+	 * Opens the package of the provided class to the module created in this
+	 * {@link #AgentModule()} instance.
+	 *
+	 * @param instrumentation
+	 *            service provided to the agent by the Java runtime
+	 * @param classInPackage
+	 *            example class of the package to open
+	 * @throws Exception
+	 *             if package cannot be opened
+	 */
+	public void openPackage(final Instrumentation instrumentation,
+			final Class<?> classInPackage) throws Exception {
+
+		// module of the package to open
+		final Object module = Class.class.getMethod("getModule")
+				.invoke(classInPackage);
+
+		// unnamed module of our classloader
+		final Object unnamedModule = ClassLoader.class
+				.getMethod("getUnnamedModule").invoke(classLoader);
+
+		// Open package java.lang to the unnamed module of our class loader
+		Instrumentation.class.getMethod("redefineModule", //
+				getModuleClass(), //
+				Set.class, //
+				Map.class, //
+				Map.class, //
+				Set.class, //
+				Map.class //
+		).invoke(instrumentation, // instance
+				module, // module
+				Collections.emptySet(), // extraReads
+				Collections.emptyMap(), // extraExports
+				Collections.singletonMap(classInPackage.getPackage().getName(),
+						Collections.singleton(unnamedModule)), // extraOpens
+				Collections.emptySet(), // extraUses
+				Collections.emptyMap() // extraProvides
+		);
+	}
+
+	/**
+	 * Loads a copy of the given class in the isolated classloader. Also any
+	 * inner classes are loader from the isolated classloader.
+	 *
+	 * @param <T>
+	 *            type of the class to load
+	 * @param original
+	 *            original class definition which is used as source
+	 * @return class object from the isolated class loader
+	 * @throws Exception
+	 *             if the class cannot be loaded
+	 */
+	@SuppressWarnings("unchecked")
+	public <T> Class<T> loadClassInModule(final Class<T> original)
+			throws Exception {
+		addToScopeWithInnerClasses(original);
+		return (Class<T>) classLoader.loadClass(original.getName());
+	}
+
+	private void addToScopeWithInnerClasses(final Class<?> c) {
+		scope.add(c.getName());
+		for (final Class<?> i : c.getDeclaredClasses()) {
+			addToScopeWithInnerClasses(i);
+		}
+	}
+
+	private static Class<?> getModuleClass() throws ClassNotFoundException {
+		return Class.forName("java.lang.Module");
+	}
+
+}

org.jacoco.agent.rt/src/org/jacoco/agent/rt/internal/PreMain.java

@@ -13,9 +13,6 @@
 package org.jacoco.agent.rt.internal;
 
 import java.lang.instrument.Instrumentation;
-import java.util.Collections;
-import java.util.Map;
-import java.util.Set;
 
 import org.jacoco.core.runtime.AgentOptions;
 import org.jacoco.core.runtime.IRuntime;
@@ -58,58 +55,17 @@ public static void premain(final String options, final Instrumentation inst)
 	private static IRuntime createRuntime(final Instrumentation inst)
 			throws Exception {
 
-		if (redefineJavaBaseModule(inst)) {
-			return new InjectedClassRuntime(Object.class, "$JaCoCo");
+		if (AgentModule.isSupported()) {
+			final AgentModule module = new AgentModule();
+			module.openPackage(inst, Object.class);
+			final Class<InjectedClassRuntime> clazz = module
+					.loadClassInModule(InjectedClassRuntime.class);
+			return clazz.getConstructor(Class.class, String.class)
+					.newInstance(Object.class, "$JaCoCo");
 		}
 
 		return ModifiedSystemClassRuntime.createFor(inst,
 				"java/lang/UnknownError");
 	}
 
-	/**
-	 * Opens {@code java.base} module for {@link InjectedClassRuntime} when
-	 * executed on Java 9 JREs or higher.
-	 *
-	 * @return <code>true</code> when running on Java 9 or higher,
-	 *         <code>false</code> otherwise
-	 * @throws Exception
-	 *             if unable to open
-	 */
-	private static boolean redefineJavaBaseModule(
-			final Instrumentation instrumentation) throws Exception {
-		try {
-			Class.forName("java.lang.Module");
-		} catch (final ClassNotFoundException e) {
-			return false;
-		}
-
-		Instrumentation.class.getMethod("redefineModule", //
-				Class.forName("java.lang.Module"), //
-				Set.class, //
-				Map.class, //
-				Map.class, //
-				Set.class, //
-				Map.class //
-		).invoke(instrumentation, // instance
-				getModule(Object.class), // module
-				Collections.emptySet(), // extraReads
-				Collections.emptyMap(), // extraExports
-				Collections.singletonMap("java.lang",
-						Collections.singleton(
-								getModule(InjectedClassRuntime.class))), // extraOpens
-				Collections.emptySet(), // extraUses
-				Collections.emptyMap() // extraProvides
-		);
-		return true;
-	}
-
-	/**
-	 * @return {@code cls.getModule()}
-	 */
-	private static Object getModule(final Class<?> cls) throws Exception {
-		return Class.class //
-				.getMethod("getModule") //
-				.invoke(cls);
-	}
-
 }

org.jacoco.ant.test/src/org/jacoco/ant/CoverageTaskTest.xml

@@ -158,4 +158,12 @@
 		<au:assertLogContains text="java/sql/Timestamp"/>
 	</target>
 
+	<target name="testIllegalReflectiveAccess">
+		<jacoco:coverage destfile="${exec.file}">
+			<java classname="org.jacoco.ant.IllegalReflectiveAccessTarget" fork="true" failonerror="true">
+				<classpath path="${org.jacoco.ant.coverageTaskTest.classes.dir}"/>
+			</java>
+		</jacoco:coverage>
+	</target>
+
 </project>

org.jacoco.ant.test/src/org/jacoco/ant/IllegalReflectiveAccessTarget.java

@@ -0,0 +1,42 @@
+/*******************************************************************************
+ * Copyright (c) 2009, 2023 Mountainminds GmbH & Co. KG and Contributors
+ * This program and the accompanying materials are made available under
+ * the terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *    Evgeny Mandrikov - initial API and implementation
+ *
+ *******************************************************************************/
+
+package org.jacoco.ant;
+
+import java.lang.reflect.Constructor;
+
+public class IllegalReflectiveAccessTarget {
+
+	public static void main(String[] args) throws Exception {
+		try {
+			Class.forName("java.net.UnixDomainSocketAddress");
+		} catch (ClassNotFoundException e) {
+			// Java < 16
+			return;
+		}
+
+		final Constructor<?> c = Class.forName("java.lang.Module")
+				.getDeclaredConstructors()[0];
+		try {
+			c.setAccessible(true);
+			throw new AssertionError("Exception expected");
+		} catch (RuntimeException e) {
+			if (!e.getClass().getName()
+					.equals("java.lang.reflect.InaccessibleObjectException")) {
+				throw new AssertionError(
+						"InaccessibleObjectException expected");
+			}
+		}
+	}
+
+}