From aae5ec758932923aa612a42d7293cef80c275f13 Mon Sep 17 00:00:00 2001 From: Tres Finocchiaro Date: Mon, 26 Jul 2021 14:20:39 -0400 Subject: [PATCH 1/5] Add optional boot library path for native libs, helps with sandboxed environments. --- .../java/jssc/DefaultJniExtractorWrapper.java | 56 +++++++++++++++++++ src/main/java/jssc/SerialNativeInterface.java | 9 ++- 2 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 src/main/java/jssc/DefaultJniExtractorWrapper.java diff --git a/src/main/java/jssc/DefaultJniExtractorWrapper.java b/src/main/java/jssc/DefaultJniExtractorWrapper.java new file mode 100644 index 000000000..fbab0dd0d --- /dev/null +++ b/src/main/java/jssc/DefaultJniExtractorWrapper.java @@ -0,0 +1,56 @@ +/** + * License: https://opensource.org/licenses/BSD-3-Clause + */ +package jssc; + +import org.scijava.nativelib.DefaultJniExtractor; +import org.scijava.nativelib.JniExtractor; + +import java.io.File; +import java.io.IOException; + +/** + * @author A. Tres Finocchiaro + * + * Wrapper around DefaultJniExtractor class to allow native-lib-loader to conditionally + * use a statically defined native search path when provided. + */ +public class DefaultJniExtractorWrapper { + private File bootPath; + private String library; + private JniExtractor extractor; + + public DefaultJniExtractorWrapper(String library, String bootPath) throws IOException { + if(bootPath != null) { + File bootTest = new File(bootPath); + if(bootTest.exists()) { + this.bootPath = bootTest; + this.library = library; + extractor = new StubJniExtractor(); + return; + } + } + extractor = new DefaultJniExtractor(null); + } + + public JniExtractor getExtractor() { + return extractor; + } + + public class StubJniExtractor implements JniExtractor { + @Override + public File extractJni(String s, String s1) { + switch(SerialNativeInterface.getOsType()) { + case SerialNativeInterface.OS_WINDOWS: + return new File(bootPath, library + ".dll"); + case SerialNativeInterface.OS_MAC_OS_X: + return new File(bootPath, "lib" + library + ".dylib"); + default: + return new File(bootPath, "lib" + library + ".so"); + } + } + + @Override + public void extractRegistered() {} // no-op + } +} diff --git a/src/main/java/jssc/SerialNativeInterface.java b/src/main/java/jssc/SerialNativeInterface.java index fb86ff389..3ccb9157f 100644 --- a/src/main/java/jssc/SerialNativeInterface.java +++ b/src/main/java/jssc/SerialNativeInterface.java @@ -85,7 +85,14 @@ else if(osName.equals("SunOS")) else if(osName.equals("Mac OS X") || osName.equals("Darwin")) osType = OS_MAC_OS_X; try { - NativeLoader.loadLibrary("jssc"); + /** + * JSSC includes a small, platform-specific shared library and uses native-lib-loader for extraction. + * - First, native-lib-loader will attempt to load this library from the system library path. + * - Next, it will fallback to jssc.boot.library.path + * - Finally it will attempt to extract the library from from the jssc.jar file, and load it. + */ + NativeLoader.setJniExtractor( new DefaultJniExtractorWrapper("jssc", System.getProperty("jssc.boot.library.path")).getExtractor()); + NativeLoader.loadLibrary("jssc"); // method call is null-safe } catch (IOException ioException) { throw new UnsatisfiedLinkError("Could not load the jssc library: " + ioException.getMessage()); } From 9d585af011bca663802e05c7d7d2bab688fce2af Mon Sep 17 00:00:00 2001 From: tresf Date: Tue, 27 Jul 2021 13:00:00 -0400 Subject: [PATCH 2/5] Simplify JniExtractor stub, add comments/documentation --- .../java/jssc/DefaultJniExtractorStub.java | 77 +++++++++++++++++++ .../java/jssc/DefaultJniExtractorWrapper.java | 56 -------------- src/main/java/jssc/SerialNativeInterface.java | 4 +- 3 files changed, 79 insertions(+), 58 deletions(-) create mode 100644 src/main/java/jssc/DefaultJniExtractorStub.java delete mode 100644 src/main/java/jssc/DefaultJniExtractorWrapper.java diff --git a/src/main/java/jssc/DefaultJniExtractorStub.java b/src/main/java/jssc/DefaultJniExtractorStub.java new file mode 100644 index 000000000..e5a614520 --- /dev/null +++ b/src/main/java/jssc/DefaultJniExtractorStub.java @@ -0,0 +1,77 @@ +/** + * License: https://opensource.org/licenses/BSD-3-Clause + */ +package jssc; + +import org.scijava.nativelib.DefaultJniExtractor; +import org.scijava.nativelib.NativeLibraryUtil; + +import java.io.File; +import java.io.IOException; + +/** + * @author A. Tres Finocchiaro + * + * Stub DefaultJniExtractor class to allow native-lib-loader to conditionally + * use a statically defined native search path bootPath when provided. + */ +public class DefaultJniExtractorStub extends DefaultJniExtractor { + private File bootPath; + private boolean useStub; + + /** + * Default constructor + */ + public DefaultJniExtractorStub(Class libraryJarClass) throws IOException { + super(libraryJarClass); + useStub = false; + } + + /** + * Force native-lib-loader to first look in the location defined as bootPath + * prior to extracting a native library, useful for sandboxed environments. + * + * NativeLoader.setJniExtractor(new DefaultJniExtractorStub(null, "/opt/nativelibs"))); + * NativeLoader.loadLibrary("mylibrary"); + * + */ + public DefaultJniExtractorStub(Class libraryJarClass, String bootPath) throws IOException { + this(libraryJarClass); + this.bootPath = new File(bootPath); + + if(bootPath != null) { + File bootTest = new File(bootPath); + if(bootTest.exists()) { + // assume a static, existing directory will contain the native libs + this.useStub = true; + } else { + System.err.println("WARNING " + DefaultJniExtractorStub.class.getCanonicalName() + ": Boot path " + bootPath + " not found, falling back to default extraction behavior."); + } + } + } + + /** + * If a bootPath was provided to the constructor and exists, + * calculate the File path without any extraction logic. + * + * If a bootPath was NOT provided or does NOT exist, fallback on + * the default extraction behavior. + */ + @Override + public File extractJni(String libPath, String libName) throws IOException { + // Lie and pretend it's already extracted at the bootPath location + if(useStub) { + return new File(bootPath, NativeLibraryUtil.getPlatformLibraryName(libName)); + } + // Fallback on default behavior + return super.extractJni(libPath, libName); + } + + @Override + public void extractRegistered() throws IOException { + if(useStub) { + return; // no-op + } + super.extractRegistered(); + } +} diff --git a/src/main/java/jssc/DefaultJniExtractorWrapper.java b/src/main/java/jssc/DefaultJniExtractorWrapper.java deleted file mode 100644 index fbab0dd0d..000000000 --- a/src/main/java/jssc/DefaultJniExtractorWrapper.java +++ /dev/null @@ -1,56 +0,0 @@ -/** - * License: https://opensource.org/licenses/BSD-3-Clause - */ -package jssc; - -import org.scijava.nativelib.DefaultJniExtractor; -import org.scijava.nativelib.JniExtractor; - -import java.io.File; -import java.io.IOException; - -/** - * @author A. Tres Finocchiaro - * - * Wrapper around DefaultJniExtractor class to allow native-lib-loader to conditionally - * use a statically defined native search path when provided. - */ -public class DefaultJniExtractorWrapper { - private File bootPath; - private String library; - private JniExtractor extractor; - - public DefaultJniExtractorWrapper(String library, String bootPath) throws IOException { - if(bootPath != null) { - File bootTest = new File(bootPath); - if(bootTest.exists()) { - this.bootPath = bootTest; - this.library = library; - extractor = new StubJniExtractor(); - return; - } - } - extractor = new DefaultJniExtractor(null); - } - - public JniExtractor getExtractor() { - return extractor; - } - - public class StubJniExtractor implements JniExtractor { - @Override - public File extractJni(String s, String s1) { - switch(SerialNativeInterface.getOsType()) { - case SerialNativeInterface.OS_WINDOWS: - return new File(bootPath, library + ".dll"); - case SerialNativeInterface.OS_MAC_OS_X: - return new File(bootPath, "lib" + library + ".dylib"); - default: - return new File(bootPath, "lib" + library + ".so"); - } - } - - @Override - public void extractRegistered() {} // no-op - } -} diff --git a/src/main/java/jssc/SerialNativeInterface.java b/src/main/java/jssc/SerialNativeInterface.java index 3ccb9157f..b680de7e9 100644 --- a/src/main/java/jssc/SerialNativeInterface.java +++ b/src/main/java/jssc/SerialNativeInterface.java @@ -91,8 +91,8 @@ else if(osName.equals("Mac OS X") || osName.equals("Darwin")) * - Next, it will fallback to jssc.boot.library.path * - Finally it will attempt to extract the library from from the jssc.jar file, and load it. */ - NativeLoader.setJniExtractor( new DefaultJniExtractorWrapper("jssc", System.getProperty("jssc.boot.library.path")).getExtractor()); - NativeLoader.loadLibrary("jssc"); // method call is null-safe + NativeLoader.setJniExtractor( new DefaultJniExtractorStub(null, System.getProperty("jssc.boot.library.path"))); + NativeLoader.loadLibrary("jssc"); } catch (IOException ioException) { throw new UnsatisfiedLinkError("Could not load the jssc library: " + ioException.getMessage()); } From a4076f60a2c63cac04013a7dbcb2d1a39aa15181 Mon Sep 17 00:00:00 2001 From: Tres Finocchiaro Date: Mon, 2 Aug 2021 11:03:42 -0400 Subject: [PATCH 3/5] Fix formatting --- src/main/java/jssc/SerialNativeInterface.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/jssc/SerialNativeInterface.java b/src/main/java/jssc/SerialNativeInterface.java index b680de7e9..d9e54f791 100644 --- a/src/main/java/jssc/SerialNativeInterface.java +++ b/src/main/java/jssc/SerialNativeInterface.java @@ -91,7 +91,7 @@ else if(osName.equals("Mac OS X") || osName.equals("Darwin")) * - Next, it will fallback to jssc.boot.library.path * - Finally it will attempt to extract the library from from the jssc.jar file, and load it. */ - NativeLoader.setJniExtractor( new DefaultJniExtractorStub(null, System.getProperty("jssc.boot.library.path"))); + NativeLoader.setJniExtractor(new DefaultJniExtractorStub(null, System.getProperty("jssc.boot.library.path"))); NativeLoader.loadLibrary("jssc"); } catch (IOException ioException) { throw new UnsatisfiedLinkError("Could not load the jssc library: " + ioException.getMessage()); From 9074e350861ea181147e711ced117561bf583060 Mon Sep 17 00:00:00 2001 From: tresf Date: Wed, 4 Aug 2021 22:42:20 -0400 Subject: [PATCH 4/5] Add unit tests Simplify stub logic --- ant/build.xml | 6 ++++ pom.xml | 8 ++++- .../java/jssc/DefaultJniExtractorStub.java | 9 ++---- .../ManualBootLibraryPathFailedTest.java | 28 ++++++++++++++++ .../bootpath/ManualBootLibraryPathTest.java | 32 +++++++++++++++++++ 5 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 src/test/java/jssc/bootpath/ManualBootLibraryPathFailedTest.java create mode 100644 src/test/java/jssc/bootpath/ManualBootLibraryPathTest.java diff --git a/ant/build.xml b/ant/build.xml index 0d346b47d..41185542b 100644 --- a/ant/build.xml +++ b/ant/build.xml @@ -8,6 +8,7 @@ + @@ -193,6 +194,11 @@ + + + + + Tests will run only if the TARGET and HOST match:${line.separator}${line.separator} TARGET: ${os.target.classifier} diff --git a/pom.xml b/pom.xml index e0ea51cd4..f2e5d1f1d 100644 --- a/pom.xml +++ b/pom.xml @@ -68,7 +68,7 @@ 1.7.0 1.1 3.0.1 - 3.0.0-M3 + 3.0.0-M4 @@ -253,6 +253,12 @@ org.apache.maven.plugins maven-surefire-plugin ${plugin.surfire.version} + + false + + ${maven.exclude.tests} + + diff --git a/src/main/java/jssc/DefaultJniExtractorStub.java b/src/main/java/jssc/DefaultJniExtractorStub.java index e5a614520..e822eff2e 100644 --- a/src/main/java/jssc/DefaultJniExtractorStub.java +++ b/src/main/java/jssc/DefaultJniExtractorStub.java @@ -17,14 +17,12 @@ */ public class DefaultJniExtractorStub extends DefaultJniExtractor { private File bootPath; - private boolean useStub; /** * Default constructor */ public DefaultJniExtractorStub(Class libraryJarClass) throws IOException { super(libraryJarClass); - useStub = false; } /** @@ -37,13 +35,12 @@ public DefaultJniExtractorStub(Class libraryJarClass) throws IOException { */ public DefaultJniExtractorStub(Class libraryJarClass, String bootPath) throws IOException { this(libraryJarClass); - this.bootPath = new File(bootPath); if(bootPath != null) { File bootTest = new File(bootPath); if(bootTest.exists()) { // assume a static, existing directory will contain the native libs - this.useStub = true; + this.bootPath = bootTest; } else { System.err.println("WARNING " + DefaultJniExtractorStub.class.getCanonicalName() + ": Boot path " + bootPath + " not found, falling back to default extraction behavior."); } @@ -60,7 +57,7 @@ public DefaultJniExtractorStub(Class libraryJarClass, String bootPath) throws IO @Override public File extractJni(String libPath, String libName) throws IOException { // Lie and pretend it's already extracted at the bootPath location - if(useStub) { + if(bootPath != null) { return new File(bootPath, NativeLibraryUtil.getPlatformLibraryName(libName)); } // Fallback on default behavior @@ -69,7 +66,7 @@ public File extractJni(String libPath, String libName) throws IOException { @Override public void extractRegistered() throws IOException { - if(useStub) { + if(bootPath != null) { return; // no-op } super.extractRegistered(); diff --git a/src/test/java/jssc/bootpath/ManualBootLibraryPathFailedTest.java b/src/test/java/jssc/bootpath/ManualBootLibraryPathFailedTest.java new file mode 100644 index 000000000..890208e47 --- /dev/null +++ b/src/test/java/jssc/bootpath/ManualBootLibraryPathFailedTest.java @@ -0,0 +1,28 @@ +package jssc.bootpath; + +import jssc.SerialNativeInterface; +import org.junit.Test; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class ManualBootLibraryPathFailedTest { + @Test + public void testBootPathOverride() { + /** + * This must be in its own class to run in a separate JVM + * - See also: https://stackoverflow.com/questions/68657855 + * - NativeLoader.loadLibrary(...) calls System.loadLibrary(...) which is static + * - maven-surefire-plugin must be configured with reuseForks=false to use a new JVM for each class + * - TODO: If JUnit adds JVM unloading between methods, this class can be consolidated + */ + String nativeLibDir = "/"; // This should be valid on all platforms + System.setProperty("jssc.boot.library.path", nativeLibDir); + try { + SerialNativeInterface.getNativeLibraryVersion(); + fail("Library loading should fail if path provided exists but does not contain a native library"); + } catch (UnsatisfiedLinkError ignore) { + assertTrue("Library loading failed as expected with an invalid jssc.boot.library.path", true); + } + } +} diff --git a/src/test/java/jssc/bootpath/ManualBootLibraryPathTest.java b/src/test/java/jssc/bootpath/ManualBootLibraryPathTest.java new file mode 100644 index 000000000..6ca8309de --- /dev/null +++ b/src/test/java/jssc/bootpath/ManualBootLibraryPathTest.java @@ -0,0 +1,32 @@ +package jssc.bootpath; + +import jssc.SerialNativeInterface; +import org.junit.Test; +import org.scijava.nativelib.NativeLibraryUtil; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + +public class ManualBootLibraryPathTest { + @Test + public void testBootPathOverride() { + /** + * This must be in its own class to run in a separate JVM + * - See also: https://stackoverflow.com/questions/68657855 + * - NativeLoader.loadLibrary(...) calls System.loadLibrary(...) which is static + * - maven-surefire-plugin must be configured with reuseForks=false to use a new JVM for each class + * - TODO: If JUnit adds JVM unloading between methods, this class can be consolidated + */ + String nativeLibDir = NativeLibraryUtil.getPlatformLibraryPath(System.getProperty("user.dir") + "/target/cmake/natives/"); + System.setProperty("jssc.boot.library.path", nativeLibDir); + try { + final String nativeLibraryVersion = SerialNativeInterface.getNativeLibraryVersion(); + assertThat(nativeLibraryVersion, is(not(nullValue()))); + assertThat(nativeLibraryVersion, is(not(""))); + } catch (UnsatisfiedLinkError linkError) { + linkError.printStackTrace(); + fail("Should be able to call method!"); + } + } +} From 01c8095683b67febf8247413e38e13a3e4e99c00 Mon Sep 17 00:00:00 2001 From: tresf Date: Wed, 4 Aug 2021 23:47:30 -0400 Subject: [PATCH 5/5] Move comments to before class --- .../ManualBootLibraryPathFailedTest.java | 17 ++++++++++------- .../bootpath/ManualBootLibraryPathTest.java | 17 ++++++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/test/java/jssc/bootpath/ManualBootLibraryPathFailedTest.java b/src/test/java/jssc/bootpath/ManualBootLibraryPathFailedTest.java index 890208e47..2ae9ea678 100644 --- a/src/test/java/jssc/bootpath/ManualBootLibraryPathFailedTest.java +++ b/src/test/java/jssc/bootpath/ManualBootLibraryPathFailedTest.java @@ -6,16 +6,19 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +/** + * Tests if a valid jssc.boot.library.path which does NOT contain a native library + * will predictably fail. This test can be run regardless of whether or not a native binary was + * created during the build process. + * + * TODO: This MUST be in its own class to run in a separate JVM (https://stackoverflow.com/questions/68657855) + * - JUnit does NOT currently offer JVM unloading between methods. + * - maven-surefire-plugin DOES offer JVM unloading between classes using reuseForks=false + * - Unloading is needed due to NativeLoader.loadLibrary(...) calls System.loadLibrary(...) which is static + */ public class ManualBootLibraryPathFailedTest { @Test public void testBootPathOverride() { - /** - * This must be in its own class to run in a separate JVM - * - See also: https://stackoverflow.com/questions/68657855 - * - NativeLoader.loadLibrary(...) calls System.loadLibrary(...) which is static - * - maven-surefire-plugin must be configured with reuseForks=false to use a new JVM for each class - * - TODO: If JUnit adds JVM unloading between methods, this class can be consolidated - */ String nativeLibDir = "/"; // This should be valid on all platforms System.setProperty("jssc.boot.library.path", nativeLibDir); try { diff --git a/src/test/java/jssc/bootpath/ManualBootLibraryPathTest.java b/src/test/java/jssc/bootpath/ManualBootLibraryPathTest.java index 6ca8309de..77983d1b6 100644 --- a/src/test/java/jssc/bootpath/ManualBootLibraryPathTest.java +++ b/src/test/java/jssc/bootpath/ManualBootLibraryPathTest.java @@ -8,16 +8,19 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; +/** + * Tests if a valid jssc.boot.library.path which DOES contain a native library + * will predictably pass. This test can ONLY be run regardless if a native binary was created + * during the build process. See also maven.exclude.tests. + * + * TODO: This MUST be in its own class to run in a separate JVM (https://stackoverflow.com/questions/68657855) + * - JUnit does NOT currently offer JVM unloading between methods. + * - maven-surefire-plugin DOES offer JVM unloading between classes using reuseForks=false + * - Unloading is needed due to NativeLoader.loadLibrary(...) calls System.loadLibrary(...) which is static + */ public class ManualBootLibraryPathTest { @Test public void testBootPathOverride() { - /** - * This must be in its own class to run in a separate JVM - * - See also: https://stackoverflow.com/questions/68657855 - * - NativeLoader.loadLibrary(...) calls System.loadLibrary(...) which is static - * - maven-surefire-plugin must be configured with reuseForks=false to use a new JVM for each class - * - TODO: If JUnit adds JVM unloading between methods, this class can be consolidated - */ String nativeLibDir = NativeLibraryUtil.getPlatformLibraryPath(System.getProperty("user.dir") + "/target/cmake/natives/"); System.setProperty("jssc.boot.library.path", nativeLibDir); try {