From 4b70c9aa210cbd70bb39283273ee551a226f924c Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Mon, 5 Oct 2026 15:15:41 +0000 Subject: [PATCH] :bug: Reserve S3 download path without creating the file The S3 file download opens the temp file with CREATE_NEW, so it fails when the path already exists. tempfile creates the empty file on reserve, which made every large download depend on the SDK deleting the placeholder between its own retries. Add tmp/temp-path, which reserves a unique path and registers cleanup without creating the file, and use it for S3 downloads. The old retry on FileAlreadyExists stays as a safety net for a name clash. AI-assisted-by: muse-spark-1.3-contributor --- backend/src/app/storage/s3.clj | 2 +- backend/src/app/storage/tmp.clj | 19 +++++++++ .../test/backend_tests/storage_tmp_test.clj | 40 +++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 backend/test/backend_tests/storage_tmp_test.clj diff --git a/backend/src/app/storage/s3.clj b/backend/src/app/storage/s3.clj index 025749bee1..676660c95a 100644 --- a/backend/src/app/storage/s3.clj +++ b/backend/src/app/storage/s3.clj @@ -316,7 +316,7 @@ ;; to the filesystem and then read with buffered inputstream; if ;; not, read the contento into memory using bytearrays. (if (> ^long size (* 1024 1024 2)) - (let [path (tmp/tempfile :prefix "penpot.storage.s3." :min-age "6h") + (let [path (tmp/temp-path :prefix "penpot.storage.s3." :min-age "6h") rxf (AsyncResponseTransformer/toFile ^Path path)] (->> (.getObject ^S3AsyncClient client ^GetObjectRequest gor diff --git a/backend/src/app/storage/tmp.clj b/backend/src/app/storage/tmp.clj index b7a3076159..987ea4a1de 100644 --- a/backend/src/app/storage/tmp.clj +++ b/backend/src/app/storage/tmp.clj @@ -93,6 +93,25 @@ (sp/offer! queue [path (some-> min-age ct/duration)]) path)) +(defn temp-path + "Reserve a unique temp path WITHOUT creating the file. + + Use it when an external writer needs to create the file itself and + fails if it already exists (like the S3 SDK file download, which + opens with CREATE_NEW). The path is still registered for cleanup, + so a crash between reserve and write does not leak." + [& {:keys [suffix prefix dir min-age] + :or {prefix "penpot." + suffix ".tmp" + dir default-tmp-dir}}] + (fs/create-dir dir) + (loop [path (fs/join dir (str prefix (uuid/next) suffix))] + (if (fs/exists? path) + (recur (fs/join dir (str prefix (uuid/next) suffix))) + (do + (sp/offer! queue [path (some-> min-age ct/duration)]) + path)))) + (defn tempfile-from "Create a new tempfile from from consuming the stream" [input & {:as options}] diff --git a/backend/test/backend_tests/storage_tmp_test.clj b/backend/test/backend_tests/storage_tmp_test.clj new file mode 100644 index 0000000000..b9141cf20b --- /dev/null +++ b/backend/test/backend_tests/storage_tmp_test.clj @@ -0,0 +1,40 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns backend-tests.storage-tmp-test + (:require + [app.storage.tmp :as tmp] + [clojure.test :as t] + [datoteka.fs :as fs]) + (:import + java.nio.file.Files + java.nio.file.Path)) + +(t/deftest temp-path-does-not-create-the-file + (let [path (tmp/temp-path :prefix "penpot.test." :min-age "6h")] + (t/testing "the reserved path does not exist yet" + (t/is (not (fs/exists? path)))) + (t/testing "an external writer can create it with CREATE_NEW" + (Files/createFile ^Path path (into-array java.nio.file.attribute.FileAttribute [])) + (t/is (fs/exists? path))) + (fs/delete path))) + +(t/deftest temp-path-is-unique + (let [a (tmp/temp-path :prefix "penpot.test.") + b (tmp/temp-path :prefix "penpot.test.")] + (t/is (not= a b)) + (t/is (not (fs/exists? a))) + (t/is (not (fs/exists? b))))) + +(t/deftest tempfile-creates-the-file-so-create-new-fails + (t/testing "documents why s3 downloads cannot use tempfile: the SDK opens with CREATE_NEW" + (let [path (tmp/tempfile :prefix "penpot.test.")] + (try + (t/is (fs/exists? path)) + (t/is (thrown? java.nio.file.FileAlreadyExistsException + (Files/createFile ^Path path (into-array java.nio.file.attribute.FileAttribute [])))) + (finally + (fs/delete path))))))