Try to get rid of legacy API which can break starting with java 17 (#409

)


Co-authored-by: Slawomir Jaranowski <[email protected]>

pom.xml

@@ -125,6 +125,7 @@
     <mavenVersion>3.6.3</mavenVersion>
     <recommendedJavaBuildVersion>11</recommendedJavaBuildVersion>
     <slf4j.version>1.7.36</slf4j.version>
+    <asm.version>9.6</asm.version>
     <invoker.parallelThreads>1C</invoker.parallelThreads>
     <project.build.outputTimestamp>2023-11-17T00:21:18Z</project.build.outputTimestamp>
   </properties>
@@ -193,6 +194,17 @@
       <version>1.4.0</version>
     </dependency>
 
+    <dependency>
+      <groupId>org.ow2.asm</groupId>
+      <artifactId>asm</artifactId>
+      <version>${asm.version}</version>
+    </dependency>
+    <dependency>
+      <groupId>org.ow2.asm</groupId>
+      <artifactId>asm-commons</artifactId>
+      <version>${asm.version}</version>
+    </dependency>
+
     <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>
@@ -354,15 +366,6 @@
         </plugins>
       </build>
     </profile>
-    <profile>
-      <id>java17+</id>
-      <activation>
-        <jdk>[17,)</jdk>
-      </activation>
-      <properties>
-        <invoker.security.manager>-Djava.security.manager=allow</invoker.security.manager>
-      </properties>
-    </profile>
     <profile>
       <id>run-its</id>
       <build>
@@ -388,11 +391,6 @@
               <scriptVariables>
                 <projectVersion>${project.version}</projectVersion>
               </scriptVariables>
-              <!--
-                Necessary on JDK 17+ for ITs "mexec-gh-389-block-exit-*" to avoid "UnsupportedOperationException:
-                The Security Manager is deprecated and will be removed in a future release". See profile 'java17+'.
-              -->
-              <mavenOpts>${invoker.security.manager}</mavenOpts>
             </configuration>
             <executions>
               <execution>

src/it/projects/mexec-gh-389-block-exit-non-zero/verify.groovy

@@ -24,4 +24,4 @@ assert buildLogLines[infoMessageLineNumber - 1] == "[one, two, three]"
 // Verify that subsequent lines contain the beginning of the thrown SystemExitException stack trace
 assert buildLogLines[infoMessageLineNumber + 1].startsWith("[WARNING]")
 assert buildLogLines[infoMessageLineNumber + 2].contains("SystemExitException: System::exit was called with return code 123")
-assert buildLogLines[infoMessageLineNumber + 3].contains("SystemExitManager.checkExit (SystemExitManager.java")
+assert buildLogLines[infoMessageLineNumber + 3].contains("SystemExitManager.exit (SystemExitManager.java")

src/main/java/org/codehaus/mojo/exec/BlockExitTransformer.java

@@ -0,0 +1,91 @@
+package org.codehaus.mojo.exec;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.lang.instrument.ClassFileTransformer;
+import java.security.ProtectionDomain;
+
+import org.objectweb.asm.ClassReader;
+import org.objectweb.asm.ClassVisitor;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.commons.GeneratorAdapter;
+
+import static org.objectweb.asm.ClassReader.EXPAND_FRAMES;
+import static org.objectweb.asm.ClassWriter.COMPUTE_FRAMES;
+import static org.objectweb.asm.Opcodes.ASM9;
+
+public class BlockExitTransformer implements ClassFileTransformer {
+    @Override
+    public byte[] transform(
+            final ClassLoader loader,
+            final String className,
+            final Class<?> classBeingRedefined,
+            final ProtectionDomain protectionDomain,
+            final byte[] classfileBuffer) {
+        try {
+            final ClassReader reader = new ClassReader(classfileBuffer);
+            final ClassWriter writer = new ClassWriter(COMPUTE_FRAMES);
+            final SystemExitOverrideVisitor visitor = new SystemExitOverrideVisitor(writer);
+            reader.accept(visitor, EXPAND_FRAMES);
+            return writer.toByteArray();
+        } catch (final RuntimeException re) { // too old asm for ex, ignore these classes to not block the rest
+            return null;
+        }
+    }
+
+    private static class SystemExitOverrideVisitor extends ClassVisitor {
+        private static final String SYSTEM_REPLACEMENT =
+                SystemExitManager.class.getName().replace('.', '/');
+
+        private SystemExitOverrideVisitor(final ClassVisitor visitor) {
+            super(ASM9, visitor);
+        }
+
+        @Override
+        public MethodVisitor visitMethod(
+                final int access,
+                final String name,
+                final String descriptor,
+                final String signature,
+                final String[] exceptions) {
+            return new GeneratorAdapter(
+                    ASM9,
+                    super.visitMethod(access, name, descriptor, signature, exceptions),
+                    access,
+                    name,
+                    descriptor) {
+                @Override
+                public void visitMethodInsn(
+                        final int opcode,
+                        final String owner,
+                        final String name,
+                        final String descriptor,
+                        final boolean isInterface) {
+                    if (owner.equals("java/lang/System") && name.equals("exit")) {
+                        mv.visitMethodInsn(opcode, SYSTEM_REPLACEMENT, name, descriptor, isInterface);
+                    } else {
+                        super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
+                    }
+                }
+            };
+        }
+    }
+}

