Delete file already wrote partially into attachment folder when content is terminated with an error (#4286)

#### What type of PR is this?

/kind bug
/area core
/milestone 2.8.x

#### What this PR does / why we need it:

If content is terminated with an error, the file already wrote partially into attachment folder won't be cleaned.

Imagine a scenario where we check that the content size is not larger than 2MB when we write content to the attachments folder. Once the limit is reached, files that have been partially written should be cleaned instead of being kept.

#### Does this PR introduce a user-facing change?

```release-note
None
```
pull/4290/head
John Niang 2023-07-24 17:22:15 +08:00 committed by GitHub
parent 15dd7826dc
commit 0d19ccdb8a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 46 additions and 15 deletions

View File

@ -3,6 +3,7 @@ package run.halo.app.core.extension.attachment.endpoint;
import static java.nio.file.StandardOpenOption.CREATE_NEW;
import static run.halo.app.infra.utils.FileNameUtils.randomFileName;
import static run.halo.app.infra.utils.FileUtils.checkDirectoryTraversal;
import static run.halo.app.infra.utils.FileUtils.deleteFileSilently;
import java.io.IOException;
import java.net.URI;
@ -236,6 +237,9 @@ class LocalAttachmentUploadHandler implements AttachmentHandler {
var fileName = randomFileName(oldPath.toString(), 4);
pathRef.set(oldPath.resolveSibling(fileName));
}))
// Delete file already wrote partially into attachment folder
// in case of content is terminated with an error
.onErrorResume(t -> deleteFileSilently(pathRef.get()).then(Mono.error(t)))
.then(Mono.fromSupplier(pathRef::get));
});
}

View File

@ -26,6 +26,8 @@ import lombok.extern.slf4j.Slf4j;
import org.springframework.lang.NonNull;
import org.springframework.util.AntPathMatcher;
import org.springframework.util.Assert;
import reactor.core.publisher.Mono;
import reactor.core.scheduler.Schedulers;
import run.halo.app.infra.exception.AccessDeniedException;
/**
@ -243,6 +245,21 @@ public abstract class FileUtils {
}
}
public static Mono<Boolean> deleteFileSilently(Path file) {
return Mono.fromSupplier(
() -> {
if (file == null || !Files.isRegularFile(file)) {
return false;
}
try {
return Files.deleteIfExists(file);
} catch (IOException ignored) {
return false;
}
})
.subscribeOn(Schedulers.boundedElastic());
}
public static void copy(Path source, Path dest, CopyOption... options) {
try {
Files.copy(source, dest, options);
@ -277,4 +294,5 @@ public abstract class FileUtils {
}
});
}
}

View File

@ -4,7 +4,7 @@ import static java.util.Objects.requireNonNull;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static run.halo.app.infra.utils.FileUtils.checkDirectoryTraversal;
import static run.halo.app.infra.utils.FileUtils.deleteRecursivelyAndSilently;
import static run.halo.app.infra.utils.FileUtils.deleteFileSilently;
import static run.halo.app.infra.utils.FileUtils.jar;
import static run.halo.app.infra.utils.FileUtils.unzip;
import static run.halo.app.infra.utils.FileUtils.zip;
@ -16,14 +16,17 @@ import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.zip.ZipInputStream;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import reactor.test.StepVerifier;
import run.halo.app.infra.exception.AccessDeniedException;
class FileUtilsTest {
@TempDir
Path tempDirectory;
@Nested
class DirectoryTraversalTest {
@ -49,18 +52,6 @@ class FileUtilsTest {
@Nested
class ZipTest {
Path tempDirectory;
@BeforeEach
void setUp() throws IOException {
tempDirectory = Files.createTempDirectory("halo-test-fileutils-zip-");
}
@AfterEach
void cleanUp() {
deleteRecursivelyAndSilently(tempDirectory);
}
@Test
void zipFolderAndUnzip() throws IOException, URISyntaxException {
var uri = requireNonNull(getClass().getClassLoader().getResource("folder-to-zip"))
@ -104,5 +95,23 @@ class FileUtilsTest {
assertThrows(NoSuchFileException.class, () ->
jar(Paths.get("no-such-folder"), tempDirectory.resolve("example.zip")));
}
}
@Test
void deleteFileSilentlyTest() throws IOException {
StepVerifier.create(deleteFileSilently(null))
.expectNext(false)
.verifyComplete();
StepVerifier.create(deleteFileSilently(tempDirectory))
.expectNext(false)
.verifyComplete();
StepVerifier.create(
deleteFileSilently(Files.createFile(tempDirectory.resolve("for-deleting"))))
.expectNext(true)
.verifyComplete();
}
}