[WiP] - Code cleanup

pom.xml

@@ -48,6 +48,7 @@
         <version>${findbugs-maven-plugin.version}</version>
         <configuration>
           <effort>Max</effort>
+          <threshold>Low</threshold>
           <!--TODO: a couple of safe issues, so default to medium for a while <threshold>Low</threshold>-->
           <xmlOutput>true</xmlOutput>
           <findbugsXmlOutput>false</findbugsXmlOutput>
@@ -86,6 +87,11 @@
       <version>3.0.0</version>
       <scope>provided</scope>
     </dependency>
+    <dependency>
+      <groupId>org.kohsuke</groupId>
+      <artifactId>access-modifier-annotation</artifactId>
+      <version>1.7</version>
+    </dependency>
   </dependencies>
 
   <distributionManagement>
@@ -94,6 +100,13 @@
       <url>gitsite:[email protected]/kohsuke/${project.artifactId}.git</url>
     </site>
   </distributionManagement>
+
+  <repositories>
+    <repository>
+      <id>repo.jenkins-ci.org</id>
+      <url>https://repo.jenkins-ci.org/public/</url>
+    </repository>
+  </repositories>
 
   <licenses>
     <license>

src/main/java/org/jvnet/winp/Main.java

@@ -1,10 +1,14 @@
 package org.jvnet.winp;
 
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+
 /**
  * Test driver class
  *
  * @author Kohsuke Kawaguchi
  */
