diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java index b3fe49db89..cb3f8aff3d 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java @@ -30,7 +30,8 @@ import java.nio.channels.FileChannel; import java.nio.charset.Charset; import java.nio.file.Files; -import static io.netty.buffer.Unpooled.*; +import static io.netty.buffer.Unpooled.EMPTY_BUFFER; +import static io.netty.buffer.Unpooled.wrappedBuffer; import static java.util.Objects.requireNonNull; /** @@ -94,7 +95,8 @@ public abstract class AbstractDiskHttpData extends AbstractHttpData { getBaseDirectory())); } if (deleteOnExit()) { - tmpFile.deleteOnExit(); + // See https://github.com/netty/netty/issues/10351 + DeleteFileOnExitHook.add(tmpFile.getPath()); } return tmpFile; } @@ -265,12 +267,21 @@ public abstract class AbstractDiskHttpData extends AbstractHttpData { } fileChannel = null; } - if (! isRenamed) { + if (!isRenamed) { + String filePath = null; + if (file != null && file.exists()) { + filePath = file.getPath(); if (!file.delete()) { + filePath = null; logger.warn("Failed to delete: {}", file); } } + + // If you turn on deleteOnExit make sure it is executed. + if (deleteOnExit() && filePath != null) { + DeleteFileOnExitHook.remove(filePath); + } file = null; } } @@ -373,7 +384,7 @@ public abstract class AbstractDiskHttpData extends AbstractHttpData { if (chunkSize < size - position) { chunkSize = size - position; } - position += in.transferTo(position, chunkSize , out); + position += in.transferTo(position, chunkSize, out); } } catch (IOException e) { exception = e; @@ -425,6 +436,7 @@ public abstract class AbstractDiskHttpData extends AbstractHttpData { /** * Utility function + * * @return the array of bytes */ private static byte[] readFrom(File src) throws IOException { diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/DeleteFileOnExitHook.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/DeleteFileOnExitHook.java new file mode 100644 index 0000000000..0253e6578e --- /dev/null +++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/DeleteFileOnExitHook.java @@ -0,0 +1,82 @@ +/* + * Copyright 2020 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.handler.codec.http.multipart; + +import java.io.File; +import java.util.Collections; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +/** + * DeleteFileOnExitHook. + */ +final class DeleteFileOnExitHook { + private static final Set FILES = Collections.newSetFromMap(new ConcurrentHashMap()); + + private DeleteFileOnExitHook() { + } + + static { + // DeleteOnExitHook must be the last shutdown hook to be invoked. + // Application shutdown hooks may add the first file to the + // delete on exit list and cause the DeleteOnExitHook to be + // registered during shutdown in progress. + Runtime.getRuntime().addShutdownHook(new Thread() { + + @Override + public void run() { + runHook(); + } + }); + } + + /** + * Remove from the pool to reduce space footprint. + * + * @param file tmp file path + */ + public static void remove(String file) { + FILES.remove(file); + } + + /** + * Add to the hook and clean up when the program exits. + * + * @param file tmp file path + */ + public static void add(String file) { + FILES.add(file); + } + + /** + * Check in the hook files. + * + * @param file target file + * @return true or false + */ + public static boolean checkFileExist(String file) { + return FILES.contains(file); + } + + /** + * Clean up all the files. + */ + static void runHook() { + for (String filename : FILES) { + new File(filename).delete(); + } + } +} diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DeleteFileOnExitHookTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DeleteFileOnExitHookTest.java new file mode 100644 index 0000000000..2eabf4ba8a --- /dev/null +++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DeleteFileOnExitHookTest.java @@ -0,0 +1,81 @@ +/* + * Copyright 2020 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.handler.codec.http.multipart; + +import io.netty.buffer.Unpooled; +import io.netty.handler.codec.http.DefaultHttpRequest; +import io.netty.handler.codec.http.HttpRequest; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.io.FilenameFilter; +import java.io.IOException; + +import static io.netty.handler.codec.http.HttpMethod.POST; +import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1; +import static org.junit.Assert.*; + +/** + * Test DeleteFileOnExitHook + */ +public class DeleteFileOnExitHookTest { + private static final HttpRequest REQUEST = new DefaultHttpRequest(HTTP_1_1, POST, "/form"); + private static final String HOOK_TEST_TMP = "target/DeleteFileOnExitHookTest/tmp"; + private FileUpload fu; + + @Before + public void setUp() throws IOException { + DefaultHttpDataFactory defaultHttpDataFactory = new DefaultHttpDataFactory(true); + defaultHttpDataFactory.setBaseDir(HOOK_TEST_TMP); + defaultHttpDataFactory.setDeleteOnExit(true); + + File baseDir = new File(HOOK_TEST_TMP); + baseDir.mkdirs(); // we don't need to clean it since it is in volatile files anyway + + fu = defaultHttpDataFactory.createFileUpload( + REQUEST, "attribute1", "tmp_f.txt", "text/plain", null, null, 0); + fu.setContent(Unpooled.wrappedBuffer(new byte[]{1, 2, 3, 4})); + + assertTrue(fu.getFile().exists()); + } + + @Test + public void testSimulateTriggerDeleteFileOnExitHook() { + + // simulate app exit + DeleteFileOnExitHook.runHook(); + + File[] files = new File(HOOK_TEST_TMP).listFiles(new FilenameFilter() { + @Override + public boolean accept(File dir, String name) { + return name.startsWith(DiskFileUpload.prefix); + } + }); + + assertEquals(0, files.length); + } + + @Test + public void testAfterHttpDataReleaseCheckFileExist() throws IOException { + + String filePath = fu.getFile().getPath(); + assertTrue(DeleteFileOnExitHook.checkFileExist(filePath)); + + fu.release(); + assertFalse(DeleteFileOnExitHook.checkFileExist(filePath)); + } +}