Fix illegal reflective access in exec provider (fixes #862) (#868)

native/src/main/java/org/jline/nativ/JLineLibrary.java

@@ -23,4 +23,6 @@ public class JLineLibrary {
     }
 
     public static native FileDescriptor newFileDescriptor(int fd);
+
+    public static native ProcessBuilder.Redirect newRedirectPipe(FileDescriptor fd);
 }

native/src/main/native/jlinenative.c

@@ -38,4 +38,26 @@ JNIEXPORT jobject JNICALL JLineLibrary_NATIVE(newFileDescriptor)(JNIEnv *env, jc
     return ret;
 }
 
+JNIEXPORT jobject JNICALL JLineLibrary_NATIVE(newRedirectPipe)(JNIEnv *env, jclass that, jobject fd)
+{
+    jfieldID field_fd;
+    jmethodID const_rpi;
+    jclass class_rpi;
+    jobject ret;
 
+    class_rpi = (*env)->FindClass(env, "java/lang/ProcessBuilder$RedirectPipeImpl");
+    if (class_rpi == NULL) return NULL;
+
+    // construct a new RedirectPipeImpl
+    const_rpi = (*env)->GetMethodID(env, class_rpi, "<init>", "()V");
+    if (const_rpi == NULL) return NULL;
+    ret = (*env)->NewObject(env, class_rpi, const_rpi);
+
+    // poke the "fd" field with the file descriptor
+    field_fd = (*env)->GetFieldID(env, class_rpi, "fd", "Ljava/io/FileDescriptor;");
+    if (field_fd == NULL) return NULL;
+    (*env)->SetObjectField(env, ret, field_fd, fd);
+
+    // and return it
+    return ret;
+}

native/src/test/java/org/jline/nativ/JLineLibraryTest.java

@@ -26,4 +26,16 @@ void testNewFileDescriptor() throws Exception {
         field.setAccessible(true);
         assertEquals(12, field.get(fd), fd.toString());
     }
+
+    @Test
+    void testNewRedirectPipeImpl() throws Exception {
+        ProcessBuilder.Redirect redirect = JLineLibrary.newRedirectPipe(FileDescriptor.out);
+        assertNotNull(redirect);
+        // This requires '--add-opens java.base/java.lang=ALL-UNNAMED', but adding this option
+        // defeats the very purpose of the method
+        //
+        // Method mth = redirect.getClass().getDeclaredMethod("getFd");
+        // mth.setAccessible(true);
+        // assertEquals(System.out, mth.invoke(redirect), redirect.toString());
+    }
 }

terminal/src/main/java/org/jline/terminal/TerminalBuilder.java

