From 515c52b0fc208cbacc3070d5f15951247f9a54bc Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sat, 22 Aug 2026 22:45:33 +0100 Subject: [PATCH 1/4] serve jar resources without reopening the jar per request Motivation: `ResourceFile` opened a `java.util.zip.ZipFile` for every request to a resource that lives in a jar, only to read the entry's size and time. That parses the whole central directory of the jar again per request, and `getFromResource`/`getFromResourceDirectory` served from a jar is the usual production layout for static resources. The result of `getEntry` was also dereferenced without a null check. Modification: Read the metadata from the `JarURLConnection` instead and leave its cache enabled, so the JDK reuses the same open jar file that the class loader already holds. Guard against a null entry, and share the plain `URLConnection` handling with the fallback branch. Result: No jar is opened or parsed per request for resources served from a jar, and a missing entry rejects the request instead of throwing. Tests: - sbt "http-tests/testOnly org.apache.pekko.http.scaladsl.server.directives.FileAndResourceDirectivesSpec" - pass, 1 new test asserting the entry metadata matches the bytes served - sbt http-tests/test - pass - sbt +http/compile - pass - sbt http/mimaReportBinaryIssues - pass - sbt http/scalafmt http-tests/Test/scalafmt - clean References: None - avoids reopening jars for every resource request --- .../FileAndResourceDirectivesSpec.scala | 13 ++++++ .../FileAndResourceDirectives.scala | 44 +++++++++---------- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectivesSpec.scala b/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectivesSpec.scala index 77a262b03..73bcba045 100644 --- a/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectivesSpec.scala +++ b/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectivesSpec.scala @@ -267,6 +267,19 @@ class FileAndResourceDirectivesSpec extends RoutingSpec with Inspectors with Ins 1.second.dilated).data.asByteBuffer.getInt shouldEqual 0xCAFEBABE } } + "return the resource content from an archive with metadata taken from the archive entry" in { + val route = getFromResource("com/typesafe/config/Config.class") + + def runCheck() = + Get() ~> route ~> check { + val entity = responseEntity.toStrict(1.second.dilated).awaitResult(1.second.dilated) + entity.contentLength shouldEqual entity.data.length + header[`Last-Modified`] shouldBe defined + } + + runCheck() + runCheck() // the archive is shared between requests, so make sure it is still usable afterwards + } "return the file content with MediaType 'application/octet-stream' on unknown file extensions" in { Get() ~> getFromResource("sample.xyz") ~> check { mediaType shouldEqual `application/octet-stream` diff --git a/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala b/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala index 19a6c9955..cfe6198e9 100644 --- a/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala +++ b/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala @@ -15,7 +15,7 @@ package org.apache.pekko.http.scaladsl.server package directives import java.io.File -import java.net.{ URI, URL } +import java.net.{ JarURLConnection, URL, URLConnection } import scala.annotation.tailrec import scala.jdk.CollectionConverters._ @@ -284,28 +284,28 @@ object FileAndResourceDirectives extends FileAndResourceDirectives { if (file.isDirectory) None else Some(ResourceFile(url, file.length(), file.lastModified())) case "jar" => - val path = new URI(url.getPath).getPath // remove "file:" prefix and normalize whitespace - val bangIndex = path.indexOf('!') - val filePath = path.substring(0, bangIndex) - val resourcePath = path.substring(bangIndex + 2) - val jar = new java.util.zip.ZipFile(filePath) - try { - val entry = jar.getEntry(resourcePath) - if (entry.isDirectory) None - else Option(jar.getInputStream(entry)).map { is => - is.close() - ResourceFile(url, entry.getSize, entry.getTime) - } - } finally jar.close() - case _ => - val conn = url.openConnection() - try { - conn.setUseCaches(false) // otherwise the JDK will keep the connection open when we close! - val len = conn.getContentLength - val lm = conn.getLastModified - Some(ResourceFile(url, len, lm)) - } finally conn.getInputStream.close() + url.openConnection() match { + case jarConnection: JarURLConnection => + // Ask the connection for the entry instead of opening the jar file here: opening it means reading and + // parsing the whole central directory again for every single request. With caching left enabled the JDK + // reuses the same open jar file as the class loader does, so nothing is opened here at all in the common + // case (and nothing must be closed either, the cached jar file is shared). + jarConnection.setUseCaches(true) + Option(jarConnection.getJarEntry) // null if the entry disappeared from the jar in the meantime + .filterNot(_.isDirectory) + .map(entry => ResourceFile(url, entry.getSize, entry.getTime)) + case connection => fromUrlConnection(url, connection) + } + case _ => fromUrlConnection(url, url.openConnection()) } + + private def fromUrlConnection(url: URL, connection: URLConnection): Option[ResourceFile] = + try { + connection.setUseCaches(false) // otherwise the JDK will keep the connection open when we close! + val len = connection.getContentLength + val lm = connection.getLastModified + Some(ResourceFile(url, len, lm)) + } finally connection.getInputStream.close() } case class ResourceFile(url: URL, length: Long, lastModified: Long) From 911a46d47c78e82ffc514fc94801b09849a4bf59 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sat, 22 Aug 2026 22:45:35 +0100 Subject: [PATCH 2/4] don't register every uploaded temp file with deleteOnExit Motivation: `fileUploadAll` called `File.deleteOnExit()` for each temporary upload file. The JVM keeps every path passed to `deleteOnExit` in a global set for the lifetime of the process, and the entry is not removed when the file itself is deleted after the stream is consumed. A long-running server accepting uploads therefore grows its heap by one entry per upload, forever. Modification: Put the temporary upload files in a directory of their own and register a single shutdown hook that removes that directory recursively on exit. Result: The on-exit cleanup that the directive documents is unchanged, but it now costs one shutdown hook per JVM instead of one permanent global entry per uploaded file. The dedicated directory is created with the owner-only permissions that `Files.createTempDirectory` applies. Tests: - sbt "http-tests/testOnly org.apache.pekko.http.scaladsl.server.directives.FileUploadDirectivesSpec" - pass, 1 new test asserting the temp files share one directory - sbt http-tests/test - pass - sbt +http/compile - pass - sbt http/mimaReportBinaryIssues - pass - sbt http/scalafmt http-tests/Test/scalafmt - clean References: None - removes an unbounded deleteOnExit registration per upload --- .../directives/FileUploadDirectivesSpec.scala | 15 +++++++ .../directives/FileUploadDirectives.scala | 40 +++++++++++++++---- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectivesSpec.scala b/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectivesSpec.scala index ce7f211ca..e0a41b033 100644 --- a/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectivesSpec.scala +++ b/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectivesSpec.scala @@ -458,6 +458,21 @@ class FileUploadDirectivesSpec extends RoutingSpec with Eventually { } + "collect its temporary files in a single directory" in { + // the directory is removed by one shutdown hook, instead of registering every single uploaded file with + // `File.deleteOnExit`, which the JVM would remember for the lifetime of the process + val first = UploadTempFiles.create() + val second = UploadTempFiles.create() + try { + first.getParentFile.getName should startWith("pekko-http-uploads") + second.getParentFile shouldEqual first.getParentFile + (first should not).equal(second) + } finally { + first.delete() + second.delete() + } + } + } private def read(file: File): String = { diff --git a/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectives.scala b/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectives.scala index f5ca47005..0269ce578 100644 --- a/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectives.scala +++ b/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectives.scala @@ -14,7 +14,7 @@ package org.apache.pekko.http.scaladsl.server.directives import java.io.File -import java.nio.file.Files +import java.nio.file.{ Files, Path } import scala.collection.immutable import scala.concurrent.{ Future, Promise } @@ -22,7 +22,7 @@ import scala.util.{ Failure, Success } import org.apache.pekko import pekko.Done -import pekko.annotation.ApiMayChange +import pekko.annotation.{ ApiMayChange, InternalApi } import pekko.http.impl.util.StreamUtils import pekko.http.javadsl import pekko.http.scaladsl.model.{ ContentType, Multipart } @@ -174,11 +174,7 @@ trait FileUploadDirectives { extractRequestContext.flatMap { ctx => implicit val ec = ctx.executionContext - def tempDest(fileInfo: FileInfo): File = { - val dest = Files.createTempFile("pekko-http-upload", ".tmp").toFile - dest.deleteOnExit() - dest - } + def tempDest(fileInfo: FileInfo): File = UploadTempFiles.create() storeUploadedFiles(fieldName, tempDest).map { files => files.map { @@ -196,6 +192,36 @@ trait FileUploadDirectives { object FileUploadDirectives extends FileUploadDirectives +/** + * INTERNAL API + * + * Temporary files for uploads that the application may never consume. + * + * The files are collected in a directory of their own that a single shutdown hook removes on exit. Registering every + * file with `File.deleteOnExit` instead would keep its path in a JVM-wide set for the lifetime of the process, also + * long after the file itself has been deleted, so that a long-running server accepting uploads would slowly grow its + * heap. + */ +@InternalApi +private[directives] object UploadTempFiles { + private lazy val directory: Path = { + val dir = Files.createTempDirectory("pekko-http-uploads") // owner-only permissions where the file system has them + Runtime.getRuntime.addShutdownHook(new Thread( + () => deleteRecursively(dir.toFile), + "pekko-http-upload-cleanup")) + dir + } + + def create(): File = Files.createTempFile(directory, "pekko-http-upload", ".tmp").toFile + + private def deleteRecursively(file: File): Unit = { + val children = file.listFiles() + if (children ne null) children.foreach(deleteRecursively) + file.delete() + () + } +} + /** * Additional metadata about the file being uploaded/that was uploaded using the [[FileUploadDirectives]] * From d3bcbe6f9f78e565d10d5f0d739e284804310cf8 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sat, 22 Aug 2026 23:29:50 +0100 Subject: [PATCH 3/4] make the jar file cache for resource serving configurable Motivation: Reading jar resource metadata through the JDK's jar file cache means the jar file stays open for the lifetime of the process, which prevents the jar from being replaced while the server runs (on Windows an open file cannot be replaced). That should be a choice rather than something the directives decide. Modification: Add a `pekko.http.routing.use-jar-file-cache` setting, on by default, and pass it from `getFromResource` into `ResourceFile`. With the setting off, the connection that reads the entry metadata owns its jar file and closes it again, and the entity stream is opened through a connection with caches disabled as well, so that nothing keeps the jar open between requests. `ResourceFile.apply(url)` keeps its previous meaning and uses the cache. Result: The default is the cached behaviour, and deployments that need to replace jar files at runtime can turn the cache off. Note that the previous implementation could not offer that at all: it opened its own `ZipFile` for the metadata but still streamed the content through `URL.openStream`, which uses the JDK caches. Tests: - sbt "http-tests/testOnly org.apache.pekko.http.scaladsl.server.directives.FileAndResourceDirectivesSpec" - pass, 1 new test serving a jar resource with the cache disabled - sbt http-tests/test - pass (TimeoutDirectivesSpec flaked in the full run, passes on its own) - sbt +http/mimaReportBinaryIssues - pass - sbt http/scalafmt http-tests/Test/scalafmt - clean References: None - follow-up to the jar resource change on this branch --- .../FileAndResourceDirectivesSpec.scala | 16 +++++ .../routing-use-jar-file-cache.excludes | 20 ++++++ http/src/main/resources/reference.conf | 9 +++ .../impl/settings/RoutingSettingsImpl.scala | 6 +- .../javadsl/settings/RoutingSettings.scala | 10 +++ .../FileAndResourceDirectives.scala | 64 +++++++++++++------ .../scaladsl/settings/RoutingSettings.scala | 19 ++++++ 7 files changed, 123 insertions(+), 21 deletions(-) create mode 100644 http/src/main/mima-filters/2.0.x.backwards.excludes/routing-use-jar-file-cache.excludes diff --git a/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectivesSpec.scala b/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectivesSpec.scala index 73bcba045..de3f1e70f 100644 --- a/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectivesSpec.scala +++ b/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectivesSpec.scala @@ -280,6 +280,22 @@ class FileAndResourceDirectivesSpec extends RoutingSpec with Inspectors with Ins runCheck() runCheck() // the archive is shared between requests, so make sure it is still usable afterwards } + "return the resource content from an archive when the jar file cache is disabled" in { + val route = + withSettings(RoutingSettings(system).withUseJarFileCache(false)) { + getFromResource("com/typesafe/config/Config.class") + } + + def runCheck() = + Get() ~> route ~> check { + val entity = responseEntity.toStrict(1.second.dilated).awaitResult(1.second.dilated) + entity.contentLength shouldEqual entity.data.length + entity.data.asByteBuffer.getInt shouldEqual 0xCAFEBABE + } + + runCheck() + runCheck() // every request opens and closes the jar file of its own, so make sure that is repeatable + } "return the file content with MediaType 'application/octet-stream' on unknown file extensions" in { Get() ~> getFromResource("sample.xyz") ~> check { mediaType shouldEqual `application/octet-stream` diff --git a/http/src/main/mima-filters/2.0.x.backwards.excludes/routing-use-jar-file-cache.excludes b/http/src/main/mima-filters/2.0.x.backwards.excludes/routing-use-jar-file-cache.excludes new file mode 100644 index 000000000..c64d04b47 --- /dev/null +++ b/http/src/main/mima-filters/2.0.x.backwards.excludes/routing-use-jar-file-cache.excludes @@ -0,0 +1,20 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF 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. + +# new use-jar-file-cache routing setting +ProblemFilters.exclude[ReversedMissingMethodProblem]("org.apache.pekko.http.javadsl.settings.RoutingSettings.getUseJarFileCache") +ProblemFilters.exclude[ReversedMissingMethodProblem]("org.apache.pekko.http.scaladsl.settings.RoutingSettings.useJarFileCache") diff --git a/http/src/main/resources/reference.conf b/http/src/main/resources/reference.conf index e3f50b7d9..1c0b69e98 100644 --- a/http/src/main/resources/reference.conf +++ b/http/src/main/resources/reference.conf @@ -19,6 +19,15 @@ pekko.http { # Enables/disables ETag and `If-Modified-Since` support for FileAndResourceDirectives file-get-conditional = on + # Enables/disables the use of the JDK's jar file cache when FileAndResourceDirectives serve a resource that + # lives in a jar file. This is the same cache that the class loader uses, so with the cache enabled a resource + # is served without opening and parsing the jar file for every single request. + # + # Turn this off if the jar files that resources are served from have to be replaceable while the server is + # running (an open jar file cannot be replaced on Windows). Note that this makes every request open and parse + # the jar file again. + use-jar-file-cache = on + # Enables/disables the rendering of the "rendered by" footer in directory listings render-vanity-footer = yes diff --git a/http/src/main/scala/org/apache/pekko/http/impl/settings/RoutingSettingsImpl.scala b/http/src/main/scala/org/apache/pekko/http/impl/settings/RoutingSettingsImpl.scala index 29012c2e3..edc4f5d26 100644 --- a/http/src/main/scala/org/apache/pekko/http/impl/settings/RoutingSettingsImpl.scala +++ b/http/src/main/scala/org/apache/pekko/http/impl/settings/RoutingSettingsImpl.scala @@ -28,7 +28,8 @@ private[http] final case class RoutingSettingsImpl( rangeCountLimit: Int, rangeCoalescingThreshold: Long, decodeMaxBytesPerChunk: Int, - decodeMaxSize: Long) extends pekko.http.scaladsl.settings.RoutingSettings { + decodeMaxSize: Long, + useJarFileCache: Boolean) extends pekko.http.scaladsl.settings.RoutingSettings { override def productPrefix = "RoutingSettings" } @@ -41,5 +42,6 @@ object RoutingSettingsImpl extends SettingsCompanionImpl[RoutingSettingsImpl]("p c.getInt("range-count-limit"), c.getBytes("range-coalescing-threshold"), c.getIntBytes("decode-max-bytes-per-chunk"), - c.getPossiblyInfiniteBytes("decode-max-size")) + c.getPossiblyInfiniteBytes("decode-max-size"), + c.getBoolean("use-jar-file-cache")) } diff --git a/http/src/main/scala/org/apache/pekko/http/javadsl/settings/RoutingSettings.scala b/http/src/main/scala/org/apache/pekko/http/javadsl/settings/RoutingSettings.scala index 3bc0d1bfa..6f4f9d7d8 100644 --- a/http/src/main/scala/org/apache/pekko/http/javadsl/settings/RoutingSettings.scala +++ b/http/src/main/scala/org/apache/pekko/http/javadsl/settings/RoutingSettings.scala @@ -32,6 +32,11 @@ abstract class RoutingSettings private[pekko] () { self: RoutingSettingsImpl => def getRangeCoalescingThreshold: Long def getDecodeMaxBytesPerChunk: Int + /** + * @since 2.0.0 + */ + def getUseJarFileCache: Boolean + def withVerboseErrorMessages(verboseErrorMessages: Boolean): RoutingSettings = self.copy(verboseErrorMessages = verboseErrorMessages) def withFileGetConditional(fileGetConditional: Boolean): RoutingSettings = @@ -44,6 +49,11 @@ abstract class RoutingSettings private[pekko] () { self: RoutingSettingsImpl => def withDecodeMaxBytesPerChunk(decodeMaxBytesPerChunk: Int): RoutingSettings = self.copy(decodeMaxBytesPerChunk = decodeMaxBytesPerChunk) def withDecodeMaxSize(decodeMaxSize: Long): RoutingSettings = self.copy(decodeMaxSize = decodeMaxSize) + + /** + * @since 2.0.0 + */ + def withUseJarFileCache(useJarFileCache: Boolean): RoutingSettings = self.copy(useJarFileCache = useJarFileCache) } object RoutingSettings extends SettingsCompanion[RoutingSettings] { diff --git a/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala b/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala index cfe6198e9..14231a9d3 100644 --- a/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala +++ b/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala @@ -14,7 +14,7 @@ package org.apache.pekko.http.scaladsl.server package directives -import java.io.File +import java.io.{ File, FileNotFoundException, InputStream } import java.net.{ JarURLConnection, URL, URLConnection } import scala.annotation.tailrec @@ -109,17 +109,20 @@ trait FileAndResourceDirectives { resourceName: String, contentType: ContentType, classLoader: ClassLoader = _defaultClassLoader): Route = if (!resourceName.endsWith('/')) get { - Option(classLoader.getResource(resourceName)).flatMap(ResourceFile.apply) match { - case Some(ResourceFile(url, length, lastModified)) => - conditionalFor(length, lastModified) { - if (length > 0) { - withRangeSupportAndPrecompressedMediaTypeSupport { - complete(HttpEntity.Default(contentType, length, - StreamConverters.fromInputStream(() => url.openStream()))) - } - } else complete(HttpEntity.Empty) - } - case _ => reject // not found or directory + extractSettings { settings => + val useJarFileCache = settings.useJarFileCache + Option(classLoader.getResource(resourceName)).flatMap(ResourceFile(_, useJarFileCache)) match { + case Some(ResourceFile(url, length, lastModified)) => + conditionalFor(length, lastModified) { + if (length > 0) { + withRangeSupportAndPrecompressedMediaTypeSupport { + complete(HttpEntity.Default(contentType, length, + StreamConverters.fromInputStream(() => openStream(url, useJarFileCache)))) + } + } else complete(HttpEntity.Empty) + } + case _ => reject // not found or directory + } } } else reject // don't serve the content of resource "directories" @@ -278,7 +281,14 @@ object FileAndResourceDirectives extends FileAndResourceDirectives { } object ResourceFile { - def apply(url: URL): Option[ResourceFile] = url.getProtocol match { + def apply(url: URL): Option[ResourceFile] = apply(url, useJarFileCache = true) + + /** + * @param useJarFileCache whether the JDK's jar file cache may be used for resources inside a jar file, see the + * `pekko.http.routing.use-jar-file-cache` setting + * @since 2.0.0 + */ + def apply(url: URL, useJarFileCache: Boolean): Option[ResourceFile] = url.getProtocol match { case "file" => val file = new File(url.toURI) if (file.isDirectory) None @@ -287,13 +297,17 @@ object FileAndResourceDirectives extends FileAndResourceDirectives { url.openConnection() match { case jarConnection: JarURLConnection => // Ask the connection for the entry instead of opening the jar file here: opening it means reading and - // parsing the whole central directory again for every single request. With caching left enabled the JDK + // parsing the whole central directory again for every single request. With the cache enabled the JDK // reuses the same open jar file as the class loader does, so nothing is opened here at all in the common - // case (and nothing must be closed either, the cached jar file is shared). - jarConnection.setUseCaches(true) - Option(jarConnection.getJarEntry) // null if the entry disappeared from the jar in the meantime - .filterNot(_.isDirectory) - .map(entry => ResourceFile(url, entry.getSize, entry.getTime)) + // case. Without it this connection owns the jar file and has to close it again. + jarConnection.setUseCaches(useJarFileCache) + try { + val entry = Option(jarConnection.getJarEntry).filterNot(_.isDirectory) + if (!useJarFileCache) jarConnection.getJarFile.close() + entry.map(e => ResourceFile(url, e.getSize, e.getTime)) + } catch { + case _: FileNotFoundException => None // the entry disappeared from the jar in the meantime + } case connection => fromUrlConnection(url, connection) } case _ => fromUrlConnection(url, url.openConnection()) @@ -307,6 +321,18 @@ object FileAndResourceDirectives extends FileAndResourceDirectives { Some(ResourceFile(url, len, lm)) } finally connection.getInputStream.close() } + + /** + * Opens the resource content. `URL.openStream` would always use the JDK's caches, so when they are disabled the + * connection has to be set up by hand. Closing the returned stream then also closes the jar file it came from. + */ + private def openStream(url: URL, useJarFileCache: Boolean): InputStream = + if (useJarFileCache || url.getProtocol != "jar") url.openStream() + else { + val connection = url.openConnection() + connection.setUseCaches(false) + connection.getInputStream + } case class ResourceFile(url: URL, length: Long, lastModified: Long) trait DirectoryRenderer extends pekko.http.javadsl.server.directives.DirectoryRenderer { diff --git a/http/src/main/scala/org/apache/pekko/http/scaladsl/settings/RoutingSettings.scala b/http/src/main/scala/org/apache/pekko/http/scaladsl/settings/RoutingSettings.scala index 9bd12b754..0c084ffb9 100644 --- a/http/src/main/scala/org/apache/pekko/http/scaladsl/settings/RoutingSettings.scala +++ b/http/src/main/scala/org/apache/pekko/http/scaladsl/settings/RoutingSettings.scala @@ -33,6 +33,14 @@ abstract class RoutingSettings private[pekko] () extends pekko.http.javadsl.sett def decodeMaxBytesPerChunk: Int def decodeMaxSize: Long + /** + * Whether resources that live in a jar file are served through the JDK's jar file cache, the same cache the class + * loader uses, instead of opening and parsing the jar file for every request. + * + * @since 2.0.0 + */ + def useJarFileCache: Boolean + /* Java APIs */ def getVerboseErrorMessages: Boolean = this.verboseErrorMessages def getFileGetConditional: Boolean = this.fileGetConditional @@ -42,6 +50,11 @@ abstract class RoutingSettings private[pekko] () extends pekko.http.javadsl.sett def getDecodeMaxBytesPerChunk: Int = this.decodeMaxBytesPerChunk def getDecodeMaxSize: Long = this.decodeMaxSize + /** + * @since 2.0.0 + */ + def getUseJarFileCache: Boolean = this.useJarFileCache + override def withVerboseErrorMessages(verboseErrorMessages: Boolean): RoutingSettings = self.copy(verboseErrorMessages = verboseErrorMessages) override def withFileGetConditional(fileGetConditional: Boolean): RoutingSettings = @@ -54,6 +67,12 @@ abstract class RoutingSettings private[pekko] () extends pekko.http.javadsl.sett override def withDecodeMaxBytesPerChunk(decodeMaxBytesPerChunk: Int): RoutingSettings = self.copy(decodeMaxBytesPerChunk = decodeMaxBytesPerChunk) override def withDecodeMaxSize(decodeMaxSize: Long): RoutingSettings = self.copy(decodeMaxSize = decodeMaxSize) + + /** + * @since 2.0.0 + */ + override def withUseJarFileCache(useJarFileCache: Boolean): RoutingSettings = + self.copy(useJarFileCache = useJarFileCache) } object RoutingSettings extends SettingsCompanion[RoutingSettings] { From 300cacafa7811e359648a1f5ca95b0e3addb3a15 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Mon, 31 Aug 2026 14:35:44 +0100 Subject: [PATCH 4/4] address review: shutdown ordering, jar connection ownership, config claims Motivation: Review of the branch found problems in both halves. The raw Runtime.addShutdownHook ran concurrently with CoordinatedShutdown's hook, so during a graceful drain it could delete temp files that in-flight uploads still use - deleteOnExit provably deleted only after application hooks finished. The memoized upload directory was never recreated if a temp-file reaper removed it while empty, failing all later uploads until restart. In the jar path, the cache-off close was not in a finally (leaking a JarFile per failing request), an explicit setUseCaches(true) silently defeated an application-wide URLConnection.setDefaultUseCaches(false), the public one-arg ResourceFile.apply changed semantics by pinning jars in the JDK cache, getContentLength truncated and turned unknown lengths into silent empty 200s, an exception from the close in fromUrlConnection's finally became a 500 where callers expect a rejection, and the cache decision was spread over three hand-synchronized places. The use-jar-file-cache documentation also wrongly claimed the JDK's jar cache is the class loader's cache and implied disabling it makes classpath jars replaceable. Modification: Make UploadTempFiles a per-actor-system extension whose directory is removed by a CoordinatedShutdown task in the actor-system-terminate phase, after the drain; recreate the directory in create() if it is gone. In ResourceFile, restore the one-arg apply to its historical no-handle-kept semantics (useJarFileCache = false), decide jar ownership from the connection's effective getUseCaches, close the owned jar in a guarded finally, only call setUseCaches when disabling, centralize that rule in one openConnection helper shared with openStream, dispatch on the connection type instead of the protocol string, use getContentLengthLong and reject unknown lengths, and map FileNotFoundException to None with a guarded stream close. Reword the reference.conf entry to scope the replaceability promise to jars no class loader holds open and document the two-parse cost of cache-off mode. Rebased onto main. Result: Upload temp files are deleted only after the system has drained and uploads keep working if the temp directory disappears; the jar path neither leaks handles nor overrides application-wide cache opt-outs; external ResourceFile(url) callers keep the pre-existing behavior; and the configuration text makes no false claims about the JDK. Tests: - sbt "http-tests/testOnly org.apache.pekko.http.scaladsl.server.directives.FileUploadDirectivesSpec org.apache.pekko.http.scaladsl.server.directives.FileAndResourceDirectivesSpec" - pass (73 tests); new tests cover directory recreation after removal and directory deletion when a separate actor system terminates - sbt http/mimaReportBinaryIssues - pass - sbt "+http/compile" - pass on 2.13.18 and 3.3.8 - native scalafmt run on the changed Scala files - clean References: Refs #1242 - keeps the threat model's "registers no JVM shutdown hook" claim true --- .../directives/FileUploadDirectivesSpec.scala | 35 ++++++++-- http/src/main/resources/reference.conf | 10 +-- .../FileAndResourceDirectives.scala | 65 ++++++++++++------- .../directives/FileUploadDirectives.scala | 42 ++++++++---- 4 files changed, 109 insertions(+), 43 deletions(-) diff --git a/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectivesSpec.scala b/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectivesSpec.scala index e0a41b033..232daada0 100644 --- a/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectivesSpec.scala +++ b/http-tests/src/test/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectivesSpec.scala @@ -16,7 +16,7 @@ package org.apache.pekko.http.scaladsl.server.directives import java.io.File import java.nio.file.Files -import scala.concurrent.Future +import scala.concurrent.{ Await, Future } import scala.concurrent.duration._ import org.apache.pekko @@ -29,6 +29,7 @@ import pekko.testkit._ import pekko.util.ByteString import org.scalatest.concurrent.Eventually +import org.scalatest.time.{ Seconds, Span } class FileUploadDirectivesSpec extends RoutingSpec with Eventually { @@ -459,10 +460,12 @@ class FileUploadDirectivesSpec extends RoutingSpec with Eventually { } "collect its temporary files in a single directory" in { - // the directory is removed by one shutdown hook, instead of registering every single uploaded file with - // `File.deleteOnExit`, which the JVM would remember for the lifetime of the process - val first = UploadTempFiles.create() - val second = UploadTempFiles.create() + // the directory is removed by a CoordinatedShutdown task when the actor system terminates, instead of + // registering every single uploaded file with `File.deleteOnExit`, which the JVM would remember for the + // lifetime of the process + val uploadTempFiles = UploadTempFiles(system) + val first = uploadTempFiles.create() + val second = uploadTempFiles.create() try { first.getParentFile.getName should startWith("pekko-http-uploads") second.getParentFile shouldEqual first.getParentFile @@ -473,6 +476,28 @@ class FileUploadDirectivesSpec extends RoutingSpec with Eventually { } } + "recreate its temporary directory when it was removed while the server runs" in { + val uploadTempFiles = UploadTempFiles(system) + val first = uploadTempFiles.create() + val directory = first.getParentFile + first.delete() + Files.delete(directory.toPath) // a temp-file reaper may remove the directory whenever it is empty + val second = uploadTempFiles.create() + second.getParentFile shouldEqual directory + second.delete() + } + + "remove its temporary directory when the actor system terminates" in { + val separateSystem = pekko.actor.ActorSystem("upload-temp-files-cleanup-spec") + val file = UploadTempFiles(separateSystem).create() + val directory = file.getParentFile + directory.exists() shouldBe true + Await.ready(separateSystem.terminate(), 10.seconds.dilated) + eventually(timeout(Span(3, Seconds))) { + directory.exists() shouldBe false + } + } + } private def read(file: File): String = { diff --git a/http/src/main/resources/reference.conf b/http/src/main/resources/reference.conf index 1c0b69e98..7410f8afe 100644 --- a/http/src/main/resources/reference.conf +++ b/http/src/main/resources/reference.conf @@ -20,12 +20,14 @@ pekko.http { file-get-conditional = on # Enables/disables the use of the JDK's jar file cache when FileAndResourceDirectives serve a resource that - # lives in a jar file. This is the same cache that the class loader uses, so with the cache enabled a resource - # is served without opening and parsing the jar file for every single request. + # lives in a jar file. With the cache enabled the jar is opened once and kept open, so a resource is served + # without opening and parsing the jar file for every single request; with it disabled every request opens and + # parses the jar once for the resource's metadata and once more for its content. # # Turn this off if the jar files that resources are served from have to be replaceable while the server is - # running (an open jar file cannot be replaced on Windows). Note that this makes every request open and parse - # the jar file again. + # running (an open jar file cannot be replaced on Windows). This can only help for jars that no class loader + # holds open: a class loader keeps its own handle to every jar it loads classes from for the lifetime of the + # JVM, unaffected by this setting, so a jar on the class path stays locked either way. use-jar-file-cache = on # Enables/disables the rendering of the "rendered by" footer in directory listings diff --git a/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala b/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala index 14231a9d3..9d8882e2b 100644 --- a/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala +++ b/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileAndResourceDirectives.scala @@ -14,7 +14,7 @@ package org.apache.pekko.http.scaladsl.server package directives -import java.io.{ File, FileNotFoundException, InputStream } +import java.io.{ File, FileNotFoundException, IOException, InputStream } import java.net.{ JarURLConnection, URL, URLConnection } import scala.annotation.tailrec @@ -281,7 +281,13 @@ object FileAndResourceDirectives extends FileAndResourceDirectives { } object ResourceFile { - def apply(url: URL): Option[ResourceFile] = apply(url, useJarFileCache = true) + + /** + * Probes the resource without keeping any handle open: for a resource inside a jar file the jar is opened, read + * and closed again, as this overload always did before the jar file cache became configurable. Use the + * two-argument overload to let the JDK's jar file cache keep the jar open across requests. + */ + def apply(url: URL): Option[ResourceFile] = apply(url, useJarFileCache = false) /** * @param useJarFileCache whether the JDK's jar file cache may be used for resources inside a jar file, see the @@ -293,46 +299,61 @@ object FileAndResourceDirectives extends FileAndResourceDirectives { val file = new File(url.toURI) if (file.isDirectory) None else Some(ResourceFile(url, file.length(), file.lastModified())) - case "jar" => - url.openConnection() match { + case _ => + openConnection(url, useJarFileCache) match { case jarConnection: JarURLConnection => // Ask the connection for the entry instead of opening the jar file here: opening it means reading and - // parsing the whole central directory again for every single request. With the cache enabled the JDK - // reuses the same open jar file as the class loader does, so nothing is opened here at all in the common - // case. Without it this connection owns the jar file and has to close it again. - jarConnection.setUseCaches(useJarFileCache) + // parsing the whole central directory again for every single request. With the JDK's cache in use the + // same open jar file is reused across requests, so nothing is opened here at all in the common case. + // A connection that does not use the cache owns its jar file and has to close it again. + val ownsJarFile = !jarConnection.getUseCaches try { val entry = Option(jarConnection.getJarEntry).filterNot(_.isDirectory) - if (!useJarFileCache) jarConnection.getJarFile.close() entry.map(e => ResourceFile(url, e.getSize, e.getTime)) } catch { case _: FileNotFoundException => None // the entry disappeared from the jar in the meantime - } + } finally if (ownsJarFile) { + try jarConnection.getJarFile.close() + catch { case _: IOException => } // the jar itself may be gone, then there is nothing to close + } case connection => fromUrlConnection(url, connection) } - case _ => fromUrlConnection(url, url.openConnection()) } private def fromUrlConnection(url: URL, connection: URLConnection): Option[ResourceFile] = try { connection.setUseCaches(false) // otherwise the JDK will keep the connection open when we close! - val len = connection.getContentLength + val len = connection.getContentLengthLong val lm = connection.getLastModified - Some(ResourceFile(url, len, lm)) - } finally connection.getInputStream.close() + if (len < 0) None // an unknown length would be served as an empty response, so reject instead + else Some(ResourceFile(url, len, lm)) + } catch { + case _: FileNotFoundException => None // nothing behind the URL: reject instead of failing the request + } finally { + try connection.getInputStream.close() + catch { case _: IOException => } // a stream that cannot be opened holds nothing to close + } + + /** + * The one place that decides how a connection relates to the JDK's caches: with the jar file cache disabled the + * connection must own its jar file, with it enabled the JVM-wide default is left untouched, so that an + * application-wide `URLConnection.setDefaultUseCaches(false)` keeps its effect. + */ + private[directives] def openConnection(url: URL, useJarFileCache: Boolean): URLConnection = { + val connection = url.openConnection() + if (!useJarFileCache) connection.setUseCaches(false) + connection + } } /** - * Opens the resource content. `URL.openStream` would always use the JDK's caches, so when they are disabled the - * connection has to be set up by hand. Closing the returned stream then also closes the jar file it came from. + * Opens the resource content. Metadata and content are read through separate connections, so with the jar file + * cache disabled a jar-hosted resource is opened and parsed once more here: the entity may never be materialized + * at all (HEAD or conditional requests), so the metadata connection cannot simply be handed over. Closing the + * returned stream also closes the jar file it came from when the connection owns it. */ private def openStream(url: URL, useJarFileCache: Boolean): InputStream = - if (useJarFileCache || url.getProtocol != "jar") url.openStream() - else { - val connection = url.openConnection() - connection.setUseCaches(false) - connection.getInputStream - } + ResourceFile.openConnection(url, useJarFileCache).getInputStream case class ResourceFile(url: URL, length: Long, lastModified: Long) trait DirectoryRenderer extends pekko.http.javadsl.server.directives.DirectoryRenderer { diff --git a/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectives.scala b/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectives.scala index 0269ce578..0b5070156 100644 --- a/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectives.scala +++ b/http/src/main/scala/org/apache/pekko/http/scaladsl/server/directives/FileUploadDirectives.scala @@ -22,6 +22,7 @@ import scala.util.{ Failure, Success } import org.apache.pekko import pekko.Done +import pekko.actor.{ CoordinatedShutdown, ExtendedActorSystem, Extension, ExtensionId } import pekko.annotation.{ ApiMayChange, InternalApi } import pekko.http.impl.util.StreamUtils import pekko.http.javadsl @@ -165,7 +166,7 @@ trait FileUploadDirectives { * Collects each body part that is a multipart file as a tuple containing metadata and a `Source` * for streaming the file contents somewhere. If there is no such field the request will be rejected. * Files are buffered into temporary files on disk so in-memory buffers don't overflow. The temporary - * files are cleaned up once materialized, or on exit if the stream is not consumed. + * files are cleaned up once materialized, or when the actor system terminates if the stream is not consumed. * * @group fileupload */ @@ -174,7 +175,8 @@ trait FileUploadDirectives { extractRequestContext.flatMap { ctx => implicit val ec = ctx.executionContext - def tempDest(fileInfo: FileInfo): File = UploadTempFiles.create() + val uploadTempFiles = UploadTempFiles(ctx.materializer.system) + def tempDest(fileInfo: FileInfo): File = uploadTempFiles.create() storeUploadedFiles(fieldName, tempDest).map { files => files.map { @@ -197,22 +199,32 @@ object FileUploadDirectives extends FileUploadDirectives * * Temporary files for uploads that the application may never consume. * - * The files are collected in a directory of their own that a single shutdown hook removes on exit. Registering every - * file with `File.deleteOnExit` instead would keep its path in a JVM-wide set for the lifetime of the process, also - * long after the file itself has been deleted, so that a long-running server accepting uploads would slowly grow its - * heap. + * The files are collected in a directory of their own, one per actor system, that a `CoordinatedShutdown` task + * removes in the `actor-system-terminate` phase — after in-flight requests have been drained, matching the ordering + * that `File.deleteOnExit` provided (its hook runs only after all application shutdown hooks have finished; a raw + * `Runtime.addShutdownHook` would run concurrently with the drain and could delete files that requests still use). + * Registering every file with `deleteOnExit` instead would keep its path in a JVM-wide set for the lifetime of the + * process, also long after the file itself has been deleted, so that a long-running server accepting uploads would + * slowly grow its heap. */ @InternalApi -private[directives] object UploadTempFiles { - private lazy val directory: Path = { +private[directives] final class UploadTempFiles(system: ExtendedActorSystem) extends Extension { + private val directory: Path = { val dir = Files.createTempDirectory("pekko-http-uploads") // owner-only permissions where the file system has them - Runtime.getRuntime.addShutdownHook(new Thread( - () => deleteRecursively(dir.toFile), - "pekko-http-upload-cleanup")) + CoordinatedShutdown(system).addTask(CoordinatedShutdown.PhaseActorSystemTerminate, "pekko-http-upload-cleanup") { + () => + deleteRecursively(dir.toFile) + Future.successful(Done) + } dir } - def create(): File = Files.createTempFile(directory, "pekko-http-upload", ".tmp").toFile + def create(): File = { + // the directory is empty whenever all uploads have been consumed, and an empty directory in the system temp + // location may be removed by a temp-file reaper while the server runs, so recreate it rather than fail uploads + Files.createDirectories(directory) + Files.createTempFile(directory, "pekko-http-upload", ".tmp").toFile + } private def deleteRecursively(file: File): Unit = { val children = file.listFiles() @@ -222,6 +234,12 @@ private[directives] object UploadTempFiles { } } +/** INTERNAL API */ +@InternalApi +private[directives] object UploadTempFiles extends ExtensionId[UploadTempFiles] { + def createExtension(system: ExtendedActorSystem): UploadTempFiles = new UploadTempFiles(system) +} + /** * Additional metadata about the file being uploaded/that was uploaded using the [[FileUploadDirectives]] *