From 11d2d9113b89a1db66f334f5206886e3762ede7e Mon Sep 17 00:00:00 2001 From: aRustyDev <36318507+aRustyDev@users.noreply.github.com> Date: Tue, 4 Aug 2026 20:42:50 -0400 Subject: [PATCH] fix(storage): support presigned PUT uploads for object stores without POST MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Browser uploads cannot land on Cloudflare R2. R2 does not implement presigned POST and answers one with HTTP 501 NotImplemented Presigned post requests are not yet implemented while presigned GET works fine — so on R2 downloads succeed, uploads silently never arrive, and the asset row is left with is_uploaded=false. Reproduced against a live R2 bucket; a presigned PUT with the same credential, bucket, Origin and byte count returns 200 and the object appears, which isolates the failure to the verb rather than to credentials, CORS or object size. Adds a presigned PUT flavour selected by AWS_S3_UPLOAD_METHOD, and a generate_presigned_upload() dispatcher that the nine asset views now call. generate_presigned_post() is untouched. The default stays "post". S3 and MinIO both implement presigned POST and every existing deployment already relies on it, so switching the default would break working installs to fix stores that are not in use. An unrecognised value falls back to "post" rather than disabling uploads. The PUT flavour is STRICTER than the POST policy it replaces, not a relaxation. Content-Type and Content-Length are signed into SignedHeaders, so the store rejects a mismatch with 403 SignatureDoesNotMatch — verified with both controls against live R2. content-length-range permitted anything in [1, file_size] whereas a signed Content-Length pins the size exactly, and the key is part of the signed URL rather than a form field the client supplies. `${filename}` templating is POST-only and cannot be expressed as a PUT; no caller uses it, so it raises rather than uploading to a key containing the literal string. Clients dispatch on the returned `method`, so an install can move between S3 or MinIO and R2 with no client-side change. `fields` stays present-but-empty on PUT to keep the response shape stable, and `method` is optional on the type because older servers omit it and are always POST. Content-Length is dropped from the headers the browser sends: it is set from the body and cannot be set from script, and the body is the exact file that was signed for. Also switches axios.isCancel to the named import in the two upload services. That is not gratuitous: lint-staged runs oxlint with --deny-warnings, and these files carried pre-existing no-named-as-default-member warnings that block any commit touching them. The sibling CancelToken suggestion is a false positive — axios v1 exports it as a type only, so the named import fails to compile with TS2693 — and is silenced inline with that reason recorded. Refs: plane-7fn Co-Authored-By: Claude Opus 5 --- apps/api/plane/api/views/asset.py | 6 +- apps/api/plane/api/views/issue.py | 2 +- apps/api/plane/app/views/asset/v2.py | 6 +- apps/api/plane/app/views/issue/attachment.py | 2 +- apps/api/plane/settings/storage.py | 71 +++++++++++ apps/api/plane/space/views/asset.py | 2 +- .../plane/tests/unit/settings/test_storage.py | 115 ++++++++++++++++++ apps/web/core/services/file-upload.service.ts | 30 +++-- apps/web/core/services/file.service.ts | 10 +- .../issue/issue_attachment.service.ts | 3 +- .../services/src/file/file-upload.service.ts | 30 +++-- packages/services/src/file/helper.ts | 35 +++++- .../services/src/file/sites-file.service.ts | 13 +- packages/types/src/file.ts | 22 +++- 14 files changed, 310 insertions(+), 37 deletions(-) diff --git a/apps/api/plane/api/views/asset.py b/apps/api/plane/api/views/asset.py index 72ce60e6819..c293fe8a309 100644 --- a/apps/api/plane/api/views/asset.py +++ b/apps/api/plane/api/views/asset.py @@ -163,7 +163,7 @@ def post(self, request): # Get the presigned URL storage = S3Storage(request=request) # Generate a presigned URL to share an S3 object - presigned_url = storage.generate_presigned_post(object_name=asset_key, file_type=type, file_size=size_limit) + presigned_url = storage.generate_presigned_upload(object_name=asset_key, file_type=type, file_size=size_limit) # Return the presigned URL return Response( { @@ -336,7 +336,7 @@ def post(self, request): # Get the presigned URL storage = S3Storage(request=request, is_server=True) # Generate a presigned URL to share an S3 object - presigned_url = storage.generate_presigned_post(object_name=asset_key, file_type=type, file_size=size_limit) + presigned_url = storage.generate_presigned_upload(object_name=asset_key, file_type=type, file_size=size_limit) # Return the presigned URL return Response( { @@ -563,7 +563,7 @@ def post(self, request, slug): # Get the presigned URL storage = S3Storage(request=request, is_server=True) - presigned_url = storage.generate_presigned_post(object_name=asset_key, file_type=type, file_size=size_limit) + presigned_url = storage.generate_presigned_upload(object_name=asset_key, file_type=type, file_size=size_limit) return Response( { diff --git a/apps/api/plane/api/views/issue.py b/apps/api/plane/api/views/issue.py index b48be56d413..8d9638ae5cf 100644 --- a/apps/api/plane/api/views/issue.py +++ b/apps/api/plane/api/views/issue.py @@ -1931,7 +1931,7 @@ def post(self, request, slug, project_id, issue_id): # Get the presigned URL storage = S3Storage(request=request) # Generate a presigned URL to share an S3 object - presigned_url = storage.generate_presigned_post(object_name=asset_key, file_type=type, file_size=size_limit) + presigned_url = storage.generate_presigned_upload(object_name=asset_key, file_type=type, file_size=size_limit) # Return the presigned URL return Response( { diff --git a/apps/api/plane/app/views/asset/v2.py b/apps/api/plane/app/views/asset/v2.py index b21f70d61fc..11bd635884f 100644 --- a/apps/api/plane/app/views/asset/v2.py +++ b/apps/api/plane/app/views/asset/v2.py @@ -157,7 +157,7 @@ def post(self, request): # Get the presigned URL storage = S3Storage(request=request) # Generate a presigned URL to share an S3 object - presigned_url = storage.generate_presigned_post(object_name=asset_key, file_type=type, file_size=size_limit) + presigned_url = storage.generate_presigned_upload(object_name=asset_key, file_type=type, file_size=size_limit) # Return the presigned URL return Response( { @@ -367,7 +367,7 @@ def post(self, request, slug): # Get the presigned URL storage = S3Storage(request=request) # Generate a presigned URL to share an S3 object - presigned_url = storage.generate_presigned_post(object_name=asset_key, file_type=type, file_size=size_limit) + presigned_url = storage.generate_presigned_upload(object_name=asset_key, file_type=type, file_size=size_limit) # Return the presigned URL return Response( { @@ -570,7 +570,7 @@ def post(self, request, slug, project_id): # Get the presigned URL storage = S3Storage(request=request) # Generate a presigned URL to share an S3 object - presigned_url = storage.generate_presigned_post(object_name=asset_key, file_type=type, file_size=size_limit) + presigned_url = storage.generate_presigned_upload(object_name=asset_key, file_type=type, file_size=size_limit) # Return the presigned URL return Response( { diff --git a/apps/api/plane/app/views/issue/attachment.py b/apps/api/plane/app/views/issue/attachment.py index 51248b8a428..6e165398c14 100644 --- a/apps/api/plane/app/views/issue/attachment.py +++ b/apps/api/plane/app/views/issue/attachment.py @@ -133,7 +133,7 @@ def post(self, request, slug, project_id, issue_id): storage = S3Storage(request=request) # Generate a presigned URL to share an S3 object - presigned_url = storage.generate_presigned_post(object_name=asset_key, file_type=type, file_size=size_limit) + presigned_url = storage.generate_presigned_upload(object_name=asset_key, file_type=type, file_size=size_limit) # Return the presigned URL return Response( diff --git a/apps/api/plane/settings/storage.py b/apps/api/plane/settings/storage.py index e4a978bd2b1..29f18398120 100644 --- a/apps/api/plane/settings/storage.py +++ b/apps/api/plane/settings/storage.py @@ -35,6 +35,16 @@ def __init__(self, request=None): self.aws_s3_endpoint_url = os.environ.get("AWS_S3_ENDPOINT_URL") or os.environ.get("MINIO_ENDPOINT_URL") # Use the SIGNED_URL_EXPIRATION environment variable for the expiration time (default: 3600 seconds) self.signed_url_expiration = int(os.environ.get("SIGNED_URL_EXPIRATION", "3600")) + # Which presigned upload flavour the browser should use: "post" (default) or "put". + # + # Default stays "post" because that is what AWS S3 and MinIO implement and what every + # existing deployment already uses. Set it to "put" for object stores that do NOT + # implement presigned POST — notably Cloudflare R2, which answers a presigned POST with + # `501 NotImplemented: Presigned post requests are not yet implemented`, so browser + # uploads can never land while server-side flows keep working. + self.upload_method = os.environ.get("AWS_S3_UPLOAD_METHOD", "post").strip().lower() + if self.upload_method not in ("post", "put"): + self.upload_method = "post" if os.environ.get("USE_MINIO") == "1": # Determine protocol based on environment variable @@ -96,8 +106,69 @@ def generate_presigned_post(self, object_name, file_type, file_size, expiration= print(f"Error generating presigned POST URL: {e}") return None + response["method"] = "POST" return response + def generate_presigned_put(self, object_name, file_type, file_size, expiration=None): + """Generate a presigned PUT URL to upload an S3 object. + + For object stores without presigned POST support (Cloudflare R2). Returns the same + envelope as generate_presigned_post so callers and the client stay uniform: + `url` plus an empty `fields`, with the headers the client must send in `headers`. + + The constraints the POST policy expressed as `conditions` are preserved by SIGNING them + as headers — `Content-Type` and `Content-Length` land in SignedHeaders, so the store + rejects any mismatch with 403 SignatureDoesNotMatch. This is STRICTER than the POST + policy it replaces: `content-length-range` allowed anything in [1, file_size], whereas a + signed Content-Length pins the size exactly, and the key is part of the signed URL rather + than a forgeable form field. + """ + if expiration is None: + expiration = self.signed_url_expiration + + # A presigned PUT addresses one concrete key, so the POST-only `${filename}` template + # cannot be expressed. No caller uses it, but fail loudly rather than silently upload to + # a key with a literal "${filename}" in it. + if object_name.startswith("${filename}"): + raise ValueError("generate_presigned_put requires a concrete object key; '${filename}' is POST-only") + + try: + url = self.s3_client.generate_presigned_url( + "put_object", + Params={ + "Bucket": self.aws_storage_bucket_name, + "Key": object_name, + "ContentType": file_type, + "ContentLength": file_size, + }, + ExpiresIn=expiration, + ) + except ClientError as e: + print(f"Error generating presigned PUT URL: {e}") + return None + + return { + "method": "PUT", + "url": url, + # Kept (empty) so the response shape is stable for clients that read `fields`. + "fields": {}, + "headers": {"Content-Type": file_type, "Content-Length": str(file_size)}, + } + + def generate_presigned_upload(self, object_name, file_type, file_size, expiration=None): + """Generate a presigned browser upload, POST or PUT per AWS_S3_UPLOAD_METHOD. + + This is what the asset views call. The client dispatches on the returned `method`, so + switching an install between S3/MinIO and R2 needs no client-side change. + """ + if self.upload_method == "put": + return self.generate_presigned_put( + object_name=object_name, file_type=file_type, file_size=file_size, expiration=expiration + ) + return self.generate_presigned_post( + object_name=object_name, file_type=file_type, file_size=file_size, expiration=expiration + ) + def _get_content_disposition(self, disposition, filename=None): """Helper method to generate Content-Disposition header value""" if filename is None: diff --git a/apps/api/plane/space/views/asset.py b/apps/api/plane/space/views/asset.py index bc20724ca80..eff82ad835d 100644 --- a/apps/api/plane/space/views/asset.py +++ b/apps/api/plane/space/views/asset.py @@ -122,7 +122,7 @@ def post(self, request, anchor): # Get the presigned URL storage = S3Storage(request=request) # Generate a presigned URL to share an S3 object - presigned_url = storage.generate_presigned_post(object_name=asset_key, file_type=type, file_size=size) + presigned_url = storage.generate_presigned_upload(object_name=asset_key, file_type=type, file_size=size) # Return the presigned URL return Response( { diff --git a/apps/api/plane/tests/unit/settings/test_storage.py b/apps/api/plane/tests/unit/settings/test_storage.py index 00856aeecb6..422c9ed4de8 100644 --- a/apps/api/plane/tests/unit/settings/test_storage.py +++ b/apps/api/plane/tests/unit/settings/test_storage.py @@ -204,3 +204,118 @@ def test_explicit_expiration_overrides_default(self, mock_boto3): mock_s3_client.generate_presigned_url.assert_called_once() call_kwargs = mock_s3_client.generate_presigned_url.call_args[1] assert call_kwargs["ExpiresIn"] == 120 + + +S3_ENV = { + "AWS_ACCESS_KEY_ID": "test-key", + "AWS_SECRET_ACCESS_KEY": "test-secret", + "AWS_S3_BUCKET_NAME": "test-bucket", + "AWS_REGION": "us-east-1", +} + + +@pytest.mark.unit +class TestS3StorageUploadMethod: + """Test AWS_S3_UPLOAD_METHOD dispatch and the presigned PUT flavour. + + Presigned PUT exists for object stores that do not implement presigned POST — Cloudflare R2 + answers a presigned POST with 501 NotImplemented, so browser uploads can never land there + while server-side flows keep working. + """ + + @patch.dict(os.environ, S3_ENV, clear=True) + @patch("plane.settings.storage.boto3") + def test_upload_method_defaults_to_post(self, mock_boto3): + """Default must stay POST — that is what S3 and MinIO implement""" + mock_boto3.client.return_value = Mock() + assert S3Storage().upload_method == "post" + + @patch.dict(os.environ, {**S3_ENV, "AWS_S3_UPLOAD_METHOD": "PUT"}, clear=True) + @patch("plane.settings.storage.boto3") + def test_upload_method_is_case_insensitive(self, mock_boto3): + mock_boto3.client.return_value = Mock() + assert S3Storage().upload_method == "put" + + @patch.dict(os.environ, {**S3_ENV, "AWS_S3_UPLOAD_METHOD": "sftp"}, clear=True) + @patch("plane.settings.storage.boto3") + def test_unknown_upload_method_falls_back_to_post(self, mock_boto3): + """An unrecognised value must not silently disable uploads""" + mock_boto3.client.return_value = Mock() + assert S3Storage().upload_method == "post" + + @patch.dict(os.environ, S3_ENV, clear=True) + @patch("plane.settings.storage.boto3") + def test_generate_presigned_upload_dispatches_to_post_by_default(self, mock_boto3): + mock_s3_client = Mock() + mock_s3_client.generate_presigned_post.return_value = {"url": "https://s3", "fields": {}} + mock_boto3.client.return_value = mock_s3_client + + response = S3Storage().generate_presigned_upload("test-object", "image/png", 1024) + + mock_s3_client.generate_presigned_post.assert_called_once() + mock_s3_client.generate_presigned_url.assert_not_called() + assert response["method"] == "POST" + + @patch.dict(os.environ, {**S3_ENV, "AWS_S3_UPLOAD_METHOD": "put"}, clear=True) + @patch("plane.settings.storage.boto3") + def test_generate_presigned_upload_dispatches_to_put(self, mock_boto3): + mock_s3_client = Mock() + mock_s3_client.generate_presigned_url.return_value = "https://r2/test-object?sig" + mock_boto3.client.return_value = mock_s3_client + + response = S3Storage().generate_presigned_upload("test-object", "image/png", 1024) + + mock_s3_client.generate_presigned_post.assert_not_called() + assert response["method"] == "PUT" + assert response["url"] == "https://r2/test-object?sig" + # `fields` stays present-but-empty so the response shape is stable for clients + assert response["fields"] == {} + + @patch.dict(os.environ, {**S3_ENV, "AWS_S3_UPLOAD_METHOD": "put"}, clear=True) + @patch("plane.settings.storage.boto3") + def test_presigned_put_signs_content_type_and_length(self, mock_boto3): + """Content-Type and Content-Length must be SIGNED. + + This is what replaces the POST policy's conditions: both land in SignedHeaders, so the + store rejects a mismatch with 403 rather than accepting a differently-sized or + differently-typed object. It is stricter than content-length-range, which permitted + anything in [1, file_size]. + """ + mock_s3_client = Mock() + mock_s3_client.generate_presigned_url.return_value = "https://r2/test-object?sig" + mock_boto3.client.return_value = mock_s3_client + + response = S3Storage().generate_presigned_put("test-object", "image/png", 1024) + + call_kwargs = mock_s3_client.generate_presigned_url.call_args[1] + assert mock_s3_client.generate_presigned_url.call_args[0][0] == "put_object" + assert call_kwargs["Params"]["ContentType"] == "image/png" + assert call_kwargs["Params"]["ContentLength"] == 1024 + assert call_kwargs["Params"]["Key"] == "test-object" + assert call_kwargs["Params"]["Bucket"] == "test-bucket" + assert response["headers"]["Content-Type"] == "image/png" + assert response["headers"]["Content-Length"] == "1024" + + @patch.dict(os.environ, {**S3_ENV, "AWS_S3_UPLOAD_METHOD": "put"}, clear=True) + @patch("plane.settings.storage.boto3") + def test_presigned_put_uses_default_expiration(self, mock_boto3): + mock_s3_client = Mock() + mock_s3_client.generate_presigned_url.return_value = "https://r2/test-object?sig" + mock_boto3.client.return_value = mock_s3_client + + S3Storage().generate_presigned_put("test-object", "image/png", 1024) + + assert mock_s3_client.generate_presigned_url.call_args[1]["ExpiresIn"] == 3600 + + @patch.dict(os.environ, {**S3_ENV, "AWS_S3_UPLOAD_METHOD": "put"}, clear=True) + @patch("plane.settings.storage.boto3") + def test_presigned_put_rejects_filename_template(self, mock_boto3): + """`${filename}` is a POST-policy feature and cannot be expressed as a PUT. + + No caller uses it, but failing loudly beats uploading to a key containing the literal + string "${filename}". + """ + mock_boto3.client.return_value = Mock() + + with pytest.raises(ValueError): + S3Storage().generate_presigned_put("${filename}", "image/png", 1024) diff --git a/apps/web/core/services/file-upload.service.ts b/apps/web/core/services/file-upload.service.ts index f8f49396799..07a9ade3230 100644 --- a/apps/web/core/services/file-upload.service.ts +++ b/apps/web/core/services/file-upload.service.ts @@ -5,7 +5,9 @@ */ import type { AxiosRequestConfig } from "axios"; -import axios from "axios"; +import axios, { isCancel } from "axios"; +// plane imports +import type { TFileUploadRequestOptions } from "@plane/types"; // services import { APIService } from "@/services/api.service"; @@ -16,23 +18,35 @@ export class FileUploadService extends APIService { super(""); } + /** + * Uploads a file to the specified signed URL. + * + * POST sends the multipart form built from the policy fields; PUT sends the raw file with the + * headers that were signed. Pass `requestOptions` from `getFileUploadRequestOptions` — for PUT + * the headers are part of the signature, so sending different ones fails with 403. + */ async uploadFile( url: string, - data: FormData, + data: FormData | File, + requestOptions?: TFileUploadRequestOptions, uploadProgressHandler?: AxiosRequestConfig["onUploadProgress"] ): Promise { + // axios v1 exports CancelToken as a TYPE only; the runtime value lives on the default + // export, so this rule's named-import suggestion does not compile (TS2693). + // oxlint-disable-next-line import/no-named-as-default-member this.cancelSource = axios.CancelToken.source(); - return this.post(url, data, { - headers: { - "Content-Type": "multipart/form-data", - }, + const { method = "POST", headers = { "Content-Type": "multipart/form-data" } } = requestOptions ?? {}; + const config: AxiosRequestConfig = { + headers, cancelToken: this.cancelSource.token, withCredentials: false, onUploadProgress: uploadProgressHandler, - }) + }; + const request = method === "PUT" ? this.put(url, data, config) : this.post(url, data, config); + return request .then((response) => response?.data) .catch((error) => { - if (axios.isCancel(error)) { + if (isCancel(error)) { console.log(error.message); } else { throw error?.response?.data; diff --git a/apps/web/core/services/file.service.ts b/apps/web/core/services/file.service.ts index 6e252d76af0..af9bd15dca5 100644 --- a/apps/web/core/services/file.service.ts +++ b/apps/web/core/services/file.service.ts @@ -7,7 +7,7 @@ import type { AxiosRequestConfig } from "axios"; // plane types import { API_BASE_URL } from "@plane/constants"; -import { getFileMetaDataForUpload, generateFileUploadPayload } from "@plane/services"; +import { getFileMetaDataForUpload, generateFileUploadPayload, getFileUploadRequestOptions } from "@plane/services"; import type { EFileAssetType, TFileEntityInfo, TFileSignedURLResponse } from "@plane/types"; import { getAssetIdFromUrl } from "@plane/utils"; // helpers @@ -86,6 +86,7 @@ export class FileService extends APIService { await this.fileUploadService.uploadFile( signedURLResponse.upload_data.url, fileUploadPayload, + getFileUploadRequestOptions(signedURLResponse), uploadProgressHandler ); await this.updateWorkspaceAssetUploadStatus(workspaceSlug.toString(), signedURLResponse.asset_id); @@ -163,6 +164,7 @@ export class FileService extends APIService { await this.fileUploadService.uploadFile( signedURLResponse.upload_data.url, fileUploadPayload, + getFileUploadRequestOptions(signedURLResponse), uploadProgressHandler ); await this.updateProjectAssetUploadStatus(workspaceSlug, projectId, signedURLResponse.asset_id); @@ -190,7 +192,11 @@ export class FileService extends APIService { .then(async (response) => { const signedURLResponse: TFileSignedURLResponse = response?.data; const fileUploadPayload = generateFileUploadPayload(signedURLResponse, file); - await this.fileUploadService.uploadFile(signedURLResponse.upload_data.url, fileUploadPayload); + await this.fileUploadService.uploadFile( + signedURLResponse.upload_data.url, + fileUploadPayload, + getFileUploadRequestOptions(signedURLResponse) + ); await this.updateUserAssetUploadStatus(signedURLResponse.asset_id); return signedURLResponse; }) diff --git a/apps/web/core/services/issue/issue_attachment.service.ts b/apps/web/core/services/issue/issue_attachment.service.ts index 98660ae6c0b..cb653b53fbb 100644 --- a/apps/web/core/services/issue/issue_attachment.service.ts +++ b/apps/web/core/services/issue/issue_attachment.service.ts @@ -7,7 +7,7 @@ import type { AxiosRequestConfig } from "axios"; import { API_BASE_URL } from "@plane/constants"; // plane types -import { getFileMetaDataForUpload, generateFileUploadPayload } from "@plane/services"; +import { getFileMetaDataForUpload, generateFileUploadPayload, getFileUploadRequestOptions } from "@plane/services"; import type { TIssueAttachment, TIssueAttachmentUploadResponse, TIssueServiceType } from "@plane/types"; import { EIssueServiceType } from "@plane/types"; // services @@ -58,6 +58,7 @@ export class IssueAttachmentService extends APIService { await this.fileUploadService.uploadFile( signedURLResponse.upload_data.url, fileUploadPayload, + getFileUploadRequestOptions(signedURLResponse), uploadProgressHandler ); await this.updateIssueAttachmentUploadStatus(workspaceSlug, projectId, issueId, signedURLResponse.asset_id); diff --git a/packages/services/src/file/file-upload.service.ts b/packages/services/src/file/file-upload.service.ts index 32ca4de51f4..9646e739147 100644 --- a/packages/services/src/file/file-upload.service.ts +++ b/packages/services/src/file/file-upload.service.ts @@ -4,7 +4,9 @@ * See the LICENSE file for details. */ -import axios from "axios"; +import axios, { isCancel } from "axios"; +// plane imports +import type { TFileUploadRequestOptions } from "@plane/types"; // api service import { APIService } from "../api.service"; @@ -22,23 +24,33 @@ export class FileUploadService extends APIService { /** * Uploads a file to the specified signed URL + * + * POST sends the multipart form built from the policy fields; PUT sends the raw file with the + * headers that were signed. Pass `requestOptions` from `getFileUploadRequestOptions` — for PUT + * the headers are part of the signature, so sending different ones fails with 403. + * * @param {string} url - The URL to upload the file to - * @param {FormData} data - The form data to upload + * @param {FormData | File} data - The form data (POST) or raw file (PUT) to upload + * @param {TFileUploadRequestOptions} requestOptions - method and signed headers * @returns {Promise} Promise resolving to void * @throws {Error} If the request fails */ - async uploadFile(url: string, data: FormData): Promise { + async uploadFile(url: string, data: FormData | File, requestOptions?: TFileUploadRequestOptions): Promise { + // axios v1 exports CancelToken as a TYPE only; the runtime value lives on the default + // export, so this rule's named-import suggestion does not compile (TS2693). + // oxlint-disable-next-line import/no-named-as-default-member this.cancelSource = axios.CancelToken.source(); - return this.post(url, data, { - headers: { - "Content-Type": "multipart/form-data", - }, + const { method = "POST", headers = { "Content-Type": "multipart/form-data" } } = requestOptions ?? {}; + const config = { + headers, cancelToken: this.cancelSource.token, withCredentials: false, - }) + }; + const request = method === "PUT" ? this.put(url, data, config) : this.post(url, data, config); + return request .then((response) => response?.data) .catch((error) => { - if (axios.isCancel(error)) { + if (isCancel(error)) { console.log(error.message); } else { throw error?.response?.data; diff --git a/packages/services/src/file/helper.ts b/packages/services/src/file/helper.ts index b8e96283986..75e7ea79dbb 100644 --- a/packages/services/src/file/helper.ts +++ b/packages/services/src/file/helper.ts @@ -7,7 +7,7 @@ // external imports import { fileTypeFromBuffer } from "file-type"; // plane imports -import type { TFileMetaDataLite, TFileSignedURLResponse } from "@plane/types"; +import type { TFileMetaDataLite, TFileSignedURLResponse, TFileUploadRequestOptions } from "@plane/types"; import { DANGEROUS_EXTENSIONS } from "@plane/constants"; /** @@ -51,17 +51,44 @@ const validateFilename = (filename: string): string | null => { /** * @description from the provided signed URL response, generate a payload to be used to upload the file + * + * POST (S3, MinIO) wants a multipart form carrying the policy fields; PUT (R2, which has no + * presigned POST) wants the raw file as the body, with the signed headers supplied separately by + * {@link getFileUploadRequestOptions}. `method` is absent on older servers, which are always POST. + * * @param {TFileSignedURLResponse} signedURLResponse * @param {File} file - * @returns {FormData} file upload request payload + * @returns {FormData | File} file upload request payload */ -export const generateFileUploadPayload = (signedURLResponse: TFileSignedURLResponse, file: File): FormData => { +export const generateFileUploadPayload = (signedURLResponse: TFileSignedURLResponse, file: File): FormData | File => { + if (signedURLResponse.upload_data.method === "PUT") return file; const formData = new FormData(); - Object.entries(signedURLResponse.upload_data.fields).forEach(([key, value]) => formData.append(key, value)); + Object.entries(signedURLResponse.upload_data.fields ?? {}).forEach(([key, value]) => + formData.append(key, value as string) + ); formData.append("file", file); return formData; }; +/** + * @description the method and headers the upload request must use for this signed URL + * + * For PUT the headers were SIGNED (Content-Type and Content-Length are in SignedHeaders), so they + * must be sent verbatim — a mismatch is rejected with 403 SignatureDoesNotMatch rather than being + * silently coerced. Content-Length is omitted deliberately: browsers set it from the body and + * forbid setting it from script, and the body is the exact file that was signed for. + * + * @param {TFileSignedURLResponse} signedURLResponse + * @returns {TFileUploadRequestOptions} + */ +export const getFileUploadRequestOptions = (signedURLResponse: TFileSignedURLResponse): TFileUploadRequestOptions => { + if (signedURLResponse.upload_data.method === "PUT") { + const { "Content-Length": _contentLength, ...sendable } = signedURLResponse.upload_data.headers ?? {}; + return { method: "PUT", headers: sendable }; + } + return { method: "POST", headers: { "Content-Type": "multipart/form-data" } }; +}; + /** * @description Detect MIME type from file signature using file-type library * @param {File} file diff --git a/packages/services/src/file/sites-file.service.ts b/packages/services/src/file/sites-file.service.ts index debe945f8d9..2a88f8beff6 100644 --- a/packages/services/src/file/sites-file.service.ts +++ b/packages/services/src/file/sites-file.service.ts @@ -11,7 +11,12 @@ import type { TFileEntityInfo, TFileSignedURLResponse } from "@plane/types"; import { FileUploadService } from "./file-upload.service"; // helpers import { FileService } from "./file.service"; -import { generateFileUploadPayload, getAssetIdFromUrl, getFileMetaDataForUpload } from "./helper"; +import { + generateFileUploadPayload, + getAssetIdFromUrl, + getFileMetaDataForUpload, + getFileUploadRequestOptions, +} from "./helper"; /** * Service class for managing file operations within plane sites application. @@ -88,7 +93,11 @@ export class SitesFileService extends FileService { .then(async (response) => { const signedURLResponse: TFileSignedURLResponse = response?.data; const fileUploadPayload = generateFileUploadPayload(signedURLResponse, file); - await this.fileUploadService.uploadFile(signedURLResponse.upload_data.url, fileUploadPayload); + await this.fileUploadService.uploadFile( + signedURLResponse.upload_data.url, + fileUploadPayload, + getFileUploadRequestOptions(signedURLResponse) + ); await this.updateAssetUploadStatus(anchor, signedURLResponse.asset_id); return signedURLResponse; }) diff --git a/packages/types/src/file.ts b/packages/types/src/file.ts index 01a189259ed..55ac894e7cc 100644 --- a/packages/types/src/file.ts +++ b/packages/types/src/file.ts @@ -20,12 +20,28 @@ export type TFileEntityInfo = { export type TFileMetaData = TFileMetaDataLite & TFileEntityInfo; +/** + * Which presigned upload flavour the server handed back. + * "POST" is the default (AWS S3, MinIO). "PUT" is for stores without presigned POST support — + * notably Cloudflare R2, which rejects a presigned POST with 501 NotImplemented. + */ +export type TFileUploadMethod = "POST" | "PUT"; + +/** Method and headers the upload request must use for a given signed URL. */ +export type TFileUploadRequestOptions = { + method: TFileUploadMethod; + headers: Record; +}; + export type TFileSignedURLResponse = { asset_id: string; asset_url: string; upload_data: { + /** Absent on older servers, which are always POST. */ + method?: TFileUploadMethod; url: string; - fields: { + /** Populated for POST; empty for PUT, where the key is signed into the URL. */ + fields: Partial<{ "Content-Type": string; key: string; "x-amz-algorithm": string; @@ -33,7 +49,9 @@ export type TFileSignedURLResponse = { "x-amz-date": string; policy: string; "x-amz-signature": string; - }; + }>; + /** PUT only: headers that were SIGNED and must be sent verbatim or the store returns 403. */ + headers?: Record; }; };