Ensure DefaultPartHttpMessageReader temp directories do not collide

poutsma · lxbzmy · commit 332257d9cf20 · 2022-03-26T22:50:33.000+08:00
This commit makes sure that the DefaultPartHttpMessageReader uses a random temporary directory to store uploaded files, so that two instances do not collide. See spring-projectsgh-26931
diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReader.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/DefaultPartHttpMessageReader.java
@@ -19,9 +19,7 @@
 import java.io.IOException;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -63,8 +61,6 @@
  */
 public class DefaultPartHttpMessageReader extends LoggingCodecSupport implements HttpMessageReader<Part> {
 
-	private static final String IDENTIFIER = "spring-multipart";
-
 	private int maxInMemorySize = 256 * 1024;
 
 	private int maxHeadersSize = 8 * 1024;
@@ -77,7 +73,7 @@ public class DefaultPartHttpMessageReader extends LoggingCodecSupport implements
 
 	private Scheduler blockingOperationScheduler = Schedulers.boundedElastic();
 
-	private Mono<Path> fileStorageDirectory = Mono.defer(this::defaultFileStorageDirectory).cache();
+	private FileStorage fileStorage = FileStorage.tempDirectory(this::getBlockingOperationScheduler);
 
 	private Charset headersCharset = StandardCharsets.UTF_8;
 
@@ -147,10 +143,7 @@ public void setMaxParts(int maxParts) {
 	 */
 	public void setFileStorageDirectory(Path fileStorageDirectory) throws IOException {
 		Assert.notNull(fileStorageDirectory, "FileStorageDirectory must not be null");
-		if (!Files.exists(fileStorageDirectory)) {
-			Files.createDirectory(fileStorageDirectory);
-		}
-		this.fileStorageDirectory = Mono.just(fileStorageDirectory);
+		this.fileStorage = FileStorage.fromPath(fileStorageDirectory);
 	}
 
 	/**
@@ -168,6 +161,10 @@ public void setBlockingOperationScheduler(Scheduler blockingOperationScheduler)
 		this.blockingOperationScheduler = blockingOperationScheduler;
 	}
 
+	private Scheduler getBlockingOperationScheduler() {
+		return this.blockingOperationScheduler;
+	}
+
 	/**
 	 * When set to {@code true}, the {@linkplain Part#content() part content}
 	 * is streamed directly from the parsed input buffer stream, and not stored
@@ -230,7 +227,7 @@ public Flux<Part> read(ResolvableType elementType, ReactiveHttpInputMessage mess
 					this.maxHeadersSize, this.headersCharset);
 
 			return PartGenerator.createParts(tokens, this.maxParts, this.maxInMemorySize, this.maxDiskUsagePerPart,
-					this.streaming, this.fileStorageDirectory, this.blockingOperationScheduler);
+					this.streaming, this.fileStorage.directory(), this.blockingOperationScheduler);
 		});
 	}
 
@@ -250,16 +247,4 @@ private byte[] boundary(HttpMessage message) {
 		return null;
 	}
 
-	@SuppressWarnings("BlockingMethodInNonBlockingContext")
-	private Mono<Path> defaultFileStorageDirectory() {
-		return Mono.fromCallable(() -> {
-			Path tempDirectory = Paths.get(System.getProperty("java.io.tmpdir"), IDENTIFIER);
-			if (!Files.exists(tempDirectory)) {
-				Files.createDirectory(tempDirectory);
-			}
-			return tempDirectory;
-		}).subscribeOn(this.blockingOperationScheduler);
-
-	}
-
 }
diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/FileStorage.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/FileStorage.java
@@ -0,0 +1,128 @@
+/*
+ * Copyright 2002-2021 the original author or authors.
+ *
+ * Licensed 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
+ *
+ *      https://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 org.springframework.http.codec.multipart;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.function.Supplier;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import reactor.core.publisher.Mono;
+import reactor.core.scheduler.Scheduler;
+
+/**
+ * Represents a directory used to store parts larger than
+ * {@link DefaultPartHttpMessageReader#setMaxInMemorySize(int)}.
+ *
+ * @author Arjen Poutsma
+ * @since 5.3.7
+ */
+abstract class FileStorage {
+
+	private static final Log logger = LogFactory.getLog(FileStorage.class);
+
+
+	protected FileStorage() {
+	}
+
+	/**
+	 * Get the mono of the directory to store files in.
+	 */
+	public abstract Mono<Path> directory();
+
+
+	/**
+	 * Create a new {@code FileStorage} from a user-specified path. Creates the
+	 * path if it does not exist.
+	 */
+	public static FileStorage fromPath(Path path) throws IOException {
+		if (!Files.exists(path)) {
+			Files.createDirectory(path);
+		}
+		return new PathFileStorage(path);
+	}
+
+	/**
+	 * Create a new {@code FileStorage} based a on a temporary directory.
+	 * @param scheduler scheduler to use for blocking operations
+	 */
+	public static FileStorage tempDirectory(Supplier<Scheduler> scheduler) {
+		return new TempFileStorage(scheduler);
+	}
+
+
+	private static final class PathFileStorage extends FileStorage {
+
+		private final Mono<Path> directory;
+
+		public PathFileStorage(Path directory) {
+			this.directory = Mono.just(directory);
+		}
+
+		@Override
+		public Mono<Path> directory() {
+			return this.directory;
+		}
+	}
+
+
+	private static final class TempFileStorage extends FileStorage {
+
+		private static final String IDENTIFIER = "spring-multipart-";
+
+		private final Supplier<Scheduler> scheduler;
+
+		private volatile Mono<Path> directory = tempDirectory();
+
+
+		public TempFileStorage(Supplier<Scheduler> scheduler) {
+			this.scheduler = scheduler;
+		}
+
+		@Override
+		public Mono<Path> directory() {
+			return this.directory
+					.flatMap(this::createNewDirectoryIfDeleted)
+					.subscribeOn(this.scheduler.get());
+		}
+
+		private Mono<Path> createNewDirectoryIfDeleted(Path directory) {
+			if (!Files.exists(directory)) {
+				// Some daemons remove temp directories. Let's create a new one.
+				Mono<Path> newDirectory = tempDirectory();
+				this.directory = newDirectory;
+				return newDirectory;
+			}
+			else {
+				return Mono.just(directory);
+			}
+		}
+
+		private static Mono<Path> tempDirectory() {
+			return Mono.fromCallable(() -> {
+				Path directory = Files.createTempDirectory(IDENTIFIER);
+				if (logger.isDebugEnabled()) {
+					logger.debug("Created temporary storage directory: " + directory);
+				}
+				return directory;
+			}).cache();
+		}
+	}
+
+}
diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/PartGenerator.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/PartGenerator.java
@@ -578,9 +578,6 @@ public void createFile() {
 
 		private WritingFileState createFileState(Path directory) {
 			try {
-				if (!Files.exists(directory)) {
-					Files.createDirectory(directory);
-				}
 				Path tempFile = Files.createTempFile(directory, null, ".multipart");
 				if (logger.isTraceEnabled()) {
 					logger.trace("Storing multipart data in file " + tempFile);
diff --git a/spring-web/src/test/java/org/springframework/http/codec/multipart/FileStorageTests.java b/spring-web/src/test/java/org/springframework/http/codec/multipart/FileStorageTests.java
@@ -0,0 +1,82 @@
+/*
+ * Copyright 2002-2021 the original author or authors.
+ *
+ * Licensed 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
+ *
+ *      https://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 org.springframework.http.codec.multipart;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import org.junit.jupiter.api.Test;
+import reactor.core.publisher.Mono;
+import reactor.core.scheduler.Schedulers;
+import reactor.test.StepVerifier;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * @author Arjen Poutsma
+ */
+class FileStorageTests {
+
+	@Test
+	void fromPath() throws IOException {
+		Path path = Files.createTempFile("spring", "test");
+		FileStorage storage = FileStorage.fromPath(path);
+
+		Mono<Path> directory = storage.directory();
+		StepVerifier.create(directory)
+				.expectNext(path)
+				.verifyComplete();
+	}
+
+	@Test
+	void tempDirectory() {
+		FileStorage storage = FileStorage.tempDirectory(Schedulers::boundedElastic);
+
+		Mono<Path> directory = storage.directory();
+		StepVerifier.create(directory)
+				.consumeNextWith(path -> {
+					assertThat(path).exists();
+					StepVerifier.create(directory)
+							.expectNext(path)
+							.verifyComplete();
+				})
+				.verifyComplete();
+	}
+
+	@Test
+	void tempDirectoryDeleted() {
+		FileStorage storage = FileStorage.tempDirectory(Schedulers::boundedElastic);
+
+		Mono<Path> directory = storage.directory();
+		StepVerifier.create(directory)
+				.consumeNextWith(path1 -> {
+					try {
+						Files.delete(path1);
+						StepVerifier.create(directory)
+								.consumeNextWith(path2 -> assertThat(path2).isNotEqualTo(path1))
+								.verifyComplete();
+					}
+					catch (IOException ex) {
+						throw new UncheckedIOException(ex);
+					}
+				})
+				.verifyComplete();
+	}
+
+}
