From 58a96f9a00bd65d44e3ed3f3a0c8805372851716 Mon Sep 17 00:00:00 2001 From: wantaek Date: Fri, 24 Jul 2026 16:13:36 +0900 Subject: [PATCH 1/2] Delete temporary file when ExportedImageTar construction fails ExportedImageTar creates a temporary file in its constructor and then copies the exported image into it and builds the layer factory. If either step throws, the constructor fails before the instance is assigned, so the try-with-resources statement that uses it never calls close() and the docker-layers- temporary file is left behind. Delete the temporary file when the constructor fails so its cleanup matches close(). See gh-51117 Signed-off-by: wantaek --- .../platform/docker/ExportedImageTar.java | 15 +++++- .../docker/ExportedImageTarTests.java | 47 +++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/buildpack/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTar.java b/buildpack/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTar.java index 776593c22283..05ecd11289cd 100644 --- a/buildpack/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTar.java +++ b/buildpack/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTar.java @@ -64,8 +64,19 @@ class ExportedImageTar implements Closeable { ExportedImageTar(ImageReference reference, InputStream inputStream) throws IOException { this.tarFile = Files.createTempFile("docker-layers-", null); - Files.copy(inputStream, this.tarFile, StandardCopyOption.REPLACE_EXISTING); - this.layerArchiveFactory = LayerArchiveFactory.create(reference, this.tarFile); + try { + Files.copy(inputStream, this.tarFile, StandardCopyOption.REPLACE_EXISTING); + this.layerArchiveFactory = LayerArchiveFactory.create(reference, this.tarFile); + } + catch (IOException | RuntimeException ex) { + try { + Files.deleteIfExists(this.tarFile); + } + catch (IOException suppressed) { + ex.addSuppressed(suppressed); + } + throw ex; + } } void exportLayers(IOBiConsumer exports) throws IOException { diff --git a/buildpack/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTarTests.java b/buildpack/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTarTests.java index 5f306b2d4322..65386b4d7d70 100644 --- a/buildpack/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTarTests.java +++ b/buildpack/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTarTests.java @@ -16,9 +16,19 @@ package org.springframework.boot.buildpack.platform.docker; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; +import org.apache.commons.compress.archivers.tar.TarArchiveEntry; +import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -26,6 +36,7 @@ import org.springframework.boot.buildpack.platform.io.TarArchive.Compression; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; /** * Tests for {@link ExportedImageTar}. @@ -56,4 +67,40 @@ void test(String tarFile) throws Exception { } } + @Test + void constructorWhenTarHasNoIndexOrManifestDeletesTempFile() throws Exception { + File tempDir = new File(System.getProperty("java.io.tmpdir")); + Set tempsBefore = listTempFileNames(tempDir); + ImageReference reference = ImageReference.of("test:latest"); + assertThatIllegalStateException().isThrownBy(() -> new ExportedImageTar(reference, tarWithoutIndexOrManifest())) + .withMessageContaining("does not contain 'index.json' or 'manifest.json'"); + Set leaked = new HashSet<>(listTempFileNames(tempDir)); + leaked.removeAll(tempsBefore); + assertThat(leaked).as("temp file must be deleted when the constructor fails").isEmpty(); + } + + private InputStream tarWithoutIndexOrManifest() throws Exception { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try (TarArchiveOutputStream tar = new TarArchiveOutputStream(out)) { + byte[] data = "test".getBytes(StandardCharsets.UTF_8); + TarArchiveEntry entry = new TarArchiveEntry("some-file.txt"); + entry.setSize(data.length); + tar.putArchiveEntry(entry); + tar.write(data); + tar.closeArchiveEntry(); + } + return new ByteArrayInputStream(out.toByteArray()); + } + + private Set listTempFileNames(File tempDir) { + File[] files = tempDir.listFiles((dir, name) -> name.startsWith("docker-layers-")); + Set names = new HashSet<>(); + if (files != null) { + for (File file : files) { + names.add(file.getName()); + } + } + return names; + } + } From e5106ef9e9658dc631e1866a351a781b74f32210 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Mon, 27 Jul 2026 09:40:57 +0200 Subject: [PATCH 2/2] Polish "Delete temporary file when ExportedImageTar construction fails" See gh-51117 --- .../platform/docker/ExportedImageTar.java | 4 +- .../docker/ExportedImageTarTests.java | 68 +++++++++---------- 2 files changed, 34 insertions(+), 38 deletions(-) diff --git a/buildpack/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTar.java b/buildpack/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTar.java index 05ecd11289cd..607f11632041 100644 --- a/buildpack/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTar.java +++ b/buildpack/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTar.java @@ -63,8 +63,8 @@ class ExportedImageTar implements Closeable { private final LayerArchiveFactory layerArchiveFactory; ExportedImageTar(ImageReference reference, InputStream inputStream) throws IOException { - this.tarFile = Files.createTempFile("docker-layers-", null); - try { + this.tarFile = Files.createTempFile("docker-layers-", ".tar"); + try (inputStream) { Files.copy(inputStream, this.tarFile, StandardCopyOption.REPLACE_EXISTING); this.layerArchiveFactory = LayerArchiveFactory.create(reference, this.tarFile); } diff --git a/buildpack/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTarTests.java b/buildpack/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTarTests.java index 65386b4d7d70..daf3f6422ff7 100644 --- a/buildpack/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTarTests.java +++ b/buildpack/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/ExportedImageTarTests.java @@ -17,17 +17,17 @@ package org.springframework.boot.buildpack.platform.docker; import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.File; +import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; -import java.util.HashSet; import java.util.List; -import java.util.Set; +import java.util.Objects; +import java.util.stream.Collectors; +import java.util.stream.Stream; -import org.apache.commons.compress.archivers.tar.TarArchiveEntry; -import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -37,6 +37,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.mockito.BDDMockito.then; +import static org.mockito.Mockito.spy; /** * Tests for {@link ExportedImageTar}. @@ -51,9 +53,15 @@ class ExportedImageTarTests { "export-docker-desktop-containerd-manifest-list.tar", "export-docker-engine.tar", "export-podman.tar", "export-docker-desktop-nested-index.tar", "export-docker-desktop-containerd-alt-mediatype.tar" }) void test(String tarFile) throws Exception { + List temporaryDockerLayersTarFilesBefore = scanTemporaryDockerLayersTarFiles(); ImageReference reference = ImageReference.of("test:latest"); - try (ExportedImageTar exportedImageTar = new ExportedImageTar(reference, - getClass().getResourceAsStream(tarFile))) { + Path temporaryDockerFile; + InputStream inputStream = spy(Objects.requireNonNull(getClass().getResourceAsStream(tarFile))); + try (ExportedImageTar exportedImageTar = new ExportedImageTar(reference, inputStream)) { + List temporaryDockerLayersTarFilesCurrent = scanTemporaryDockerLayersTarFiles(); + temporaryDockerLayersTarFilesCurrent.removeAll(temporaryDockerLayersTarFilesBefore); + assertThat(temporaryDockerLayersTarFilesCurrent).hasSize(1); + temporaryDockerFile = temporaryDockerLayersTarFilesCurrent.get(0); Compression expectedCompression = (!tarFile.contains("containerd")) ? Compression.NONE : Compression.GZIP; String expectedName = (expectedCompression != Compression.GZIP) ? "5caae51697b248b905dca1a4160864b0e1a15c300981736555cdce6567e8d477" @@ -65,42 +73,30 @@ void test(String tarFile) throws Exception { }); assertThat(names).filteredOn((name) -> name.contains(expectedName)).isNotEmpty(); } + then(inputStream).should().close(); + assertThat(temporaryDockerFile).doesNotExist(); } @Test - void constructorWhenTarHasNoIndexOrManifestDeletesTempFile() throws Exception { - File tempDir = new File(System.getProperty("java.io.tmpdir")); - Set tempsBefore = listTempFileNames(tempDir); + void constructorWhenTarHasNoIndexOrManifestDeletesTempFile() throws IOException { + List temporaryDockerLayersTarFilesBefore = scanTemporaryDockerLayersTarFiles(); ImageReference reference = ImageReference.of("test:latest"); - assertThatIllegalStateException().isThrownBy(() -> new ExportedImageTar(reference, tarWithoutIndexOrManifest())) + InputStream notATarFile = spy(new ByteArrayInputStream("not-a-tar-file".getBytes(StandardCharsets.UTF_8))); + assertThatIllegalStateException().isThrownBy(() -> new ExportedImageTar(reference, notATarFile)) .withMessageContaining("does not contain 'index.json' or 'manifest.json'"); - Set leaked = new HashSet<>(listTempFileNames(tempDir)); - leaked.removeAll(tempsBefore); - assertThat(leaked).as("temp file must be deleted when the constructor fails").isEmpty(); + then(notATarFile).should().close(); + assertThat(scanTemporaryDockerLayersTarFiles()).containsOnlyOnceElementsOf(temporaryDockerLayersTarFilesBefore) + .hasSize(temporaryDockerLayersTarFilesBefore.size()); } - private InputStream tarWithoutIndexOrManifest() throws Exception { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - try (TarArchiveOutputStream tar = new TarArchiveOutputStream(out)) { - byte[] data = "test".getBytes(StandardCharsets.UTF_8); - TarArchiveEntry entry = new TarArchiveEntry("some-file.txt"); - entry.setSize(data.length); - tar.putArchiveEntry(entry); - tar.write(data); - tar.closeArchiveEntry(); + private static List scanTemporaryDockerLayersTarFiles() throws IOException { + Path tempDir = Path.of(System.getProperty("java.io.tmpdir")); + try (Stream stream = Files.list(tempDir)) { + return stream.filter(Files::isRegularFile).filter((candidate) -> { + String filename = candidate.getFileName().toString(); + return filename.startsWith("docker-layers") && filename.endsWith(".tar"); + }).collect(Collectors.toCollection(ArrayList::new)); } - return new ByteArrayInputStream(out.toByteArray()); - } - - private Set listTempFileNames(File tempDir) { - File[] files = tempDir.listFiles((dir, name) -> name.startsWith("docker-layers-")); - Set names = new HashSet<>(); - if (files != null) { - for (File file : files) { - names.add(file.getName()); - } - } - return names; } }