From 162a381aedb0aa5d7244d7fb26f6f1d131ed5a15 Mon Sep 17 00:00:00 2001 From: Jan Kahmen <36455663+kah-ja@users.noreply.github.com> Date: Tue, 18 Aug 2026 13:33:24 +0200 Subject: [PATCH] :bug: Apply the asset attachment disposition on the s3 backend too (#10989) 29dbf9ab1 marks non public buckets as attachments, which works on the fs backend because nginx applies those headers to the internally redirected response. On the s3 backend the handler answers 307 and the client then fetches the bytes from the object store, so the header set on the redirect does not reach the response that carries the object. Sign the disposition into the presigned url as well, so the object store returns it. It is only signed when the bucket is not public, so urls for inline served objects are unchanged. Also cover the disposition in the handler tests, for the non public buckets and for the public ones that stay inline. Co-authored-by: Andrey Antukh --- backend/src/app/http/assets.clj | 12 ++++- backend/src/app/storage/s3.clj | 16 +++++-- .../test/backend_tests/http_assets_test.clj | 44 +++++++++++++++++++ 3 files changed, 66 insertions(+), 6 deletions(-) diff --git a/backend/src/app/http/assets.clj b/backend/src/app/http/assets.clj index 6258760548..98ba0d8ba5 100644 --- a/backend/src/app/http/assets.clj +++ b/backend/src/app/http/assets.clj @@ -49,13 +49,21 @@ [{:keys [::sto/storage ::signature-max-age ::cache-max-age] :as cfg} obj] (let [sig-max-age (or signature-max-age default-signature-max-age) cch-max-age (or cache-max-age default-cache-max-age) - {:keys [host port] :as url} (sto/get-object-url storage obj {:max-age sig-max-age}) bucket (-> obj meta :bucket) + public? (contains? public-buckets bucket) + ;; The disposition is also signed into the presigned url: this + ;; response is a redirect, so the header below applies to the + ;; redirect itself and not to the bytes the client then fetches + ;; from the object store. + {:keys [host port] :as url} (sto/get-object-url storage obj + (cond-> {:max-age sig-max-age} + (not public?) + (assoc :content-disposition "attachment"))) headers (cond-> {"location" (str url) "x-host" (cond-> host port (str ":" port)) "x-mtype" (-> obj meta :content-type) "cache-control" (str "max-age=" (inst-ms cch-max-age))} - (not (contains? public-buckets bucket)) + (not public?) (assoc "content-disposition" "attachment"))] {::yres/status 307 ::yres/headers headers})) diff --git a/backend/src/app/storage/s3.clj b/backend/src/app/storage/s3.clj index 025749bee1..a56a23d99d 100644 --- a/backend/src/app/storage/s3.clj +++ b/backend/src/app/storage/s3.clj @@ -346,13 +346,21 @@ (ct/duration {:minutes 10})) (defn- get-object-url - [{:keys [::presigner ::bucket ::prefix]} {:keys [id]} {:keys [max-age] :or {max-age default-max-age}}] + [{:keys [::presigner ::bucket ::prefix]} {:keys [id]} + {:keys [max-age content-disposition] :or {max-age default-max-age}}] (assert (ct/duration? max-age) "expected valid duration instance") - (let [gor (.. (GetObjectRequest/builder) + ;; The content-disposition option is signed into the presigned url, so the + ;; object store sets that header on the response the client fetches after + ;; following the redirect. It is only set when asked for, so urls for + ;; objects served inline stay byte identical to before. + (let [gorb (.. (GetObjectRequest/builder) (bucket bucket) - (key (dm/str prefix (impl/id->path id))) - (build)) + (key (dm/str prefix (impl/id->path id)))) + gorb (cond-> gorb + (some? content-disposition) + (.responseContentDisposition ^String content-disposition)) + gor (.build gorb) gopr (.. (GetObjectPresignRequest/builder) (signatureDuration ^Duration max-age) (getObjectRequest ^GetObjectRequest gor) diff --git a/backend/test/backend_tests/http_assets_test.clj b/backend/test/backend_tests/http_assets_test.clj index 94510d73d6..bac3dbfe9a 100644 --- a/backend/test/backend_tests/http_assets_test.clj +++ b/backend/test/backend_tests/http_assets_test.clj @@ -269,6 +269,50 @@ (t/is (string? redirect)) (t/is (clojure.string/includes? redirect (sto/object->relative-path object))))) +;; ---------------------------------------------------------------- +;; Tests: objects-handler — content disposition +;; ---------------------------------------------------------------- + +(t/deftest objects-handler-non-public-bucket-served-as-attachment + ;; A non-public bucket holds bytes the user uploaded and is reachable by + ;; direct navigation, so the response marks it as an attachment. + (let [storage (-> (:app.storage/storage th/*system*) + (configure-storage-backend)) + cfg (make-handler-cfg storage) + profile (th/create-profile* 1)] + + (doseq [bucket ["profile" + "tempfile" + "file-data" + "file-thumbnail" + "file-change"]] + (t/testing (str "bucket: " bucket) + (let [object (create-storage-object! storage bucket "some data") + request {:path-params {:id (str (:id object))} + ::session/profile-id (:id profile)} + response (assets/objects-handler cfg request)] + (t/is (= "attachment" (get (::yres/headers response) "content-disposition")) + (str "bucket " bucket " should be served as an attachment"))))))) + +(t/deftest objects-handler-public-bucket-served-inline + ;; Public buckets are embedded by the viewer and by outgoing mail, so they + ;; keep being served without a disposition. + (let [storage (-> (:app.storage/storage th/*system*) + (configure-storage-backend)) + cfg (make-handler-cfg storage)] + + (doseq [bucket ["file-media-object" + "file-object-thumbnail" + "team-font-variant" + "file-data-fragment" + "organization"]] + (t/testing (str "bucket: " bucket) + (let [object (create-storage-object! storage bucket "some data") + request {:path-params {:id (str (:id object))}} + response (assets/objects-handler cfg request)] + (t/is (nil? (get (::yres/headers response) "content-disposition")) + (str "bucket " bucket " should stay inline"))))))) + ;; ---------------------------------------------------------------- ;; Tests: objects-handler — cache headers ;; ----------------------------------------------------------------