Fix a bug where Netty fails to load a shaded native library (netty#12358

) Related: netty/netty-jni-util#13 Motivation: Netty can't load a shaded native library because `netty-jni-util` has a bug that appends an extra `/` when attmpting JNI `FindClass`. For example, `parsePackagePrefix()` returns `shaded//` instead of `shaded/`, leading to a bad attempt to load `shaded//io/netty/...`. Netty also doesn't handle the case where a shaded package name contains an underscore (`_`), such as `my_shaded_deps.io.netty.*`, because it simply replaces `.` with `_` and vice versa. JNI specification defines a mangling rule to avoid this issue: - https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#resolving_native_method_names Modifications: - Replace `_` into `_1` so that `parsePackagePrefix()` in `netty-jni-utils.c` can get the correct package name later. - Update the `docker-compose.yaml` so that the integration tests in `testsuite-shading` are always run as a part of CI, so we don't have a regression in the future. Result: - A user can use a functionality that requires a native library even if they shaded Netty *and* the shaded package name contains an underscore (`_`).
raidyue · Jul 8, 2022 · 549e6ab · 549e6ab
1 parent 435dde5
commit 549e6ab
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 14 deletions.
diff --git a/common/src/main/java/io/netty/util/internal/NativeLibraryLoader.java b/common/src/main/java/io/netty/util/internal/NativeLibraryLoader.java
@@ -117,11 +117,25 @@ public static void loadFirstAvailable(ClassLoader loader, String... names) {
     }
 
     /**
-     * The shading prefix added to this class's full name.
+     * Calculates the mangled shading prefix added to this class's full name.
+     *
+     * <p>This method mangles the package name as follows, so we can unmangle it back later:
+     * <ul>
+     *   <li>{@code _} to {@code _1}</li>
+     *   <li>{@code .} to {@code _}</li>
+     * </ul>
+     *
+     * <p>Note that we don't mangle non-ASCII characters here because it's extremely unlikely to have
+     * a non-ASCII character in a package name. For more information, see:
+     * <ul>
+     *   <li><a href="https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html">JNI
+     *       specification</a></li>
+     *   <li>{@code parsePackagePrefix()} in {@code netty_jni_util.c}.</li>
+     * </ul>
      *
      * @throws UnsatisfiedLinkError if the shader used something other than a prefix
      */
-    private static String calculatePackagePrefix() {
+    private static String calculateMangledPackagePrefix() {
         String maybeShaded = NativeLibraryLoader.class.getName();
         // Use ! instead of . to avoid shading utilities from modifying the string
         String expected = "io!netty!util!internal!NativeLibraryLoader".replace('!', '.');
@@ -130,16 +144,17 @@ private static String calculatePackagePrefix() {
                     "Could not find prefix added to %s to get %s. When shading, only adding a "
                     + "package prefix is supported", expected, maybeShaded));
         }