src/main/java/org/codehaus/mojo/exec/ExecJavaMojo.java

@@ -202,11 +202,6 @@ public class ExecJavaMojo extends AbstractExecMojo {
      * exception. This way, the error is propagated without terminating the whole Maven JVM. In previous versions, users
      * had to use the {@code exec} instead of the {@code java} goal in such cases, which now with this option is no
      * longer necessary.
-     * <p>
-     * <b>Caveat:</b> Since JDK 17, you need to explicitly allow security manager usage when using this option, e.g. by
-     * setting {@code -Djava.security.manager=allow} in {@code MAVEN_OPTS}. Otherwise, the JVM will throw an
-     * {@link UnsupportedOperationException} with a message like "The Security Manager is deprecated and will be removed
-     * in a future release".
      *
      * @since 3.2.0
      */
@@ -266,8 +261,6 @@ public void execute() throws MojoExecutionException, MojoFailureException {
                         bootClassName = mainClass;
                     }
 
-                    SecurityManager originalSecurityManager = System.getSecurityManager();
-
                     try {
                         Class<?> bootClass =
                                 Thread.currentThread().getContextClassLoader().loadClass(bootClassName);
@@ -277,9 +270,6 @@ public void execute() throws MojoExecutionException, MojoFailureException {
                         MethodHandle mainHandle =
                                 lookup.findStatic(bootClass, "main", MethodType.methodType(void.class, String[].class));
 
-                        if (blockSystemExit) {
-                            System.setSecurityManager(new SystemExitManager(originalSecurityManager));
-                        }
                         mainHandle.invoke(arguments);
                     } catch (IllegalAccessException | NoSuchMethodException | NoSuchMethodError e) { // just pass it on
                         Thread.currentThread()
@@ -302,10 +292,6 @@ public void execute() throws MojoExecutionException, MojoFailureException {
                         }
                     } catch (Throwable e) { // just pass it on
                         Thread.currentThread().getThreadGroup().uncaughtException(Thread.currentThread(), e);
-                    } finally {
-                        if (blockSystemExit) {
-                            System.setSecurityManager(originalSecurityManager);
-                        }
                     }
                 },
                 mainClass + ".main()");
@@ -329,7 +315,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
 
             try {
                 threadGroup.destroy();
-            } catch (IllegalThreadStateException e) {
+            } catch (RuntimeException /* missing method in future java version */ e) {
                 getLog().warn("Couldn't destroy threadgroup " + threadGroup, e);
             }
         }
