From 8e210420560cc573ebf5ad5c95fe8acb0a5b97b7 Mon Sep 17 00:00:00 2001 From: nikhilerigila09 Date: Thu, 12 Sep 2024 10:56:17 +0530 Subject: [PATCH 1/3] Fix for nar unpacking --- .../apache/pulsar/common/nar/NarUnpacker.java | 29 ++++++++++++++----- .../pulsar/common/nar/NarUnpackerTest.java | 24 +++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/nar/NarUnpacker.java b/pulsar-common/src/main/java/org/apache/pulsar/common/nar/NarUnpacker.java index e1806836d2833..ef802674b421a 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/nar/NarUnpacker.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/nar/NarUnpacker.java @@ -32,7 +32,9 @@ import java.io.RandomAccessFile; import java.nio.channels.FileChannel; import java.nio.channels.FileLock; +import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.Base64; @@ -86,19 +88,32 @@ static File doUnpackNar(final File nar, final File baseWorkingDirectory, Runnabl try (FileChannel channel = new RandomAccessFile(lockFile, "rw").getChannel(); FileLock lock = channel.lock()) { File narWorkingDirectory = new File(parentDirectory, md5Sum); - if (narWorkingDirectory.mkdir()) { + if (!narWorkingDirectory.exists()) { + File narExtractionTempDirectory = new File(parentDirectory, md5Sum + ".tmp"); + if (narExtractionTempDirectory.exists()) { + FileUtils.deleteFile(narExtractionTempDirectory, true); + } + if (!narExtractionTempDirectory.mkdir()) { + throw new IOException("Cannot create " + narExtractionTempDirectory); + } try { - log.info("Extracting {} to {}", nar, narWorkingDirectory); + log.info("Extracting {} to {}", nar, narExtractionTempDirectory); if (extractCallback != null) { extractCallback.run(); } - unpack(nar, narWorkingDirectory); + unpack(nar, narExtractionTempDirectory); } catch (IOException e) { log.error("There was a problem extracting the nar file. Deleting {} to clean up state.", - narWorkingDirectory, e); - FileUtils.deleteFile(narWorkingDirectory, true); + narExtractionTempDirectory, e); + try { + FileUtils.deleteFile(narExtractionTempDirectory, true); + } catch (IOException e2) { + log.error("Failed to delete temporary directory {}", narExtractionTempDirectory, e2); + } throw e; } + Files.move(narExtractionTempDirectory.toPath(), narWorkingDirectory.toPath(), + StandardCopyOption.ATOMIC_MOVE); } return narWorkingDirectory; } @@ -166,7 +181,7 @@ private static void makeFile(final InputStream inputStream, final File file) thr * @throws IOException * if cannot read file */ - private static byte[] calculateMd5sum(final File file) throws IOException { + protected static byte[] calculateMd5sum(final File file) throws IOException { try (final FileInputStream inputStream = new FileInputStream(file)) { // codeql[java/weak-cryptographic-algorithm] - md5 is sufficient for this use case final MessageDigest md5 = MessageDigest.getInstance("md5"); @@ -184,4 +199,4 @@ private static byte[] calculateMd5sum(final File file) throws IOException { throw new IllegalArgumentException(nsae); } } -} +} \ No newline at end of file diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/nar/NarUnpackerTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/nar/NarUnpackerTest.java index a1f915c8b7828..ad36c2a15edd6 100644 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/nar/NarUnpackerTest.java +++ b/pulsar-common/src/test/java/org/apache/pulsar/common/nar/NarUnpackerTest.java @@ -118,6 +118,30 @@ public static void main(String[] args) { } } + @Test + void shouldReExtractWhenUnpackedDirectoryIsMissing() throws InterruptedException { + CountDownLatch countDownLatch = new CountDownLatch(1); + AtomicInteger exceptionCounter = new AtomicInteger(); + AtomicInteger extractCounter = new AtomicInteger(); + + new Thread(() -> { + try { + File narWorkingDirectory = NarUnpacker.doUnpackNar(sampleZipFile, extractDirectory, extractCounter::incrementAndGet); + FileUtils.deleteFile(narWorkingDirectory, true); + NarUnpacker.doUnpackNar(sampleZipFile, extractDirectory, extractCounter::incrementAndGet); + } catch (Exception e) { + log.error("Unpacking failed", e); + exceptionCounter.incrementAndGet(); + } finally { + countDownLatch.countDown(); + } + }).start(); + + assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); + assertEquals(exceptionCounter.get(), 0); + assertEquals(extractCounter.get(), 2); + } + @Test void shouldExtractFilesOnceInDifferentProcess() throws InterruptedException { int processes = 5; From 8a6bdbf2386cf89dfc5ca9184b53ff417aab2291 Mon Sep 17 00:00:00 2001 From: nikhilerigila09 Date: Thu, 12 Sep 2024 13:53:52 +0530 Subject: [PATCH 2/3] Removed thread --- .../pulsar/common/nar/NarUnpackerTest.java | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/nar/NarUnpackerTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/nar/NarUnpackerTest.java index ad36c2a15edd6..8e678ebafb1a7 100644 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/nar/NarUnpackerTest.java +++ b/pulsar-common/src/test/java/org/apache/pulsar/common/nar/NarUnpackerTest.java @@ -124,18 +124,16 @@ void shouldReExtractWhenUnpackedDirectoryIsMissing() throws InterruptedException AtomicInteger exceptionCounter = new AtomicInteger(); AtomicInteger extractCounter = new AtomicInteger(); - new Thread(() -> { - try { - File narWorkingDirectory = NarUnpacker.doUnpackNar(sampleZipFile, extractDirectory, extractCounter::incrementAndGet); - FileUtils.deleteFile(narWorkingDirectory, true); - NarUnpacker.doUnpackNar(sampleZipFile, extractDirectory, extractCounter::incrementAndGet); - } catch (Exception e) { - log.error("Unpacking failed", e); - exceptionCounter.incrementAndGet(); - } finally { - countDownLatch.countDown(); - } - }).start(); + try { + File narWorkingDirectory = NarUnpacker.doUnpackNar(sampleZipFile, extractDirectory, extractCounter::incrementAndGet); + FileUtils.deleteFile(narWorkingDirectory, true); + NarUnpacker.doUnpackNar(sampleZipFile, extractDirectory, extractCounter::incrementAndGet); + } catch (Exception e) { + log.error("Unpacking failed", e); + exceptionCounter.incrementAndGet(); + } finally { + countDownLatch.countDown(); + } assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); assertEquals(exceptionCounter.get(), 0); From 78687627997c89ca110ea41c7ed72505faddeb61 Mon Sep 17 00:00:00 2001 From: nikhilerigila09 Date: Thu, 12 Sep 2024 17:01:16 +0530 Subject: [PATCH 3/3] Updated test case --- .../pulsar/common/nar/NarUnpackerTest.java | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/nar/NarUnpackerTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/nar/NarUnpackerTest.java index 8e678ebafb1a7..1c3a2c276537b 100644 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/nar/NarUnpackerTest.java +++ b/pulsar-common/src/test/java/org/apache/pulsar/common/nar/NarUnpackerTest.java @@ -119,24 +119,13 @@ public static void main(String[] args) { } @Test - void shouldReExtractWhenUnpackedDirectoryIsMissing() throws InterruptedException { - CountDownLatch countDownLatch = new CountDownLatch(1); - AtomicInteger exceptionCounter = new AtomicInteger(); + void shouldReExtractWhenUnpackedDirectoryIsMissing() throws IOException { AtomicInteger extractCounter = new AtomicInteger(); - try { - File narWorkingDirectory = NarUnpacker.doUnpackNar(sampleZipFile, extractDirectory, extractCounter::incrementAndGet); - FileUtils.deleteFile(narWorkingDirectory, true); - NarUnpacker.doUnpackNar(sampleZipFile, extractDirectory, extractCounter::incrementAndGet); - } catch (Exception e) { - log.error("Unpacking failed", e); - exceptionCounter.incrementAndGet(); - } finally { - countDownLatch.countDown(); - } + File narWorkingDirectory = NarUnpacker.doUnpackNar(sampleZipFile, extractDirectory, extractCounter::incrementAndGet); + FileUtils.deleteFile(narWorkingDirectory, true); + NarUnpacker.doUnpackNar(sampleZipFile, extractDirectory, extractCounter::incrementAndGet); - assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); - assertEquals(exceptionCounter.get(), 0); assertEquals(extractCounter.get(), 2); }