From 945d8b54bce63d7454238e11e08d073b88748378 Mon Sep 17 00:00:00 2001 From: exceptionfactory Date: Wed, 1 Nov 2023 13:10:00 -0500 Subject: [PATCH] NIFI-12294 Standardized NAR Entry Loading (#7958) - Consolidated duplicative NAR file entry normalization --- .../org/apache/nifi/nar/NarUnpackerTest.java | 49 ++++----- .../java/org/apache/nifi/nar/NarUnpacker.java | 99 ++++++++++--------- 2 files changed, 72 insertions(+), 76 deletions(-) diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-nar-utils/src/test/java/org/apache/nifi/nar/NarUnpackerTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-nar-utils/src/test/java/org/apache/nifi/nar/NarUnpackerTest.java index beb673f0a8..5733d1bb60 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-nar-utils/src/test/java/org/apache/nifi/nar/NarUnpackerTest.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-nar-utils/src/test/java/org/apache/nifi/nar/NarUnpackerTest.java @@ -33,6 +33,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.stream.Stream; @@ -44,17 +45,17 @@ import static org.junit.jupiter.api.Assertions.assertTrue; public class NarUnpackerTest { + private static final String PROPERTIES_PATH = "/NarUnpacker/conf/nifi.properties"; + @BeforeAll public static void copyResources() throws IOException { - final Path sourcePath = Paths.get("./src/test/resources"); final Path targetPath = Paths.get("./target"); - Files.walkFileTree(sourcePath, new SimpleFileVisitor() { + Files.walkFileTree(sourcePath, new SimpleFileVisitor<>() { @Override - public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) - throws IOException { + public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { Path relativeSource = sourcePath.relativize(dir); Path target = targetPath.resolve(relativeSource); @@ -62,12 +63,10 @@ public class NarUnpackerTest { Files.createDirectories(target); return FileVisitResult.CONTINUE; - } @Override - public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) - throws IOException { + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { Path relativeSource = sourcePath.relativize(file); Path target = targetPath.resolve(relativeSource); @@ -81,8 +80,7 @@ public class NarUnpackerTest { @Test public void testUnpackNars() { - - NiFiProperties properties = loadSpecifiedProperties("/NarUnpacker/conf/nifi.properties", Collections.EMPTY_MAP); + NiFiProperties properties = loadSpecifiedProperties(Collections.emptyMap()); assertEquals("./target/NarUnpacker/lib/", properties.getProperty("nifi.nar.library.directory")); @@ -93,10 +91,10 @@ public class NarUnpackerTest { assertEquals(2, extensionMapping.getAllExtensionNames().size()); - assertTrue(extensionMapping.getAllExtensionNames().keySet().contains("org.apache.nifi.processors.dummy.one")); - assertTrue(extensionMapping.getAllExtensionNames().keySet().contains("org.apache.nifi.processors.dummy.two")); + assertTrue(extensionMapping.getAllExtensionNames().containsKey("org.apache.nifi.processors.dummy.one")); + assertTrue(extensionMapping.getAllExtensionNames().containsKey("org.apache.nifi.processors.dummy.two")); final File extensionsWorkingDir = properties.getExtensionsWorkingDirectory(); - File[] extensionFiles = extensionsWorkingDir.listFiles(); + File[] extensionFiles = Objects.requireNonNull(extensionsWorkingDir.listFiles()); Set expectedNars = new HashSet<>(); expectedNars.add("dummy-one.nar-unpacked"); @@ -110,24 +108,22 @@ public class NarUnpackerTest { } @Test - public void testUnpackNarsFromEmptyDir() throws IOException { - + public void testUnpackNarsFromEmptyDir() { final File emptyDir = new File("./target/empty/dir"); - emptyDir.delete(); emptyDir.deleteOnExit(); assertTrue(emptyDir.mkdirs()); final Map others = new HashMap<>(); others.put("nifi.nar.library.directory.alt", emptyDir.toString()); - NiFiProperties properties = loadSpecifiedProperties("/NarUnpacker/conf/nifi.properties", others); + NiFiProperties properties = loadSpecifiedProperties(others); final ExtensionMapping extensionMapping = NarUnpacker.unpackNars(properties, SystemBundle.create(properties), NarUnpackMode.UNPACK_INDIVIDUAL_JARS); assertEquals(1, extensionMapping.getAllExtensionNames().size()); - assertTrue(extensionMapping.getAllExtensionNames().keySet().contains("org.apache.nifi.processors.dummy.one")); + assertTrue(extensionMapping.getAllExtensionNames().containsKey("org.apache.nifi.processors.dummy.one")); final File extensionsWorkingDir = properties.getExtensionsWorkingDirectory(); - File[] extensionFiles = extensionsWorkingDir.listFiles(); + File[] extensionFiles = Objects.requireNonNull(extensionsWorkingDir.listFiles()); assertEquals(2, extensionFiles.length); @@ -139,23 +135,21 @@ public class NarUnpackerTest { @Test public void testUnpackNarsFromNonExistantDir() { - final File nonExistantDir = new File("./target/this/dir/should/not/exist/"); - nonExistantDir.delete(); nonExistantDir.deleteOnExit(); final Map others = new HashMap<>(); others.put("nifi.nar.library.directory.alt", nonExistantDir.toString()); - NiFiProperties properties = loadSpecifiedProperties("/NarUnpacker/conf/nifi.properties", others); + NiFiProperties properties = loadSpecifiedProperties(others); final ExtensionMapping extensionMapping = NarUnpacker.unpackNars(properties, SystemBundle.create(properties), NarUnpackMode.UNPACK_INDIVIDUAL_JARS); - assertTrue(extensionMapping.getAllExtensionNames().keySet().contains("org.apache.nifi.processors.dummy.one")); + assertTrue(extensionMapping.getAllExtensionNames().containsKey("org.apache.nifi.processors.dummy.one")); assertEquals(1, extensionMapping.getAllExtensionNames().size()); final File extensionsWorkingDir = properties.getExtensionsWorkingDirectory(); - File[] extensionFiles = extensionsWorkingDir.listFiles(); + File[] extensionFiles = Objects.requireNonNull(extensionsWorkingDir.listFiles()); assertEquals(2, extensionFiles.length); @@ -167,24 +161,23 @@ public class NarUnpackerTest { @Test public void testUnpackNarsFromNonDir() throws IOException { - final File nonDir = new File("./target/file.txt"); - nonDir.createNewFile(); + assertTrue(nonDir.createNewFile()); nonDir.deleteOnExit(); final Map others = new HashMap<>(); others.put("nifi.nar.library.directory.alt", nonDir.toString()); - NiFiProperties properties = loadSpecifiedProperties("/NarUnpacker/conf/nifi.properties", others); + NiFiProperties properties = loadSpecifiedProperties(others); final ExtensionMapping extensionMapping = NarUnpacker.unpackNars(properties, SystemBundle.create(properties), NarUnpackMode.UNPACK_INDIVIDUAL_JARS); assertNull(extensionMapping); } - private NiFiProperties loadSpecifiedProperties(final String propertiesFile, final Map others) { + private NiFiProperties loadSpecifiedProperties(final Map others) { String filePath; try { - filePath = NarUnpackerTest.class.getResource(propertiesFile).toURI().getPath(); + filePath = Objects.requireNonNull(NarUnpackerTest.class.getResource(PROPERTIES_PATH)).toURI().getPath(); } catch (URISyntaxException ex) { throw new RuntimeException("Cannot load properties file due to " + ex.getLocalizedMessage(), ex); } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-nar-utils/src/main/java/org/apache/nifi/nar/NarUnpacker.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-nar-utils/src/main/java/org/apache/nifi/nar/NarUnpacker.java index 52b0dd58a0..7f2a49014c 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-nar-utils/src/main/java/org/apache/nifi/nar/NarUnpacker.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-nar-utils/src/main/java/org/apache/nifi/nar/NarUnpacker.java @@ -40,6 +40,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; @@ -55,13 +56,11 @@ import java.util.jar.JarInputStream; import java.util.jar.JarOutputStream; import java.util.jar.Manifest; -import static java.lang.String.format; - -/** - * - */ public final class NarUnpacker { public static final String BUNDLED_DEPENDENCIES_DIRECTORY = "NAR-INF/bundled-dependencies"; + + private static final String BUNDLED_DEPENDENCIES_PREFIX = "META-INF/bundled-dependencies"; + private static final Logger logger = LoggerFactory.getLogger(NarUnpacker.class); private static final String HASH_FILENAME = "nar-digest"; private static final FileFilter NAR_FILTER = pathname -> { @@ -93,8 +92,6 @@ public final class NarUnpacker { final boolean requireFrameworkNar, final String frameworkNarId, final boolean requireJettyNar, final boolean verifyHash, final NarUnpackMode unpackMode, final Predicate narFilter) { - final Map unpackedNars = new HashMap<>(); - try { File unpackedJetty = null; File unpackedFramework = null; @@ -128,13 +125,13 @@ public final class NarUnpacker { if (!narFiles.isEmpty()) { final long startTime = System.nanoTime(); - logger.info("Expanding " + narFiles.size() + " NAR files with all processors..."); + logger.info("Expanding {} NAR files started", narFiles.size()); for (File narFile : narFiles) { if (!narFile.canRead()) { throw new IllegalStateException("Unable to read NAR file: " + narFile.getAbsolutePath()); } - logger.debug("Expanding NAR file: " + narFile.getAbsolutePath()); + logger.debug("Expanding NAR file: {}", narFile.getAbsolutePath()); // get the manifest for this nar try (final JarFile nar = new JarFile(narFile)) { @@ -211,11 +208,11 @@ public final class NarUnpacker { } final long duration = System.nanoTime() - startTime; - logger.info("NAR loading process took " + duration + " nanoseconds " - + "(" + (int) TimeUnit.SECONDS.convert(duration, TimeUnit.NANOSECONDS) + " seconds)."); + final double durationSeconds = TimeUnit.NANOSECONDS.toMillis(duration) / 1000.0; + logger.info("Expanded {} NAR files in {} seconds ({} ns)", narFiles.size(), durationSeconds, duration); } - unpackedNars.putAll(createUnpackedNarBundleCoordinateMap(extensionsWorkingDir)); + final Map unpackedNars = new HashMap<>(createUnpackedNarBundleCoordinateMap(extensionsWorkingDir)); final ExtensionMapping extensionMapping = new ExtensionMapping(); mapExtensions(unpackedNars, docsWorkingDir, extensionMapping); @@ -224,7 +221,7 @@ public final class NarUnpacker { return extensionMapping; } catch (IOException e) { - logger.warn("Unable to load NAR library bundles due to {} Will proceed without loading any further Nar bundles", e.toString(), e); + logger.warn("Unable to load NAR bundles. Proceeding without loading any further NAR bundles", e); } return null; @@ -236,8 +233,12 @@ public final class NarUnpacker { * @return map of coordinates for bundles */ private static Map createUnpackedNarBundleCoordinateMap(File extensionsWorkingDir) { - Map result = new HashMap<>(); File[] unpackedDirs = extensionsWorkingDir.listFiles(file -> file.isDirectory() && file.getName().endsWith("nar-unpacked")); + if (unpackedDirs == null) { + return Collections.emptyMap(); + } + + final Map result = new HashMap<>(); for (File unpackedDir : unpackedDirs) { Path mf = Paths.get(unpackedDir.getAbsolutePath(), "META-INF", "MANIFEST.MF"); try(InputStream is = Files.newInputStream(mf)) { @@ -245,19 +246,18 @@ public final class NarUnpacker { BundleCoordinate bundleCoordinate = createBundleCoordinate(manifest); result.put(unpackedDir, bundleCoordinate); } catch (IOException e) { - logger.error(format("Unable to parse NAR information from unpacked nar directory [%s].", unpackedDir.getAbsoluteFile()), e); + logger.error("Unable to parse NAR information from unpacked directory [{}]", unpackedDir.getAbsoluteFile(), e); } } return result; } - private static BundleCoordinate createBundleCoordinate(Manifest manifest) { + private static BundleCoordinate createBundleCoordinate(final Manifest manifest) { Attributes mainAttributes = manifest.getMainAttributes(); String groupId = mainAttributes.getValue(NarManifestEntry.NAR_GROUP.getManifestName()); String narId = mainAttributes.getValue(NarManifestEntry.NAR_ID.getManifestName()); String version = mainAttributes.getValue(NarManifestEntry.NAR_VERSION.getManifestName()); - BundleCoordinate bundleCoordinate = new BundleCoordinate(groupId, narId, version); - return bundleCoordinate; + return new BundleCoordinate(groupId, narId, version); } private static void mapExtensions(final Map unpackedNars, final File docsDirectory, final ExtensionMapping mapping) throws IOException { @@ -319,7 +319,7 @@ public final class NarUnpacker { } else { final byte[] hashFileContents = Files.readAllBytes(workingHashFile.toPath()); if (!Arrays.equals(hashFileContents, narDigest)) { - logger.info("Contents of nar {} have changed. Reloading.", new Object[] { nar.getAbsolutePath() }); + logger.info("Reloading changed NAR [{}]", nar.getAbsolutePath()); FileUtils.deleteFile(narWorkingDirectory, true); unpackIndividualJars(nar, narWorkingDirectory, narDigest, unpackMode); } @@ -353,16 +353,11 @@ public final class NarUnpacker { final Enumeration jarEntries = jarFile.entries(); while (jarEntries.hasMoreElements()) { final JarEntry jarEntry = jarEntries.nextElement(); - String name = jarEntry.getName(); - if (name.contains("META-INF/bundled-dependencies")){ - name = name.replace("META-INF/bundled-dependencies", BUNDLED_DEPENDENCIES_DIRECTORY); - } - - final File f = new File(workingDirectory, name); + final File jarEntryFile = getMappedJarEntryFile(workingDirectory, jarEntry); if (jarEntry.isDirectory()) { - FileUtils.ensureDirectoryExistAndCanReadAndWrite(f); + FileUtils.ensureDirectoryExistAndCanReadAndWrite(jarEntryFile); } else { - makeFile(jarFile.getInputStream(jarEntry), f); + makeFile(jarFile.getInputStream(jarEntry), jarEntryFile); } } } @@ -398,31 +393,27 @@ public final class NarUnpacker { final Enumeration jarEntries = jarFile.entries(); while (jarEntries.hasMoreElements()) { final JarEntry jarEntry = jarEntries.nextElement(); - String name = jarEntry.getName(); - if (name.contains("META-INF/bundled-dependencies")){ - name = name.replace("META-INF/bundled-dependencies", BUNDLED_DEPENDENCIES_DIRECTORY); - } - - logger.debug("Unpacking NAR entry {}", name); + final File jarEntryFile = getMappedJarEntryFile(workingDirectory, jarEntry); + logger.debug("Unpacking NAR entry {}", jarEntryFile); // If we've not yet created this entry, create it now. If we've already created the entry, ignore it. - if (!entriesCreated.add(name)) { + if (!entriesCreated.add(jarEntry.getName())) { continue; } // Explode anything from META-INF and any WAR files into the nar's output directory instead of copying it to the uber jar. // The WAR files are important so that NiFi can load its UI. The META-INF/ directory is important in order to ensure that our // NarClassLoader has all of the information that it needs. - if (name.contains("META-INF/") || (name.contains("NAR-INF") && name.endsWith(".war"))) { + final String jarEntryFilePath = jarEntryFile.getAbsolutePath(); + if (jarEntryFilePath.contains("META-INF") || (jarEntryFilePath.contains("NAR-INF") && jarEntryFilePath.endsWith(".war"))) { if (jarEntry.isDirectory()) { continue; } - final File outFile = new File(workingDirectory, name); - Files.createDirectories(outFile.getParentFile().toPath()); + Files.createDirectories(jarEntryFile.getParentFile().toPath()); try (final InputStream entryIn = jarFile.getInputStream(jarEntry); - final OutputStream manifestOut = new FileOutputStream(outFile)) { + final OutputStream manifestOut = new FileOutputStream(jarEntryFile)) { copy(entryIn, manifestOut); } @@ -431,9 +422,9 @@ public final class NarUnpacker { if (jarEntry.isDirectory()) { uberJarOut.putNextEntry(new JarEntry(jarEntry.getName())); - } else if (name.endsWith(".jar")) { + } else if (jarEntryFilePath.endsWith(".jar")) { // Unpack each .jar file into the uber jar, taking care to deal with META-INF/ files, etc. carefully. - logger.debug("Unpacking Jar {}", name); + logger.debug("Unpacking JAR {}", jarEntryFile); try (final InputStream entryIn = jarFile.getInputStream(jarEntry); final InputStream in = new BufferedInputStream(entryIn)) { @@ -475,6 +466,7 @@ public final class NarUnpacker { JarEntry jarEntry; while ((jarEntry = jarInputStream.getNextJarEntry()) != null) { final String entryName = jarEntry.getName(); + final File outFile = getJarEntryFile(workingDirectory, entryName); // The META-INF/ directory can contain several different types of files. For example, it contains: // MANIFEST.MF @@ -491,8 +483,6 @@ public final class NarUnpacker { if ((entryName.contains("META-INF/") && !entryName.contains("META-INF/MANIFEST.MF") ) && !jarEntry.isDirectory()) { logger.debug("Found META-INF/services file {}", entryName); - final File outFile = new File(workingDirectory, entryName); - // Because we're combining multiple jar files into one, we can run into situations where there may be conflicting filenames // such as 1 jar has a file named META-INF/license and another jar file has a META-INF/license/my-license.txt. We can generally // just ignore these, though, as they are not necessary in this temporarily created jar file. So we log it at a debug level and @@ -569,10 +559,12 @@ public final class NarUnpacker { // go through each entry in this jar for (final Enumeration jarEnumeration = jarFile.entries(); jarEnumeration.hasMoreElements();) { final JarEntry jarEntry = jarEnumeration.nextElement(); + final File jarEntryFile = getJarEntryFile(docsDirectory, jarEntry.getName()); + final String jarEntryName = jarEntryFile.getName(); // if this entry is documentation for this component - if (jarEntry.getName().startsWith(entryName)) { - final String name = StringUtils.substringAfter(jarEntry.getName(), "docs/"); + if (jarEntryName.startsWith(entryName)) { + final String name = StringUtils.substringAfter(jarEntryName, "docs/"); final String path = coordinate.getGroup() + "/" + coordinate.getId() + "/" + coordinate.getVersion() + "/" + name; // if this is a directory create it @@ -581,7 +573,7 @@ public final class NarUnpacker { // ensure the documentation directory can be created if (!componentDocsDirectory.exists() && !componentDocsDirectory.mkdirs()) { - logger.warn("Unable to create docs directory " + componentDocsDirectory.getAbsolutePath()); + logger.warn("Unable to create docs directory {}", componentDocsDirectory.getAbsolutePath()); break; } } else { @@ -596,9 +588,6 @@ public final class NarUnpacker { } } - /* - * Returns true if this jar file contains a NiFi component - */ private static ExtensionMapping determineDocumentedNiFiComponents(final BundleCoordinate coordinate, final File jar) throws IOException { final ExtensionMapping mapping = new ExtensionMapping(); @@ -670,6 +659,20 @@ public final class NarUnpacker { } } + private static File getMappedJarEntryFile(final File workingDirectory, final JarEntry jarEntry) { + final String jarEntryName = jarEntry.getName().replace(BUNDLED_DEPENDENCIES_PREFIX, BUNDLED_DEPENDENCIES_DIRECTORY); + return getJarEntryFile(workingDirectory, jarEntryName); + } + + private static File getJarEntryFile(final File workingDirectory, final String jarEntryName) { + final Path workingDirectoryPath = workingDirectory.toPath().normalize(); + final Path jarEntryPath = workingDirectoryPath.resolve(jarEntryName).normalize(); + if (jarEntryPath.startsWith(workingDirectoryPath)) { + return jarEntryPath.toFile(); + } + throw new IllegalArgumentException(String.format("NAR Entry path not valid [%s]", jarEntryName)); + } + private NarUnpacker() { } }