Close streams and delete intermediate temps in ImageBuildpack

ExportedLayers left the intermediate create-builder-scratch-source
temp file behind after rebased layer files were written, and opened
layer InputStreams without closing them when StreamUtils.copy does
not close either stream. Delete the source temp in a finally block
and use try-with-resources for the apply path so layer files can be
deleted reliably.

See gh-50919

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
This commit is contained in:
Sebastien Tardif
2026-07-14 08:33:33 +02:00
committed by Stéphane Nicoll
parent 401a719654
commit 7bdb1320bf
2 changed files with 67 additions and 22 deletions
@@ -132,35 +132,41 @@ final class ImageBuildpack implements Buildpack {
private Path createLayerFile(TarArchive tarArchive) throws IOException {
Path sourceTarFile = Files.createTempFile("create-builder-scratch-source-", null);
try (OutputStream out = Files.newOutputStream(sourceTarFile)) {
tarArchive.writeTo(out);
}
Path layerFile = Files.createTempFile("create-builder-scratch-", null);
try (TarArchiveOutputStream out = new TarArchiveOutputStream(Files.newOutputStream(layerFile))) {
try (TarArchiveInputStream in = new TarArchiveInputStream(Files.newInputStream(sourceTarFile))) {
out.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX);
TarArchiveEntry entry = in.getNextEntry();
while (entry != null) {
String entryName = entry.getName();
Path entryPath = Path.of(entryName);
Assert.state(entryPath.toAbsolutePath().equals(entryPath.toAbsolutePath().normalize()),
() -> "Malformed zip entry name '%s'".formatted(entryName));
out.putArchiveEntry(entry);
StreamUtils.copy(in, out);
out.closeArchiveEntry();
entry = in.getNextEntry();
}
out.finish();
try {
try (OutputStream out = Files.newOutputStream(sourceTarFile)) {
tarArchive.writeTo(out);
}
Path layerFile = Files.createTempFile("create-builder-scratch-", null);
try (TarArchiveOutputStream out = new TarArchiveOutputStream(Files.newOutputStream(layerFile))) {
try (TarArchiveInputStream in = new TarArchiveInputStream(Files.newInputStream(sourceTarFile))) {
out.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX);
TarArchiveEntry entry = in.getNextEntry();
while (entry != null) {
String entryName = entry.getName();
Path entryPath = Path.of(entryName);
Assert.state(entryPath.toAbsolutePath().equals(entryPath.toAbsolutePath().normalize()),
() -> "Malformed zip entry name '%s'".formatted(entryName));
out.putArchiveEntry(entry);
StreamUtils.copy(in, out);
out.closeArchiveEntry();
entry = in.getNextEntry();
}
out.finish();
}
}
return layerFile;
}
finally {
Files.deleteIfExists(sourceTarFile);
}
return layerFile;
}
void apply(IOConsumer<Layer> layers) throws IOException {
for (Path path : this.layerFiles) {
layers.accept(Layer.fromTarArchive((out) -> {
InputStream in = Files.newInputStream(path);
StreamUtils.copy(in, out);
try (InputStream in = Files.newInputStream(path)) {
StreamUtils.copy(in, out);
}
}));
Files.delete(path);
}
@@ -24,8 +24,11 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Random;
import java.util.Set;
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
@@ -197,6 +200,42 @@ class ImageBuildpackTests extends AbstractJsonTests {
.withMessage("Malformed zip entry name '../cnb/'");
}
@Test
void resolveDeletesIntermediateSourceTarTempFiles() throws Exception {
File tempDir = new File(System.getProperty("java.io.tmpdir"));
Set<String> tempsBefore = listTempFileNames(tempDir, "create-builder-scratch-");
Image image = Image.of(getContent("buildpack-image.json"));
ImageReference imageReference = ImageReference.of("example/buildpack1:1.0.0");
BuildpackResolverContext resolverContext = mock(BuildpackResolverContext.class);
given(resolverContext.getBuildpackLayersMetadata()).willReturn(BuildpackLayersMetadata.fromJson("{}"));
given(resolverContext.fetchImage(eq(imageReference), eq(ImageType.BUILDPACK))).willReturn(image);
willAnswer(this::withMockLayers).given(resolverContext).exportImageLayers(eq(imageReference), any());
BuildpackReference reference = BuildpackReference.of("docker://example/buildpack1:1.0.0");
Buildpack buildpack = ImageBuildpack.resolve(resolverContext, reference);
assertThat(buildpack).isNotNull();
Set<String> createdOnResolve = new HashSet<>(listTempFileNames(tempDir, "create-builder-scratch-"));
createdOnResolve.removeAll(tempsBefore);
assertThat(createdOnResolve).as("intermediate source tar temp files must be deleted after createLayerFile")
.noneMatch((name) -> name.startsWith("create-builder-scratch-source-"));
assertThat(createdOnResolve).isNotEmpty();
assertAppliesExpectedLayers(buildpack);
Set<String> remainingCreated = new HashSet<>(listTempFileNames(tempDir, "create-builder-scratch-"));
remainingCreated.retainAll(createdOnResolve);
assertThat(remainingCreated).as("layer temp files created on resolve must be deleted after apply").isEmpty();
}
private Set<String> listTempFileNames(File tempDir, String prefix) {
File[] files = tempDir.listFiles((dir, name) -> name.startsWith(prefix));
if (files == null) {
return Collections.emptySet();
}
Set<String> names = new HashSet<>();
for (File file : files) {
names.add(file.getName());
}
return names;
}
private @Nullable Object withMockLayers(InvocationOnMock invocation) {
return withMockLayers(invocation, "");
}