@@ -544,11 +530,14 @@ private URLClassLoader getClassLoader() throws MojoExecutionException {
         this.addAdditionalClasspathElements(classpathURLs);
 
         try {
-            return URLClassLoaderBuilder.builder()
+            final URLClassLoaderBuilder builder = URLClassLoaderBuilder.builder()
                     .setLogger(getLog())
                     .setPaths(classpathURLs)
-                    .setExclusions(classpathFilenameExclusions)
-                    .build();
+                    .setExclusions(classpathFilenameExclusions);
+            if (blockSystemExit) {
+                builder.setTransformer(new BlockExitTransformer());
+            }
+            return builder.build();
         } catch (NullPointerException | IOException e) {
             throw new MojoExecutionException(e.getMessage(), e);
         }

src/main/java/org/codehaus/mojo/exec/SystemExitManager.java

@@ -16,19 +16,14 @@
  * limitations under the License.
  */
 
-import java.security.Permission;
-
 /**
- * A special security manager (SM) passing on permission checks to the original SM it replaces, except for
- * {@link #checkExit(int)}
+ * Will be used by {@link BlockExitTransformer} to replace {@link System#exit(int)} by this implementation.
  *
  * @author Alexander Kriegisch
  */
-public class SystemExitManager extends SecurityManager {
-    private final SecurityManager originalSecurityManager;
-
-    public SystemExitManager(SecurityManager originalSecurityManager) {
-        this.originalSecurityManager = originalSecurityManager;
+public final class SystemExitManager {
+    private SystemExitManager() {
+        // no-op
     }
 
     /**
@@ -52,15 +47,7 @@ public SystemExitManager(SecurityManager originalSecurityManager) {
      *
      * @param status the exit status
      */
-    @Override
-    public void checkExit(int status) {
+    public static void exit(final int status) {
         throw new SystemExitException("System::exit was called with return code " + status, status);
     }
-
-    @Override
-    public void checkPermission(Permission perm) {
-        if (originalSecurityManager != null) {
-            originalSecurityManager.checkPermission(perm);
-        }
-    }
 }

src/main/java/org/codehaus/mojo/exec/URLClassLoaderBuilder.java

@@ -22,6 +22,9 @@
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
+import java.lang.instrument.ClassFileTransformer;
+import java.lang.instrument.IllegalClassFormatException;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLClassLoader;
@@ -34,6 +37,7 @@
 import java.util.List;
 
 import org.apache.maven.plugin.logging.Log;
+import org.codehaus.plexus.util.IOUtil;
 
 import static java.util.Arrays.asList;
 
@@ -46,13 +50,19 @@ class URLClassLoaderBuilder {
     private Log logger;
     private Collection<Path> paths;
     private Collection<String> exclusions;
+    private ClassFileTransformer transformer;
 
     private URLClassLoaderBuilder() {}
 
     static URLClassLoaderBuilder builder() {
         return new URLClassLoaderBuilder();
     }
 
+    public URLClassLoaderBuilder setTransformer(final ClassFileTransformer transformer) {
+        this.transformer = transformer;
+        return this;
+    }
+
     URLClassLoaderBuilder setLogger(Log logger) {
         this.logger = logger;
         return this;
@@ -86,7 +96,7 @@ URLClassLoader build() throws IOException {
             }
         }
 
-        return new ExecJavaClassLoader(urls.toArray(new URL[0]));
+        return new ExecJavaClassLoader(urls.toArray(new URL[0]), transformer, logger);
     }
 
     // child first strategy
@@ -100,10 +110,14 @@ private static class ExecJavaClassLoader extends URLClassLoader {
         }
 
         private final String jre;
+        private final Log logger;
+        private final ClassFileTransformer transformer;
 
-        public ExecJavaClassLoader(URL[] urls) {
+        public ExecJavaClassLoader(final URL[] urls, final ClassFileTransformer transformer, final Log logger) {
             super(urls);
-            jre = getJre();
+            this.jre = getJre();
+            this.logger = logger;
+            this.transformer = transformer;
         }
 
         @Override
@@ -112,6 +126,10 @@ public Class<?> loadClass(final String name, final boolean resolve) throws Class
                 throw new ClassNotFoundException();
             }
 
+            if ("org.codehaus.mojo.exec.SystemExitManager".equals(name)) {
+                return SystemExitManager.class;
+            }
+
             synchronized (getClassLoadingLock(name)) {
                 Class<?> clazz;
 
@@ -135,7 +153,7 @@ public Class<?> loadClass(final String name, final boolean resolve) throws Class
 
                 // look for it in this classloader
                 try {
-                    clazz = super.findClass(name);
+                    clazz = transformer != null ? doFindClass(name) : super.findClass(name);
                     if (clazz != null) {
                         if (postLoad(resolve, clazz)) {
                             return clazz;
@@ -164,6 +182,23 @@ public Class<?> loadClass(final String name, final boolean resolve) throws Class
             }
         }
 
+        private Class<?> doFindClass(final String name) throws ClassNotFoundException {
+            final String resource = name.replace('.', '/') + ".class";
+            final URL url = super.findResource(resource);
+            if (url == null) {
+                throw new ClassNotFoundException(name);
+            }
+
+            try (final InputStream inputStream = url.openStream()) {
+                final byte[] raw = IOUtil.toByteArray(inputStream);
+                final byte[] res = transformer.transform(this, name, null, null, raw);
+                final byte[] bin = res == null ? raw : res;
+                return super.defineClass(name, bin, 0, bin.length);
+            } catch (final ClassFormatError | IOException | IllegalClassFormatException var4) {
+                throw new ClassNotFoundException(name, var4);
+            }
+        }
+
         @Override
         public Enumeration<URL> getResources(String name) throws IOException {
             final Enumeration<URL> selfResources = findResources(name);