Permalink
Browse files

Fix for JRUBY-4908: IO.popen4 returns the wrong pid in Linux

  • Loading branch information...
1 parent 97a7924 commit edadd2bd1d339e60e6dcfab1bc556102c1aa7400 @headius headius committed Nov 1, 2010
Showing with 50 additions and 11 deletions.
  1. +2 −2 src/org/jruby/RubyIO.java
  2. +34 −9 src/org/jruby/util/ShellLauncher.java
  3. +14 −0 test/test_io.rb
@@ -3490,7 +3490,7 @@ public static IRubyObject popen(ThreadContext context, IRubyObject recv, IRubyOb
}
}
- @JRubyMethod(required = 1, rest = true, frame = true, meta = true)
+ @JRubyMethod(rest = true, frame = true, meta = true)
public static IRubyObject popen3(ThreadContext context, IRubyObject recv, IRubyObject[] args, Block block) {
Ruby runtime = context.getRuntime();
@@ -3516,7 +3516,7 @@ public static IRubyObject popen3(ThreadContext context, IRubyObject recv, IRubyO
}
}
- @JRubyMethod(required = 1, rest = true, frame = true, meta = true)
+ @JRubyMethod(rest = true, frame = true, meta = true)
public static IRubyObject popen4(ThreadContext context, IRubyObject recv, IRubyObject[] args, Block block) {
Ruby runtime = context.getRuntime();
@@ -536,18 +536,33 @@ private static Process popenShared(Ruby runtime, IRubyObject[] strings) throws I
File pwd = new File(runtime.getCurrentDirectory());
try {
+ String[] args = parseCommandLine(runtime.getCurrentContext(), runtime, strings);
+ boolean useShell = false;
+ for (String arg : args) useShell |= shouldUseShell(arg);
+
// CON: popen is a case where I think we should just always shell out.
if (strings.length == 1) {
- // single string command, pass to sh to expand wildcards
- String[] argArray = new String[3];
- argArray[0] = shell;
- argArray[1] = shell.endsWith("sh") ? "-c" : "/c";
- argArray[2] = strings[0].asJavaString();
- childProcess = Runtime.getRuntime().exec(argArray, getCurrentEnv(runtime), pwd);
+ if (useShell) {
+ // single string command, pass to sh to expand wildcards
+ String[] argArray = new String[3];
+ argArray[0] = shell;
+ argArray[1] = shell.endsWith("sh") ? "-c" : "/c";
+ argArray[2] = strings[0].asJavaString();
+ childProcess = Runtime.getRuntime().exec(argArray, getCurrentEnv(runtime), pwd);
+ } else {
+ childProcess = Runtime.getRuntime().exec(args, getCurrentEnv(runtime), pwd);
+ }
} else {
- // direct invocation of the command
- String[] args = parseCommandLine(runtime.getCurrentContext(), runtime, strings);
- childProcess = Runtime.getRuntime().exec(args, getCurrentEnv(runtime), pwd);
+ if (useShell) {
+ String[] argArray = new String[args.length + 2];
+ argArray[0] = shell;
+ argArray[1] = shell.endsWith("sh") ? "-c" : "/c";
+ System.arraycopy(args, 0, argArray, 2, args.length);
+ childProcess = Runtime.getRuntime().exec(argArray, getCurrentEnv(runtime), pwd);
+ } else {
+ // direct invocation of the command
+ childProcess = Runtime.getRuntime().exec(args, getCurrentEnv(runtime), pwd);
+ }
}
} catch (SecurityException se) {
throw runtime.newSecurityError(se.getLocalizedMessage());
@@ -1298,6 +1313,16 @@ private static String getShell(Ruby runtime) {
return RbConfigLibrary.jrubyShell();
}
+ private static boolean shouldUseShell(String command) {
+ boolean useShell = false;
+ for (char c : command.toCharArray()) {
+ if (c != ' ' && !Character.isLetter(c) && "*?{}[]<>()~&|\\$;'`\"\n".indexOf(c) != -1) {
+ useShell = true;
+ }
+ }
+ return useShell;
+ }
+
static void log(Ruby runtime, String msg) {
if (RubyInstanceConfig.DEBUG_LAUNCHING) {
runtime.getErr().println("ShellLauncher: " + msg);
View
@@ -473,4 +473,18 @@ def test_clear_dollar_bang_after_open_block
def ensure_files(*files)
files.each {|f| File.open(f, "w") {|g| g << " " } }
end
+ private :ensure_files
+
+ # JRUBY-4908
+ unless WINDOWS
+ def test_sh_used_appropriately
+ # should not use sh
+ p, o, i, e = IO.popen4("/bin/ps -a")
+ assert_match p.to_s, i.read.lines.grep(/\/bin\/ps/).first
+
+ # should use sh
+ p, o, i, e = IO.popen4("/bin/ps -a | grep ps'")
+ assert_no_match Regexp.new(p.to_s), i.read.grep(/\/bin\/ps/).first
+ end
+ end
end

0 comments on commit edadd2b

Please sign in to comment.