Commits

David Black [Atlassian] committed 1d67c80 Merge

Merged in db_atlass/atlassian-processutils/detect_fixed_windows_unicode_jvms (pull request #4)

Where there is no need to use the windows unicode workaround - do not use it

Comments (0)

Files changed (4)

src/main/java/com/atlassian/utils/process/ExternalProcessImpl.java

 
     protected Process createProcess(List<String> command, Map<String, String> environment, File workingDir)
             throws IOException {
-        if (!suppressSpecialWindowsBehaviour && isWindows()) {
+        if (!suppressSpecialWindowsBehaviour && isWindows() && !ProcessUtils.is4947220Fixed()) {
             return createWinProcess(command, environment, workingDir);
         } else {
             ProcessBuilder builder = new ProcessBuilder(command).directory(workingDir);
         }
         return false;
     }
-    
+
     private void wrapUpProcess() {
         if (finished.get()) {
             return;

src/main/java/com/atlassian/utils/process/ProcessUtils.java

 package com.atlassian.utils.process;
 
+import org.apache.log4j.Logger;
+
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Locale;
 
 /**
  */
 public class ProcessUtils {
+    private static final Logger log = Logger.getLogger(ProcessUtils.class);
     private ProcessUtils() {
     }
 
     public static List<String> tokenizeAsList(String commandLine) {
         return new ArrayList<String>(Arrays.asList(tokenizeCommand(commandLine)));
     }
+
+    /**
+     *
+     * @return true iff this JVM has a fix for http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4947220.
+     * According to http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4947220 &
+     * http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=2213209
+     * Sun/Oracle JVM versions 7 & 6u30(and above) do not suffer from 4947220.
+     */
+    public static boolean is4947220Fixed() {
+        final String jvmVersion = System.getProperty("java.version");
+        final String jvmVendor = System.getProperty("java.vendor");
+        int minorVersion = -1;
+        double majorVersion = -1.0;
+        if (jvmVersion.split("_").length == 2) {
+            try {
+                majorVersion = Double.parseDouble(jvmVersion.substring(0, 3));
+                minorVersion = Integer.parseInt(jvmVersion.split("_")[1]);
+            }
+            catch (NumberFormatException e) {
+                log.error("Failed to parse the JVM version", e);
+            }
+        }
+        for (String vendor : Arrays.asList("sun", "oracle", "openjdk")) {
+            if (jvmVendor.toLowerCase(Locale.US).contains(vendor)) {
+                if (majorVersion == 1.6 && minorVersion < 30) {
+                    return false;
+                }
+                if (majorVersion < 1.6) {
+                    return false;
+                }
+            }
+        }
+        return true;
+    }
+
 }

src/test/java/com/atlassian/utils/process/ProcessBuilderEncodingTest.java

 package com.atlassian.utils.process;
 
-import org.apache.commons.lang.SystemUtils;
-import org.junit.Test;
+import static org.junit.Assert.*;
 
 import java.io.BufferedReader;
 import java.io.IOException;
-import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assume.assumeTrue;
+import org.junit.Test;
 
 public class ProcessBuilderEncodingTest {
 
+    private static class EchoResult {
+        public String input;
+        public String output;
+    }
+
+    /*
+     * Starts the CallEcho as an separate process. This is necessary because the jvm uses the default encoding
+     * (-Dfile.encoding) for encoding command line arguments that are provided to external processes.This test tests
+     * whether encodings that should be compatible are actually compatible. This is needed for
+     * http://jira.atlassian.com/browse/CRUC-4793 which provides a workaround for a JVM bug on windows.
+     */
+    protected EchoResult spawnEcho(String encoding, String... testStrings) throws Exception {
+        List<String> cmd = new ArrayList<String>(Arrays.asList("java", "-Dfile.encoding=" + encoding, CallEcho.class.getName()));
+        cmd.addAll(Arrays.asList(testStrings));
+
+        ProcessBuilder processBuilder = new ProcessBuilder(cmd);
+        processBuilder.environment().put("CLASSPATH", System.getProperty("java.class.path"));
+        final Process process = processBuilder.start();
+
+        Thread th = new Thread(new Runnable() {
+            public void run() {
+                while (true)
+                {
+                    try {
+                         process.getErrorStream().read();
+                    } catch (IOException e) {
+                    }
+                }
+            }
+        }) ;
+        th.start();
+
+        BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()));
+        try {
+            EchoResult result = new EchoResult();
+            result.input = reader.readLine();
+            result.output = reader.readLine();
+
+            return result;
+        } finally {
+            IOUtils.closeQuietly(reader);
+        }
+    }
+
+    protected void assertNotEquals(String message, String value1, String value2) {
+        assertFalse(message + "; result was : " + value1, (value1 == null && value2 == null) || (value1 != null && value1.equals(value2)));
+    }
+
+    protected boolean isWindows() {
+        return System.getProperty("os.name").toLowerCase().contains("windows");
+    }
+
+    protected boolean isLinux() {
+        return System.getProperty("os.name").toLowerCase().contains("linux");
+    }
+
     @Test
     public void testIncompatibleCommandLineArgumentsASCII() throws Exception {
         EchoResult result = spawnEcho("ASCII", "iso-8859-1");
     public void testCommandLineArgumentsUTF8IncompatibleWithCp1252() throws Exception {
         EchoResult result = spawnEcho("UTF-8", "unicode-chinese");
 
-        if (SystemUtils.IS_OS_WINDOWS) {
+        if (isWindows()) {
             assertNotEquals("UTF-8 - incompatibility expected", result.input, result.output);
         } else {
             assertEquals("UTF-8 - incompatibility detected", result.input, result.output);
 
     @Test
     public void testCommandLineArgumentsWin1252() throws Exception {
-        assumeTrue(Charset.isSupported("windows-1252") && !SystemUtils.IS_OS_LINUX);
-
-        // test is disabled in linux because (ubuntu) linux always uses UTF-8 to encode parameters, causing this test to fail
-        EchoResult result = spawnEcho("windows-1252", "iso-8859-1");
-        assertEquals("Windows cp 1252 -  incompatibility detected:", result.input, result.output);
+        if (Charset.isSupported("windows-1252") && !isLinux()) {
+            // test is disabled in linux because (ubuntu) linux always uses UTF-8 to encode parameters, causing this test to fail
+            EchoResult result = spawnEcho("windows-1252", "iso-8859-1");
+            assertEquals("Windows cp 1252 -  incompatibility detected:", result.input, result.output);
+        }
     }
 
     @Test
     public void testCommandLineArgumentsWin1252Iso88591Incompatibilities() throws Exception {
-        assumeTrue(Charset.isSupported("windows-1252") && SystemUtils.IS_OS_WINDOWS);
-
-        // other platforms claim to support windows-1252, but it looks like only iso-8859-1 is supported.
-        EchoResult result = spawnEcho("windows-1252", "win-1252-not-in-iso-8859-1");
-        assertEquals("Windows cp 1252 -  incompatibility detected:", result.input, result.output);
+        if (Charset.isSupported("windows-1252") && isWindows()) {
+            // other platforms claim to support windows-1252, but it looks like only iso-8859-1 is supported.
+            EchoResult result = spawnEcho("windows-1252", "win-1252-not-in-iso-8859-1");
+            assertEquals("Windows cp 1252 -  incompatibility detected:", result.input, result.output);
+        }
     }
 
     @Test
     public void testCommandLineArgumentsWin437() throws Exception {
-        assumeTrue(Charset.isSupported("windows-437"));
-
-        EchoResult result = spawnEcho("windows-437", "windows-437");
-        assertEquals("Windows cp 437 - incompatibility detected:", result.input, result.output);
+        if (Charset.isSupported("windows-437")) {
+            EchoResult result = spawnEcho("windows-437", "windows-437");
+            assertEquals("Windows cp 437 - incompatibility detected:", result.input, result.output);
+        }
     }
 
     @Test
     public void testCommandLineArgumentsIso88591() throws Exception {
-        assumeTrue(!SystemUtils.IS_OS_LINUX);
-
-        // test is disabled in linux because (ubuntu) linux always uses UTF-8 to encode parameters, causing this test to fail
-        EchoResult result = spawnEcho("iso-8859-1", "iso-8859-1");
-        assertEquals("ISO-8859-1 - incompatibility detected:", result.input, result.output);
-    }
-
-    /*
-     * Starts the CallEcho as an separate process. This is necessary because the jvm uses the default encoding
-     * (-Dfile.encoding) for encoding command line arguments that are provided to external processes.This test tests
-     * whether encodings that should be compatible are actually compatible. This is needed for
-     * http://jira.atlassian.com/browse/CRUC-4793 which provides a workaround for a JVM bug on windows.
-     */
-    protected EchoResult spawnEcho(String encoding, String... testStrings) throws Exception {
-        List<String> cmd = new ArrayList<String>(Arrays.asList("java", "-Dfile.encoding=" + encoding, CallEcho.class.getName()));
-        cmd.addAll(Arrays.asList(testStrings));
-
-        ProcessBuilder processBuilder = new ProcessBuilder(cmd);
-        processBuilder.environment().put("CLASSPATH", System.getProperty("java.class.path"));
-        final Process process = processBuilder.start();
-
-        Thread errorPump = new Thread() {
-
-            @Override
-            @SuppressWarnings("StatementWithEmptyBody")
-            public void run() {
-                byte[] buffer = new byte[256];
-                try {
-                    InputStream errorStream = process.getErrorStream();
-                    while (errorStream.read(buffer) != -1) {
-                        //Discard all output to stderr, but read it just to ensure the process doesn't hang
-                    }
-                } catch (IOException ignored) {
-                    //If an I/O error happens, just drop out
-                }
-            }
-        };
-        errorPump.start();
-
-        BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()));
-        try {
-            EchoResult result = new EchoResult();
-            result.input = reader.readLine();
-            result.output = reader.readLine();
-
-            //Wait for CallEcho to complete
-            process.waitFor();
-            errorPump.join();
-
-            return result;
-        } finally {
-            IOUtils.closeQuietly(reader);
+        if (!isLinux()) {
+            // test is disabled in linux because (ubuntu) linux always uses UTF-8 to encode parameters, causing this test to fail
+            EchoResult result = spawnEcho("iso-8859-1", "iso-8859-1");
+            assertEquals("ISO-8859-1 - incompatibility detected:", result.input, result.output);
         }
     }
-
-    private static void assertNotEquals(String message, String value1, String value2) {
-        assertFalse(message + "; result was : " + value1, (value1 == null && value2 == null) || (value1 != null && value1.equals(value2)));
-    }
-
-    private static class EchoResult {
-        public String input;
-        public String output;
-    }
 }

src/test/java/com/atlassian/utils/process/ProcessUtilsTest.java

+package com.atlassian.utils.process;
+
+import org.junit.After;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+public class ProcessUtilsTest {
+
+    private static final String JVM_VENDOR = "java.vendor";
+    private static final String JVM_VERSION = "java.version";
+    private final String ORIGINAL_VENDOR = System.getProperty(JVM_VENDOR);
+    private final String ORIGINAL_VERSION = System.getProperty(JVM_VERSION);
+
+    @After
+    public void tearDown() {
+        System.setProperty(JVM_VENDOR, ORIGINAL_VENDOR);
+        System.setProperty(JVM_VERSION, ORIGINAL_VERSION);
+    }
+
+    @Test
+    public void testIs4947220FixedAgainstNotFixedVersions() throws InterruptedException {
+        for (String vendor : Arrays.asList("sun", "oracle", "openjdk")) {
+            System.setProperty(JVM_VENDOR, vendor);
+            for (String notFixedVersion : Arrays.asList("1.3", "1.4.0_18", "1.5.0_19", "1.6.0_29")) {
+                System.setProperty(JVM_VERSION, notFixedVersion);
+                assertThat(ProcessUtils.is4947220Fixed(), is(false));
+            }
+        }
+    }
+
+    @Test
+    public void testIs4947220FixedAgainstFixedVersions() {
+        for (String vendor : Arrays.asList("sun", "oracle", "openjdk")) {
+            System.setProperty(JVM_VENDOR, vendor);
+            for (String fixedVersion : Arrays.asList("1.6.0_30", "1.6.0_31", "1.7.0_21", "1.8.0_21")) {
+                System.setProperty(JVM_VERSION, fixedVersion);
+                assertThat(ProcessUtils.is4947220Fixed(), is(true));
+            }
+        }
+    }
+
+    @Test
+    public void testIs4947220FixedInNonSunJVM() {
+        System.setProperty(JVM_VENDOR, "ibm");
+        System.setProperty(JVM_VERSION, "1.4.0_18");
+        assertThat(ProcessUtils.is4947220Fixed(), is(true));
+    }
+}