@@ -79,6 +79,15 @@ public final class TerminalBuilder {
     public static final String PROP_FILE_DESCRIPTOR_CREATION_MODE_REFLECTION = "reflection";
     public static final String PROP_FILE_DESCRIPTOR_CREATION_MODE_DEFAULT = "reflection,native";
 
+    //
+    // System properties controlling how RedirectPipe are created.
+    // The value can be a comma separated list of defined mechanisms.
+    //
+    public static final String PROP_REDIRECT_PIPE_CREATION_MODE = "org.jline.terminal.exec.redirectPipeCreationMode";
+    public static final String PROP_REDIRECT_PIPE_CREATION_MODE_NATIVE = "native";
+    public static final String PROP_REDIRECT_PIPE_CREATION_MODE_REFLECTION = "reflection";
+    public static final String PROP_REDIRECT_PIPE_CREATION_MODE_DEFAULT = "reflection,native";
+
     //
     // Terminal output control
     //

terminal/src/main/java/org/jline/terminal/impl/exec/ExecTerminalProvider.java

@@ -16,6 +16,8 @@
 import java.lang.reflect.Field;
 import java.nio.charset.Charset;
 
+import org.jline.nativ.JLineLibrary;
+import org.jline.nativ.JLineNativeLoader;
 import org.jline.terminal.Attributes;
 import org.jline.terminal.Size;
 import org.jline.terminal.Terminal;
@@ -28,6 +30,11 @@
 import org.jline.utils.Log;
 import org.jline.utils.OSUtils;
 
+import static org.jline.terminal.TerminalBuilder.PROP_REDIRECT_PIPE_CREATION_MODE;
+import static org.jline.terminal.TerminalBuilder.PROP_REDIRECT_PIPE_CREATION_MODE_DEFAULT;
+import static org.jline.terminal.TerminalBuilder.PROP_REDIRECT_PIPE_CREATION_MODE_NATIVE;
+import static org.jline.terminal.TerminalBuilder.PROP_REDIRECT_PIPE_CREATION_MODE_REFLECTION;
+
 public class ExecTerminalProvider implements TerminalProvider {
 
     private static boolean warned;
@@ -137,7 +144,7 @@ public String systemStreamName(Stream stream) {
         try {
             ProcessBuilder.Redirect input = stream == Stream.Input
                     ? ProcessBuilder.Redirect.INHERIT
-                    : getRedirect(stream == Stream.Output ? FileDescriptor.out : FileDescriptor.err);
+                    : newDescriptor(stream == Stream.Output ? FileDescriptor.out : FileDescriptor.err);
             Process p =
                     new ProcessBuilder(OSUtils.TTY_COMMAND).redirectInput(input).start();
             String result = ExecHelper.waitAndCapture(p);
@@ -157,20 +164,82 @@ public String systemStreamName(Stream stream) {
         return null;
     }
 
+    private static RedirectPipeCreator redirectPipeCreator;
+
+    protected static ProcessBuilder.Redirect newDescriptor(FileDescriptor fd) {
+        if (redirectPipeCreator == null) {
+            String str = System.getProperty(PROP_REDIRECT_PIPE_CREATION_MODE, PROP_REDIRECT_PIPE_CREATION_MODE_DEFAULT);
+            String[] modes = str.split(",");
+            IllegalStateException ise = new IllegalStateException("Unable to create RedirectPipe");
+            for (String mode : modes) {
+                try {
+                    switch (mode) {
+                        case PROP_REDIRECT_PIPE_CREATION_MODE_NATIVE:
+                            redirectPipeCreator = new NativeRedirectPipeCreator();
+                            break;
+                        case PROP_REDIRECT_PIPE_CREATION_MODE_REFLECTION:
+                            redirectPipeCreator = new ReflectionRedirectPipeCreator();
+                            break;
+                    }
+                } catch (Throwable t) {
+                    // ignore
+                    ise.addSuppressed(t);
+                }
+                if (redirectPipeCreator != null) {
+                    break;
+                }
+            }
+            if (redirectPipeCreator == null) {
+                throw ise;
+            }
+        }
+        return redirectPipeCreator.newRedirectPipe(fd);
+    }
+
+    interface RedirectPipeCreator {
+        ProcessBuilder.Redirect newRedirectPipe(FileDescriptor fd);
+    }
+
     /**
-     * This requires --add-opens java.base/java.lang=ALL-UNNAMED
+     * Reflection based file descriptor creator.
+     * This requires the following option
+     *   --add-opens java.base/java.lang=ALL-UNNAMED
      */
-    private ProcessBuilder.Redirect getRedirect(FileDescriptor fd) throws ReflectiveOperationException {
-        // This is not really allowed, but this is the only way to redirect the output or error stream
-        // to the input.  This is definitely not something you'd usually want to do, but in the case of
-        // the `tty` utility, it provides a way to get
-        Class<?> rpi = Class.forName("java.lang.ProcessBuilder$RedirectPipeImpl");
-        Constructor<?> cns = rpi.getDeclaredConstructor();
-        cns.setAccessible(true);
-        ProcessBuilder.Redirect input = (ProcessBuilder.Redirect) cns.newInstance();
-        Field f = rpi.getDeclaredField("fd");
-        f.setAccessible(true);
-        f.set(input, fd);
-        return input;
+    static class ReflectionRedirectPipeCreator implements RedirectPipeCreator {
+        private final Constructor<ProcessBuilder.Redirect> constructor;
+        private final Field fdField;
+
+        @SuppressWarnings("unchecked")
+        ReflectionRedirectPipeCreator() throws Exception {
+            Class<?> rpi = Class.forName("java.lang.ProcessBuilder$RedirectPipeImpl");
+            constructor = (Constructor<ProcessBuilder.Redirect>) rpi.getDeclaredConstructor();
+            constructor.setAccessible(true);
+            fdField = rpi.getDeclaredField("fd");
+            fdField.setAccessible(true);
+        }
+
+        @Override
+        public ProcessBuilder.Redirect newRedirectPipe(FileDescriptor fd) {
+            try {
+                ProcessBuilder.Redirect input = constructor.newInstance();
+                fdField.set(input, fd);
+                return input;
+            } catch (ReflectiveOperationException e) {
+                // This should not happen as the field has been set accessible
+                throw new IllegalStateException(e);
+            }
+        }
+    }
+
+    static class NativeRedirectPipeCreator implements RedirectPipeCreator {
+        public NativeRedirectPipeCreator() {
+            // Force load the library
+            JLineNativeLoader.initialize();
+        }
+
+        @Override
+        public ProcessBuilder.Redirect newRedirectPipe(FileDescriptor fd) {
+            return JLineLibrary.newRedirectPipe(fd);
+        }
     }
 }