From 0cb8d5b434b921edc3f337a398eea24f612fdac2 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 31 May 2024 16:51:00 -0400 Subject: [PATCH 1/9] Fix BugPatternNaming warnings --- .../test/java/com/uber/nullaway/jarinfer/JarInferTest.java | 6 +----- nullaway/src/main/java/com/uber/nullaway/NullAway.java | 2 -- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java index 897e96e2ed..aee882a69c 100644 --- a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java +++ b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java @@ -61,11 +61,7 @@ public class JarInferTest { * A dummy checker to allow us to use {@link CompilationTestHelper} to compile Java code for * testing, as it requires a {@link BugChecker} to run. */ - @BugPattern( - name = "DummyChecker", - summary = "Dummy checker to use CompilationTestHelper", - severity = WARNING) - @SuppressWarnings("BugPatternNaming") // remove once we require EP 2.11+ + @BugPattern(summary = "Dummy checker to use CompilationTestHelper", severity = WARNING) public static class DummyChecker extends BugChecker { public DummyChecker() {} } diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index e5de3a9e1b..401c54cb51 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -151,12 +151,10 @@ */ @AutoService(BugChecker.class) @BugPattern( - name = "NullAway", altNames = {"CheckNullabilityTypes"}, summary = "Nullability type error.", tags = BugPattern.StandardTags.LIKELY_ERROR, severity = WARNING) -@SuppressWarnings("BugPatternNaming") // remove once we require EP 2.11+ public class NullAway extends BugChecker implements BugChecker.MethodInvocationTreeMatcher, BugChecker.AssignmentTreeMatcher, From 9e4123ab249bd6fe42bb616bf64d2a760ae154ee Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 31 May 2024 16:59:39 -0400 Subject: [PATCH 2/9] another cleanup --- .../PreservedAnnotationTreeVisitor.java | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java b/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java index 42797b0ff6..51d5224951 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/PreservedAnnotationTreeVisitor.java @@ -266,27 +266,7 @@ public Type createWithBaseTypeAndTypeArgs(Type baseType, List typeArgs) { /** The TypeMetadataBuilder to be used for the current JDK version. */ private static final TypeMetadataBuilder TYPE_METADATA_BUILDER = - getVersion() >= 21 + Runtime.version().feature() >= 21 ? new JDK21TypeMetadataBuilder() : new JDK17AndEarlierTypeMetadataBuilder(); - - /** - * Utility method to get the current JDK version, that works on Java 8 and above. - * - *

TODO remove this method once we drop support for Java 8 - * - * @return the current JDK version - */ - private static int getVersion() { - String version = System.getProperty("java.version"); - if (version.startsWith("1.")) { - version = version.substring(2, 3); - } else { - int dot = version.indexOf("."); - if (dot != -1) { - version = version.substring(0, dot); - } - } - return Integer.parseInt(version); - } } From 44199709ad68d833713dde918dcf87bf61abd341 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 31 May 2024 17:07:32 -0400 Subject: [PATCH 3/9] more cleanup --- .../main/java/com/uber/nullaway/NullAway.java | 29 ++----------------- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index 401c54cb51..f5bff29c74 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -115,6 +115,7 @@ import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; import javax.lang.model.element.Modifier; +import javax.lang.model.element.ModuleElement; import javax.lang.model.element.NestingKind; import javax.lang.model.type.TypeKind; import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode; @@ -265,14 +266,6 @@ public Config getConfig() { */ private final Map computedNullnessMap = new LinkedHashMap<>(); - /** - * Used to check if a symbol represents a module in {@link #matchMemberSelect(MemberSelectTree, - * VisitorState)}. We need to use reflection to preserve compatibility with Java 8. - * - *

TODO remove this once NullAway requires JDK 11 - */ - @Nullable private final Class moduleElementClass; - /** * Error Prone requires us to have an empty constructor for each Plugin, in addition to the * constructor taking an ErrorProneFlags object. This constructor should not be used anywhere @@ -284,7 +277,6 @@ public NullAway() { handler = Handlers.buildEmpty(); nonAnnotatedMethod = this::isMethodUnannotated; errorBuilder = new ErrorBuilder(config, "", ImmutableSet.of()); - moduleElementClass = null; } @Inject // For future Error Prone versions in which checkers are loaded using Guice @@ -293,14 +285,6 @@ public NullAway(ErrorProneFlags flags) { handler = Handlers.buildDefault(config); nonAnnotatedMethod = this::isMethodUnannotated; errorBuilder = new ErrorBuilder(config, canonicalName(), allNames()); - Class moduleElementClass = null; - try { - moduleElementClass = - getClass().getClassLoader().loadClass("javax.lang.model.element.ModuleElement"); - } catch (ClassNotFoundException e) { - // can occur pre JDK 11 - } - this.moduleElementClass = moduleElementClass; } private boolean isMethodUnannotated(MethodInvocationNode invocationNode) { @@ -584,7 +568,7 @@ public Description matchMemberSelect(MemberSelectTree tree, VisitorState state) if (symbol == null || symbol.getSimpleName().toString().equals("class") || symbol.isEnum() - || isModuleSymbol(symbol)) { + || symbol instanceof ModuleElement) { return Description.NO_MATCH; } @@ -600,15 +584,6 @@ public Description matchMemberSelect(MemberSelectTree tree, VisitorState state) return Description.NO_MATCH; } - /** - * Checks if {@code symbol} represents a JDK 9+ module using reflection. - * - *

TODO just check using instanceof once NullAway requires JDK 11 - */ - private boolean isModuleSymbol(Symbol symbol) { - return moduleElementClass != null && moduleElementClass.isAssignableFrom(symbol.getClass()); - } - /** * Look for @NullMarked or @NullUnmarked annotations at the method level and adjust our scan for * annotated code accordingly (fast scan for a fully annotated/unannotated top-level class or From a68d6ed9a52ee8cf3ab1078f6e2b514644d41351 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 31 May 2024 17:21:02 -0400 Subject: [PATCH 4/9] rewrite jar signing code --- .../uber/nullaway/jarinfer/JarInferTest.java | 54 ++++++++----------- 1 file changed, 21 insertions(+), 33 deletions(-) diff --git a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java index aee882a69c..b79b125037 100644 --- a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java +++ b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java @@ -25,19 +25,15 @@ import com.google.errorprone.CompilationTestHelper; import com.google.errorprone.bugpatterns.BugChecker; import com.sun.tools.javac.main.Main; -import java.io.File; -import java.io.FileInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.nio.file.FileSystems; -import java.nio.file.Files; -import java.nio.file.Paths; -import java.nio.file.StandardCopyOption; -import java.security.MessageDigest; +import java.io.*; +import java.security.*; +import java.security.cert.CertificateException; import java.util.Arrays; import java.util.HashMap; import java.util.Map; import java.util.Set; +import java.util.zip.ZipFile; +import jdk.security.jarsigner.JarSigner; import org.apache.commons.io.FilenameUtils; import org.junit.Assert; import org.junit.Before; @@ -510,32 +506,24 @@ public void testSignedJars() throws Exception { /** copy the jar at {@code baseJarPath} to a signed jar at {@code signedJarPath} */ private void copyAndSignJar(String baseJarPath, String signedJarPath) - throws IOException, InterruptedException { - Files.copy( - Paths.get(baseJarPath), Paths.get(signedJarPath), StandardCopyOption.REPLACE_EXISTING); + throws CertificateException, + NoSuchAlgorithmException, + KeyStoreException, + IOException, + UnrecoverableEntryException { String ksPath = Thread.currentThread().getContextClassLoader().getResource("testKeyStore.jks").getPath(); - // A public API for signing jars was added for Java 9+, but there is only an internal API on - // Java 8. And we need the test code to compile on Java 8. For simplicity and uniformity, we - // just run jarsigner as an executable (which slightly slows down test execution) - String jarsignerExecutable = - String.join( - FileSystems.getDefault().getSeparator(), - new String[] {System.getenv("JAVA_HOME"), "bin", "jarsigner"}); - Process process = - Runtime.getRuntime() - .exec( - new String[] { - jarsignerExecutable, - "-keystore", - ksPath, - "-storepass", - "testPassword", - signedJarPath, - "testKeystore" - }); - int exitCode = process.waitFor(); - Assert.assertEquals("jarsigner process failed", 0, exitCode); + var ksPwd = "testPassword"; + var ksPwdArray = ksPwd.toCharArray(); + var keystore = KeyStore.getInstance(new File(ksPath), ksPwdArray); + var protParam = new KeyStore.PasswordProtection(ksPwdArray); + var pkEntry = (KeyStore.PrivateKeyEntry) keystore.getEntry("codecheck", protParam); + + JarSigner signer = new JarSigner.Builder(pkEntry).build(); + try (ZipFile in = new ZipFile(baseJarPath); + FileOutputStream out = new FileOutputStream(signedJarPath)) { + signer.sign(in, out); + } } private byte[] sha1sum(String path) throws Exception { From 47da1ac16d042ce85e2fb0197ccccbfdc47c52c3 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 31 May 2024 17:22:16 -0400 Subject: [PATCH 5/9] remove star imports --- .../com/uber/nullaway/jarinfer/JarInferTest.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java index b79b125037..5112547013 100644 --- a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java +++ b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java @@ -25,8 +25,16 @@ import com.google.errorprone.CompilationTestHelper; import com.google.errorprone.bugpatterns.BugChecker; import com.sun.tools.javac.main.Main; -import java.io.*; -import java.security.*; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.security.UnrecoverableEntryException; import java.security.cert.CertificateException; import java.util.Arrays; import java.util.HashMap; From fa160f33fc2eab7bad08b686b245079501acd627 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Fri, 31 May 2024 17:48:16 -0400 Subject: [PATCH 6/9] try fix --- .../src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java index 5112547013..b3fccfc18e 100644 --- a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java +++ b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java @@ -525,7 +525,7 @@ private void copyAndSignJar(String baseJarPath, String signedJarPath) var ksPwdArray = ksPwd.toCharArray(); var keystore = KeyStore.getInstance(new File(ksPath), ksPwdArray); var protParam = new KeyStore.PasswordProtection(ksPwdArray); - var pkEntry = (KeyStore.PrivateKeyEntry) keystore.getEntry("codecheck", protParam); + var pkEntry = (KeyStore.PrivateKeyEntry) keystore.getEntry(ksPwd, protParam); JarSigner signer = new JarSigner.Builder(pkEntry).build(); try (ZipFile in = new ZipFile(baseJarPath); From 9d60665df44507d02a56b8a2fa607a0b3cd479ed Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 1 Jun 2024 09:21:37 -0400 Subject: [PATCH 7/9] fix --- .../java/com/uber/nullaway/jarinfer/JarInferTest.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java index b3fccfc18e..8b6cdc06ef 100644 --- a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java +++ b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java @@ -515,17 +515,16 @@ public void testSignedJars() throws Exception { /** copy the jar at {@code baseJarPath} to a signed jar at {@code signedJarPath} */ private void copyAndSignJar(String baseJarPath, String signedJarPath) throws CertificateException, - NoSuchAlgorithmException, KeyStoreException, IOException, + NoSuchAlgorithmException, UnrecoverableEntryException { String ksPath = Thread.currentThread().getContextClassLoader().getResource("testKeyStore.jks").getPath(); - var ksPwd = "testPassword"; - var ksPwdArray = ksPwd.toCharArray(); - var keystore = KeyStore.getInstance(new File(ksPath), ksPwdArray); - var protParam = new KeyStore.PasswordProtection(ksPwdArray); - var pkEntry = (KeyStore.PrivateKeyEntry) keystore.getEntry(ksPwd, protParam); + var ksPassword = "testPassword".toCharArray(); + var keystore = KeyStore.getInstance(new File(ksPath), ksPassword); + var protParam = new KeyStore.PasswordProtection(ksPassword); + var pkEntry = (KeyStore.PrivateKeyEntry) keystore.getEntry("testKeystore", protParam); JarSigner signer = new JarSigner.Builder(pkEntry).build(); try (ZipFile in = new ZipFile(baseJarPath); From 53c894226753ba56ddf4a7295e5b16485d0d82b2 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 1 Jun 2024 11:32:42 -0400 Subject: [PATCH 8/9] fix test? --- .../src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java index 8b6cdc06ef..7f09c1af95 100644 --- a/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java +++ b/jar-infer/jar-infer-lib/src/test/java/com/uber/nullaway/jarinfer/JarInferTest.java @@ -526,7 +526,7 @@ private void copyAndSignJar(String baseJarPath, String signedJarPath) var protParam = new KeyStore.PasswordProtection(ksPassword); var pkEntry = (KeyStore.PrivateKeyEntry) keystore.getEntry("testKeystore", protParam); - JarSigner signer = new JarSigner.Builder(pkEntry).build(); + JarSigner signer = new JarSigner.Builder(pkEntry).digestAlgorithm("SHA-256").build(); try (ZipFile in = new ZipFile(baseJarPath); FileOutputStream out = new FileOutputStream(signedJarPath)) { signer.sign(in, out); From 09762dc235373fc6829328529701e112aec777e8 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sat, 1 Jun 2024 11:35:16 -0400 Subject: [PATCH 9/9] add comment --- .../java/com/uber/nullaway/jarinfer/BytecodeAnnotator.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/BytecodeAnnotator.java b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/BytecodeAnnotator.java index b0bc58eedd..c23f5214b1 100644 --- a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/BytecodeAnnotator.java +++ b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/BytecodeAnnotator.java @@ -249,6 +249,10 @@ private static void copyAndAnnotateJarEntry( String manifestText = stringBuilder.toString(); // Check for evidence of jar signing, note that lines can be split if too long so regex // matching line by line will have false negatives. + // NOTE: this code only handles the case where the message digest algorithm used when signing + // was SHA-256. Eventually we may need a more robust solution for other digest algorithms. + // E.g., on JDK 21, the default message digest algorithm is SHA-384, and this code does not + // work for that algorithm (the DIGEST_ENTRY_PATTERN regex is hardcoded for SHA-256) String manifestMinusDigests = manifestText.replaceAll(DIGEST_ENTRY_PATTERN, ""); if (!manifestText.equals(manifestMinusDigests) && !stripJarSignatures) { throw new SignedJarException(SIGNED_JAR_ERROR_MESSAGE);