+@Restricted(NoExternalUse.class)
 public class Main {
     public static void main(String[] args) {
         WinProcess.enableDebugPrivilege();

src/main/java/org/jvnet/winp/Native.java

@@ -10,6 +10,8 @@
 import java.security.NoSuchAlgorithmException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
 
 /**
  * Functions defined in the DLL.
@@ -55,13 +57,16 @@ class Native {
     private static final Logger LOGGER = Logger.getLogger(Native.class.getName());
     // system property holding the preferred folder for copying the dll file to.
     private static final String DLL_TARGET = "winp.folder.preferred";
+    //TODO: usage of this field has been removed in https://github.com/kohsuke/winp/pull/27
+    // Likely it needs to be fixed
     private static final String UNPACK_DLL_TO_PARENT_DIR = "winp.unpack.dll.to.parent.dir";
 
     static {
         load();
     }
 
-    private static String md5(URL res) {
+    @Nonnull
+    private static String md5(@Nonnull URL res) throws IOException {
         try {
             MessageDigest md5 = MessageDigest.getInstance("MD5");
             InputStream in = res.openStream();
@@ -75,13 +80,13 @@ private static String md5(URL res) {
                 in.close();
             }
         } catch (NoSuchAlgorithmException e) {
-            throw new AssertionError(e);
+            throw new IOException("Cannot find MD5 algorithm", e);
         } catch (IOException e) {
-            throw new Error("failed to checksum " + res + ": " + e, e);
+            throw new IOException("failed to checksum " + res + ": " + e, e);
         }
     }
 
-    private static void load() {
+    private static void load() throws UnsatisfiedLinkError {
 
         final URL res = Native.class.getClassLoader().getResource(DLL_NAME + ".dll");
 
@@ -100,7 +105,7 @@ private static void load() {
         }
     }
 
-    private static void loadByUrl(URL res) throws IOException {
+    private static void loadByUrl(@Nonnull URL res) throws IOException {
 
         String url = res.toExternalForm();
 
@@ -128,11 +133,12 @@ private static void loadByUrl(URL res) throws IOException {
         loadDll(dllFile);
     }
 
-    private static File extractToStaticLocation(URL url) throws IOException {
+    @Nonnull
+    private static File extractToStaticLocation(@Nonnull URL url) throws IOException {
 
         File jarFile = getJarFile(url);
         if (jarFile == null) {
-            throw new RuntimeException("Failed to locate JAR file by URL " + url);
+            throw new IOException("Failed to locate JAR file by URL " + url);
         }
 
         String preferred = System.getProperty(DLL_TARGET);
@@ -143,15 +149,17 @@ private static File extractToStaticLocation(URL url) throws IOException {
         return destFile;
     }
 
-    private static File extractToTmpLocation(URL res) throws IOException {
+    @Nonnull
+    private static File extractToTmpLocation(@Nonnull URL res) throws IOException {
 
         File tmpFile = File.createTempFile(DLL_NAME, ".dll");
         tmpFile.deleteOnExit();
         copyStream(res.openStream(), new FileOutputStream(tmpFile));
         return tmpFile;
     }
 
-    private static File getJarFile(URL res) {
+    @CheckForNull
+    /**package*/ static File getJarFile(@Nonnull URL res) {
 
         String url = res.toExternalForm();
         if (!(url.startsWith("jar:") || url.startsWith("wsjar:"))) {
@@ -175,6 +183,7 @@ private static File getJarFile(URL res) {
             // this indicates file://host/path-in-host format
             // Windows maps UNC path to this. On Unix, there's no well defined
             // semantics for  this.
+            LOGGER.log(Level.FINE, "file://PATH semantics is used. On Unix the behavior is undefined, hence the results may differ from the expectation.");
         }
 
         filePortion = URLDecoder.decode(filePortion);
@@ -226,7 +235,8 @@ private static void copyStream(InputStream in, OutputStream out) throws IOExcept
     /**
      * Convert 128bit data into hex string.
      */
-    private static String toHex32(byte[] b) {
+    @Nonnull
+    private static String toHex32(@Nonnull byte[] b) {
         return String.format("%032X",new BigInteger(1,b));
     }
 }

src/main/java/org/jvnet/winp/WinProcess.java

@@ -1,13 +1,17 @@
 package org.jvnet.winp;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.lang.reflect.Field;
 import java.util.Comparator;
 import java.util.TreeMap;
 import java.util.Iterator;
+import java.util.Locale;
+import java.util.NoSuchElementException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import static java.util.logging.Level.FINE;
+import javax.annotation.Nonnull;
 
 /**
  * Represents a Windows process.
@@ -36,8 +40,12 @@ public WinProcess(int pid) {
 
     /**
      * Wraps {@link Process} into {@link WinProcess}.
+     *
+     * @param proc Process to be wrapped
      */
-    public WinProcess(Process proc) {
+    @SuppressFBWarnings(value = "DP_DO_INSIDE_DO_PRIVILEGED", 
+            justification = "Enabled for now, the calling code is expected to have appropriate security permissions")
+    public WinProcess(@Nonnull Process proc) {
         try {
             Field f = proc.getClass().getDeclaredField("handle");
             f.setAccessible(true);
@@ -121,13 +129,15 @@ public synchronized String getCommandLine() {
      *      If we fail to obtain the command line. For example,
      *      maybe we didn't have enough security privileges.
      */
-    public synchronized TreeMap<String,String> getEnvironmentVariables() {
-        if(envVars==null)
+    @Nonnull
+    public synchronized TreeMap<String, String> getEnvironmentVariables() {
+        if (envVars == null) {
             parseCmdLineAndEnvVars();
+        }
         return envVars;
     }
 
-    private void parseCmdLineAndEnvVars() {
+    private void parseCmdLineAndEnvVars() throws WinpException {
         String s = Native.getCmdLineAndEnvVars(pid);
         if(s==null)
             throw new WinpException("Failed to obtain for PID="+pid);
@@ -157,7 +167,9 @@ private void parseCmdLineAndEnvVars() {
 
     private static final Comparator<String> CASE_INSENSITIVE_COMPARATOR = new Comparator<String>() {
         public int compare(String o1, String o2) {
-            return o1.toUpperCase().compareTo(o2.toUpperCase());
+            // Rely on the default system locale
+            Locale loc = Locale.getDefault();
+            return o1.toUpperCase(loc).compareTo(o2.toUpperCase(loc));
         }
     };
 
@@ -192,7 +204,10 @@ public boolean hasNext() {
                         return pos<total;
                     }
 
-                    public WinProcess next() {
+                    public WinProcess next() throws NoSuchElementException {
+                        if (!hasNext()) {
+                            throw new NoSuchElementException();
+                        }
                         return new WinProcess(pids[pos++]);
                     }
 

src/test/java/org/jvnet/winp/NativeAPITest.java

@@ -1,20 +1,28 @@
 package org.jvnet.winp;
 
+import java.io.File;
+import java.net.URL;
 import static org.hamcrest.CoreMatchers.equalTo;
 import org.junit.Assert;
 import static org.junit.Assume.assumeThat;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
-import org.jvnet.winp.WinProcess;
-import org.jvnet.winp.WinpException;
 
 /**
  * Basic tests of the native library.
  * @author Kohsuke Kawaguchi
  */
 public class NativeAPITest extends Assert {
 
+    @BeforeClass
+    public static void enableDebug() {
+        // No use of Assume here, because we want all tests to be reported as skipped
+        if (TestHelper.isWindows()) {
+            WinProcess.enableDebugPrivilege();
+        }
+    }
+
     @Before
     public void runOnWindowsOnly() {
         TestHelper.assumeIsWindows();
@@ -112,9 +120,10 @@ public void testKill() throws Exception {
         Thread.sleep(100);
         wp.killRecursively();
     }
-
-    @BeforeClass
-    public static void enableDebug() {
-        WinProcess.enableDebugPrivilege();
+
+    @Test
+    public void shouldNotFailWithUNCPath() throws Exception {
+        File jarFile = Native.getJarFile(new URL("file://myUnc/path.jar"));
+        assertNotNull("JAR path to UNC should have been resolved somehow", jarFile);
     }
 }

src/test/java/org/jvnet/winp/TestHelper.java

@@ -33,6 +33,15 @@
  */
 class TestHelper {
 
+    public static boolean isWindows() throws AssertionError {
+        String property = System.getProperty("os.name");
+        if (property == null || property.trim().isEmpty()) {
+            return false;
+        }
+
+        return property.contains("Windows");
+    }
+
     public static void assumeIsWindows() {
         Assume.assumeThat("The test utilizes the native WinP Library. It can be executed on Windows only.", 
                 System.getProperty("os.name"), StringContains.containsString("Windows"));