Open ytoku opened 5 months ago
Ah, good catch! Thanks for reporting that. It looks like its actually been around since 4.0 (since https://github.com/netty/netty/commit/c58f28dfdd4a75708d6ffa909d7d1ab1e6a3fadc) but you're right regardless, it's always an inner class.
Can you tweak the hook to fix that, test that it does work correctly for you, and then open a PR?
Changing to FingerprintTrustManagerFactory$2
is an easy fix.
But I wonder if a compiler always names it as FingerprintTrustManagerFactory$2
.
Could it be FingerprintTrustManagerFactory$1
?
One possible robust design is to enumerate anonymous inner classes and patches specified methods.
May I open a PR only supporting for FingerprintTrustManagerFactory$2
?
May I open a PR only supporting for FingerprintTrustManagerFactory$2?
Yep, go for it :+1:
My understanding is that anonymous inner classes are stored with numbers as $x based on the order they appear in the source file. This is the 2nd inner class (the first being the FastThreadLocal
just above).
So it looks like $2
will be correct for all releases so far, over the 9 years since this was added, and if it changes in future releases then we can always update to add more extra cases as required.
I have tried to test the above change, but I have found some problems.
Testing application: https://github.com/ytoku/NettyPinningTest
FingerprintTrustManagerFactory$1
. Because TrustManager
anonymous class is placed before FastThreadLocal
anonymous class.FingerprintTrustManagerFactory
without SSLContext
. When I remove Native SSLContext patch, the patch for FingerprintTrustManagerFactory$1
works correctly.In netty-4.1.104Final (I tested), the anonymous inner class becomes FingerprintTrustManagerFactory$1. Because TrustManager anonymous class is placed before FastThreadLocal anonymous class.
Huh, that's surprising! I think it's fine though - just add two identical patches (for $1 and $2) and it will just skip the other one anyway (since it won't find a matching checkTrusted
method in FastThreadLocal
).
It's a bit messy, but I think that's OK at this stage, it's not a big problem. It'd be very helpful if you could add comments explaining that (and preferably documenting the versions the patches apply to, if you can work that out) so we remember what's going on here later on.
For Netty, the current version patches the
checkTrusted
method inio.netty.handler.ssl.util.FingerprintTrustManagerFactory
.But actual
checkTrusted
method is in an anonymous inner class such asio.netty.handler.ssl.util.FingerprintTrustManagerFactory$2
.The first version which has
checkTrusted
method: netty-4.0.20.Final https://github.com/netty/netty/blob/1709113a1f27be021e890d07c4d94576e2e7710c/handler/src/main/java/io/netty/handler/ssl/util/FingerprintTrustManagerFactory.java#L95The latest version: netty-4.1.101Final https://github.com/netty/netty/blob/c6a6aadaface1b2b66d2608dcdc6e4c04c1648cc/handler/src/main/java/io/netty/handler/ssl/util/FingerprintTrustManagerFactory.java#L111