-        return maybeShaded.substring(0, maybeShaded.length() - expected.length());
+        return maybeShaded.substring(0, maybeShaded.length() - expected.length())
+                          .replace("_", "_1")
+                          .replace('.', '_');
     }
 
     /**
      * Load the given library with the specified {@link ClassLoader}
      */
     public static void load(String originalName, ClassLoader loader) {
-        // Adjust expected name to support shading of native libraries.
-        String packagePrefix = calculatePackagePrefix().replace('.', '_');
-        String name = packagePrefix + originalName;
+        String mangledPackagePrefix = calculateMangledPackagePrefix();
+        String name = mangledPackagePrefix + originalName;
         List<Throwable> suppressed = new ArrayList<Throwable>();
         try {
             // first try to load from java.library.path
@@ -189,7 +204,7 @@ public static void load(String originalName, ClassLoader loader) {
             }
             out.flush();
 
-            if (shouldShadedLibraryIdBePatched(packagePrefix)) {
+            if (shouldShadedLibraryIdBePatched(mangledPackagePrefix)) {
                 // Let's try to patch the id and re-sign it. This is a best-effort and might fail if a
                 // SecurityManager is setup or the right executables are not installed :/
                 tryPatchShadedLibraryIdAndSign(tmpFile, originalName);

diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml
@@ -29,7 +29,11 @@ services:
 
   build:
     <<: *common
-    command: /bin/bash -cl "./mvnw -B -ntp clean install -Dio.netty.testsuite.badHost=netty.io -Dtcnative.classifier=linux-x86_64-fedora"
+    command: '/bin/bash -cl "
+      ./mvnw -B -ntp clean install -Dio.netty.testsuite.badHost=netty.io -Dtcnative.classifier=linux-x86_64-fedora && 
+      cd testsuite-shading &&
+      ../mvnw -B -ntp integration-test failsafe:verify -Dtcnative.classifier=linux-x86_64-fedora
+    "'
 
   deploy:
     <<: *common

diff --git a/pom.xml b/pom.xml
@@ -621,7 +621,7 @@
       <dependency>
         <groupId>io.netty</groupId>
         <artifactId>netty-jni-util</artifactId>
-        <version>0.0.5.Final</version>
+        <version>0.0.6.Final</version>
         <classifier>sources</classifier>
         <optional>true</optional>
       </dependency>

diff --git a/testsuite-shading/pom.xml b/testsuite-shading/pom.xml
@@ -34,7 +34,9 @@
     <classesShadedDir>${project.build.directory}/classes-shaded</classesShadedDir>
     <classesShadedNativeDir>${classesShadedDir}/META-INF/native</classesShadedNativeDir>
     <shadingPrefix>shaded</shadingPrefix>
-    <shadingPrefix2>shaded2</shadingPrefix2>
+    <shadingPrefix2>shaded_2</shadingPrefix2>
+    <mangledShadingPrefix>shaded</mangledShadingPrefix>
+    <mangledShadingPrefix2>shaded_12</mangledShadingPrefix2>
     <skipShadingTestsuite>true</skipShadingTestsuite>
     <shadedPackagePrefix>io.netty.</shadedPackagePrefix>
     <japicmp.skip>true</japicmp.skip>
@@ -126,12 +128,12 @@
                     <include name="shaded2.jar" />
                   </fileset>
                 </unzip>
-                <copy file="${classesShadedNativeDir}/lib${nativeTransportLib}" tofile="${classesShadedNativeDir}/lib${shadingPrefix}_${nativeTransportLib}" />
-                <copy file="${classesShadedNativeDir}/lib${nativeTransportLib}" tofile="${classesShadedNativeDir}/lib${shadingPrefix2}_${nativeTransportLib}" />
+                <copy file="${classesShadedNativeDir}/lib${nativeTransportLib}" tofile="${classesShadedNativeDir}/lib${mangledShadingPrefix}_${nativeTransportLib}" />
+                <copy file="${classesShadedNativeDir}/lib${nativeTransportLib}" tofile="${classesShadedNativeDir}/lib${mangledShadingPrefix2}_${nativeTransportLib}" />
                 <delete file="${classesShadedNativeDir}/lib${nativeTransportLib}" />
 
-                <copy file="${classesShadedNativeDir}/lib${nativeTcnativeLib}" tofile="${classesShadedNativeDir}/lib${shadingPrefix}_${nativeTcnativeLib}" />
-                <copy file="${classesShadedNativeDir}/lib${nativeTcnativeLib}" tofile="${classesShadedNativeDir}/lib${shadingPrefix2}_${nativeTcnativeLib}" />
+                <copy file="${classesShadedNativeDir}/lib${nativeTcnativeLib}" tofile="${classesShadedNativeDir}/lib${mangledShadingPrefix}_${nativeTcnativeLib}" />
+                <copy file="${classesShadedNativeDir}/lib${nativeTcnativeLib}" tofile="${classesShadedNativeDir}/lib${mangledShadingPrefix2}_${nativeTcnativeLib}" />
                 <delete file="${classesShadedNativeDir}/lib${nativeTcnativeLib}" />
 
                 <delete file="${project.build.directory}/shaded1.jar" />