diff --git a/client/packages/admin/src/index.ts b/client/packages/admin/src/index.ts index 63e8c5c7b2..d4ab71133e 100644 --- a/client/packages/admin/src/index.ts +++ b/client/packages/admin/src/index.ts @@ -845,6 +845,10 @@ const isNodeReadable = (v: any): v is Readable => const isWebReadable = (v: any): v is ReadableStream => v && typeof v.getReader === 'function'; +function isHeaderSafe(value: string): boolean { + return /^[\x20-\x7e\xa0-\xff]*$/.test(value); +} + /** * Functions to manage file storage. */ @@ -872,10 +876,14 @@ class Storage { ): Promise => { const headers = { ...authorizedHeaders(this.config, this.impersonationOpts), - path, }; + if (isHeaderSafe(path)) { + headers['path'] = path; + } if (metadata.contentDisposition) { - headers['content-disposition'] = metadata.contentDisposition; + if (isHeaderSafe(metadata.contentDisposition)) { + headers['content-disposition'] = metadata.contentDisposition; + } } // headers.content-type will become "undefined" (string) @@ -906,10 +914,12 @@ class Storage { ...(duplex && { duplex }), }; - return jsonFetch( - `${this.config.apiURI}/admin/storage/upload?app_id=${this.config.appId}`, - options, - ); + let url = `${this.config.apiURI}/admin/storage/upload?app_id=${encodeURIComponent(this.config.appId)}&path=${encodeURIComponent(path)}`; + if (metadata.contentDisposition) { + url += `&content-disposition=${encodeURIComponent(metadata.contentDisposition)}`; + } + + return jsonFetch(url, options); }; /** diff --git a/client/packages/components/src/components/explorer/inner-explorer.tsx b/client/packages/components/src/components/explorer/inner-explorer.tsx index 7eba2ed498..6d5dabf33f 100644 --- a/client/packages/components/src/components/explorer/inner-explorer.tsx +++ b/client/packages/components/src/components/explorer/inner-explorer.tsx @@ -1490,6 +1490,10 @@ export async function jsonFetch( : Promise.reject({ status: res.status, body: json }); } +function isHeaderSafe(value: string): boolean { + return /^[\x20-\x7e\xa0-\xff]*$/.test(value); +} + async function upload( token: string, appId: string, @@ -1497,15 +1501,20 @@ async function upload( customFilename: string, apiUri: string, ): Promise { - const headers = { + const path = customFilename || file.name; + const headers: Record = { 'app-id': appId, app_id: appId, - path: customFilename || file.name, authorization: `Bearer ${token}`, 'content-type': file.type, }; + if (isHeaderSafe(path)) { + headers['path'] = path; + } + + const url = `${apiUri}/dash/apps/${appId}/storage/upload?path=${encodeURIComponent(path)}`; - const data = await jsonFetch(`${apiUri}/dash/apps/${appId}/storage/upload`, { + const data = await jsonFetch(url, { method: 'PUT', headers, body: file, diff --git a/client/packages/core/src/StorageAPI.ts b/client/packages/core/src/StorageAPI.ts index f5197db748..da9628ba0c 100644 --- a/client/packages/core/src/StorageAPI.ts +++ b/client/packages/core/src/StorageAPI.ts @@ -1,5 +1,9 @@ import { jsonFetch } from './utils/fetch.js'; +function isHeaderSafe(value: string): boolean { + return /^[\x20-\x7e\xa0-\xff]*$/.test(value); +} + export type UploadFileResponse = { data: { id: string; @@ -32,15 +36,22 @@ export async function uploadFile({ const headers = { 'app-id': appId, app_id: appId, - path, authorization: `Bearer ${refreshToken}`, 'content-type': contentType || file.type, }; - if (contentDisposition) { + if (isHeaderSafe(path)) { + headers['path'] = path; + } + if (contentDisposition && isHeaderSafe(contentDisposition)) { headers['content-disposition'] = contentDisposition; } - const data = await jsonFetch(`${apiURI}/storage/upload`, { + let url = `${apiURI}/storage/upload?app_id=${encodeURIComponent(appId)}&path=${encodeURIComponent(path)}`; + if (contentDisposition) { + url += `&content-disposition=${encodeURIComponent(contentDisposition)}`; + } + + const data = await jsonFetch(url, { method: 'PUT', headers, body: file, diff --git a/client/www/app/docs/http-api/page.md b/client/www/app/docs/http-api/page.md index 653d68bbab..a613ba903a 100644 --- a/client/www/app/docs/http-api/page.md +++ b/client/www/app/docs/http-api/page.md @@ -323,10 +323,8 @@ You can also manage your app's [storage](/docs/storage) with the HTTP API. Upload a file with `PUT /admin/storage/upload`: ```shell -curl -X PUT "https://api.instantdb.com/admin/storage/upload" \ +curl -X PUT "https://api.instantdb.com/admin/storage/upload?app_id=$APP_ID&path=snippets/demo.txt" \ -H "Authorization: Bearer $ADMIN_TOKEN" \ - -H "App-Id: $APP_ID" \ - -H "Path: snippets/demo.txt" \ -H "Content-Type: text/plain" \ --data-binary "@demo.txt" ``` diff --git a/server/src/instant/admin/routes.clj b/server/src/instant/admin/routes.clj index 9ebf1da15f..b8eb7004ee 100644 --- a/server/src/instant/admin/routes.clj +++ b/server/src/instant/admin/routes.clj @@ -627,11 +627,12 @@ (defn upload-put [req] (let [{:keys [app-id] :as perms} (get-perms! req :storage/write) - params (:headers req) - path (ex/get-param! params ["path"] string-util/coerce-non-blank-str) + params (merge (w/keywordize-keys (:headers req)) + (:params req)) + path (ex/get-param! params [:path] string-util/coerce-non-blank-str) file (ex/get-param! req [:body] identity) - content-type (storage-coordinator/coerce-content-type (get params "content-type")) - content-disposition (ex/get-optional-param! params ["content-disposition"] string-util/coerce-non-blank-str) + content-type (storage-coordinator/coerce-content-type (:content-type params)) + content-disposition (ex/get-optional-param! params [:content-disposition] string-util/coerce-non-blank-str) data (storage-coordinator/upload-file! {:app-id app-id :path path :content-type content-type diff --git a/server/src/instant/dash/routes.clj b/server/src/instant/dash/routes.clj index a8cf75c7e0..1a3d357f9d 100644 --- a/server/src/instant/dash/routes.clj +++ b/server/src/instant/dash/routes.clj @@ -1764,8 +1764,9 @@ (let [{{app-id :id} :app} (req->app-accepting-superadmin-or-ref-token! :collaborator :apps/read req) - params (:headers req) - path (ex/get-param! params ["path"] string-util/coerce-non-blank-str) + params (merge (w/keywordize-keys (:headers req)) + (:params req)) + path (ex/get-param! params [:path] string-util/coerce-non-blank-str) file (ex/get-param! req [:body] identity) content-type (storage-coordinator/coerce-content-type (:content-type req)) data (storage-coordinator/upload-file! diff --git a/server/src/instant/storage/routes.clj b/server/src/instant/storage/routes.clj index d6bb684129..84d13e2ae4 100644 --- a/server/src/instant/storage/routes.clj +++ b/server/src/instant/storage/routes.clj @@ -27,7 +27,8 @@ :content-disposition (ex/get-optional-param! params [:content-disposition] string-util/coerce-non-blank-str)})) (defn upload-put [req] - (let [params (w/keywordize-keys (:headers req)) + (let [params (merge (w/keywordize-keys (:headers req)) + (:params req)) ctx (req->app-file! req params) file (ex/get-param! req [:body] identity) data (storage-coordinator/upload-file! ctx file)] diff --git a/server/test/instant/admin/routes_test.clj b/server/test/instant/admin/routes_test.clj index ec9676d4a0..ad0fa927fd 100644 --- a/server/test/instant/admin/routes_test.clj +++ b/server/test/instant/admin/routes_test.clj @@ -1088,6 +1088,17 @@ (is (= 200 (:status ret))) (is (some? (-> ret :body :data :id))))) + (testing "admin can upload a file with a non-ASCII filename via query params" + (let [ret (upload-put + {:body (make-file-content) + :params {:path "café à noite.txt"} + :headers {"app-id" app-id + "authorization" (str "Bearer " admin-token) + "content-type" "text/plain"} + :content-length 5})] + (is (= 200 (:status ret))) + (is (some? (-> ret :body :data :id))))) + (testing "user with email can upload" (let [ret (upload-put {:body (make-file-content) diff --git a/server/test/instant/storage/routes_test.clj b/server/test/instant/storage/routes_test.clj new file mode 100644 index 0000000000..07aa7a8cda --- /dev/null +++ b/server/test/instant/storage/routes_test.clj @@ -0,0 +1,44 @@ +(ns instant.storage.routes-test + (:require [clojure.test :as test :refer [deftest is testing]] + [instant.storage.routes :as storage-routes] + [instant.storage.coordinator :as storage-coordinator])) + +(deftest upload-put-reads-path-from-query-params + (let [captured-ctx (atom nil) + app-id (random-uuid)] + (with-redefs [storage-coordinator/upload-file! + (fn [ctx _file] + (reset! captured-ctx ctx) + {:id "fake-file-id"})] + (testing "path comes from query params when present" + (let [ret (storage-routes/upload-put + {:body "file-contents" + :params {:app_id (str app-id) + :path "café à noite.txt"} + :headers {"app-id" (str app-id) + "content-type" "text/plain"} + :content-length 5})] + (is (= 200 (:status ret))) + (is (= "café à noite.txt" (:path @captured-ctx))))) + + (testing "path still works from the legacy header for ASCII filenames" + (let [ret (storage-routes/upload-put + {:body "file-contents" + :params {} + :headers {"app-id" (str app-id) + "path" "legacy-file.txt" + "content-type" "text/plain"} + :content-length 5})] + (is (= 200 (:status ret))) + (is (= "legacy-file.txt" (:path @captured-ctx))))) + + (testing "query param takes priority over the header when both are present" + (let [ret (storage-routes/upload-put + {:body "file-contents" + :params {:path "café.txt"} + :headers {"app-id" (str app-id) + "path" "stale-ascii-name.txt" + "content-type" "text/plain"} + :content-length 5})] + (is (= 200 (:status ret))) + (is (= "café.txt" (:path @captured-ctx))))))))