Skip to content

Commit

Permalink
Fix a bug where Netty fails to load a shaded native library (netty#12358
Browse files Browse the repository at this point in the history
)


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
  (`_`).
  • Loading branch information
trustin authored and franz1981 committed Aug 22, 2022
1 parent 0c0aae3 commit f52cc28
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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('!', '.');
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion docker/docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
12 changes: 7 additions & 5 deletions testsuite-shading/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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" />
Expand Down

0 comments on commit f52cc28

Please sign in to comment.