From 2a1e74bfa00b3d6673d84930f29220e4ab2bbfee Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 4 Jun 2024 12:44:07 -0400 Subject: [PATCH] Various cleanups enabled by bumping minimum Java and Error Prone versions (#962) These warning suppressions and compatibility code are no longed needed given minimum JDK 11 and Error Prone 2.14.0 --- .../nullaway/jarinfer/BytecodeAnnotator.java | 4 ++ .../uber/nullaway/jarinfer/JarInferTest.java | 57 ++++++++----------- .../main/java/com/uber/nullaway/NullAway.java | 31 +--------- .../PreservedAnnotationTreeVisitor.java | 22 +------ 4 files changed, 31 insertions(+), 83 deletions(-) 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); 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..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 @@ -27,17 +27,21 @@ import com.sun.tools.javac.main.Main; import java.io.File; import java.io.FileInputStream; +import java.io.FileOutputStream; 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.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; 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; @@ -61,11 +65,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() {} } @@ -514,32 +514,23 @@ 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, + KeyStoreException, + IOException, + NoSuchAlgorithmException, + 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 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).digestAlgorithm("SHA-256").build(); + try (ZipFile in = new ZipFile(baseJarPath); + FileOutputStream out = new FileOutputStream(signedJarPath)) { + signer.sign(in, out); + } } private byte[] sha1sum(String path) throws Exception { diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index e5de3a9e1b..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; @@ -151,12 +152,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, @@ -267,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 @@ -286,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 @@ -295,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) { @@ -586,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; } @@ -602,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 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); - } }