mirror/formbricks-formbricks
1
0
Fork 0
mirror of https://github.com/formbricks/formbricks.git synced 2026-05-08 06:41:45 -05:00
Code Issues Packages Projects Releases 100 Wiki Activity

fix: coderabbit feedback

Browse Source
This commit is contained in:
pandeymangg
2025-09-01 13:46:55 +05:30
parent 8f7d5f8fd5
commit 941098a3fa
16 changed files with 178 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/web/modules/storage/service.test.ts
+39 -3
View File
@@ -30,9 +30,11 @@ vi.mock("@formbricks/logger", () => ({
vi.mock("@formbricks/storage", () => ({
StorageErrorCode: {
Unknown: "UNKNOWN",
FileNotFound: "FILE_NOT_FOUND",
FileSizeExceeded: "FILE_SIZE_EXCEEDED",
Unknown: "unknown",
S3ClientError: "s3_client_error",
S3CredentialsError: "s3_credentials_error",
FileNotFoundError: "file_not_found_error",
InvalidInput: "invalid_input",
},
deleteFile: vi.fn(),
deleteFilesByPrefix: vi.fn(),
@@ -126,6 +128,40 @@ describe("storage service", () => {
}
});
test("should properly encode filenames with special characters like # in URL", async () => {
const mockSignedUrlResponse = {
ok: true,
data: {
signedUrl: "https://s3.example.com/upload",
presignedFields: { key: "value" },
},
} as MockedSignedUploadReturn;
vi.mocked(getSignedUploadUrl).mockResolvedValue(mockSignedUrlResponse);
const result = await getSignedUrlForUpload(
"test#file.txt",
"env-123",
"text/plain",
"public" as TAccessType
);
expect(result.ok).toBe(true);
if (result.ok) {
// The filename should be URL-encoded to prevent # from being treated as a URL fragment
expect(result.data.fileUrl).toBe(
`https://public.example.com/storage/env-123/public/test%23file--fid--${mockUUID}.txt`
);
}
expect(getSignedUploadUrl).toHaveBeenCalledWith(
`test#file--fid--${mockUUID}.txt`,
"text/plain",
"env-123/public",
1024 * 1024 * 10 // 10MB default
);
});
test("should handle files with multiple dots in filename", async () => {
const mockSignedUrlResponse = {
ok: true,
apps/web/modules/storage/service.ts
+3 -1
View File
@@ -52,7 +52,9 @@ export const getSignedUrlForUpload = async (
return ok({
signedUrl: signedUrlResult.data.signedUrl,
presignedFields: signedUrlResult.data.presignedFields,
fileUrl: new URL(`${baseUrl}/storage/${environmentId}/${accessType}/${updatedFileName}`).href,
fileUrl: new URL(
`${baseUrl}/storage/${environmentId}/${accessType}/${encodeURIComponent(updatedFileName)}`
).href,
});
} catch (error) {
logger.error({ error }, "Error getting signed url for upload");
apps/web/modules/storage/utils.test.ts
+16 -7
View File
@@ -23,7 +23,16 @@ vi.mock("@/modules/storage/utils", async () => {
describe("storage utils", () => {
beforeEach(() => {
vi.clearAllMocks();
vi.resetAllMocks();
// Default: derive filename from URL path for positive-path tests
mockGetOriginalFileNameFromUrl.mockImplementation((url: string) => {
try {
return new URL(url).pathname.split("/").filter(Boolean).pop();
} catch {
return undefined;
}
});
});
describe("isAllowedFileExtension", () => {
@@ -167,7 +176,7 @@ describe("storage utils", () => {
test("should return false when file name cannot be extracted", () => {
// Mock implementation to return null for this specific URL
mockGetOriginalFileNameFromUrl.mockImplementation(() => undefined);
mockGetOriginalFileNameFromUrl.mockImplementationOnce(() => undefined);
const responseData = {
question1: ["https://example.com/invalid-url"],
@@ -185,7 +194,7 @@ describe("storage utils", () => {
});
test("should return false when file has no extension", () => {
mockGetOriginalFileNameFromUrl.mockImplementation(() => "file-without-extension");
mockGetOriginalFileNameFromUrl.mockImplementationOnce(() => "file-without-extension");
const responseData = {
question1: ["https://example.com/storage/file-without-extension"],
@@ -248,22 +257,22 @@ describe("storage utils", () => {
});
test("should return false when file name cannot be extracted", () => {
mockGetOriginalFileNameFromUrl.mockImplementation(() => undefined);
mockGetOriginalFileNameFromUrl.mockImplementationOnce(() => undefined);
expect(isValidImageFile("https://example.com/invalid-url")).toBe(false);
});
test("should return false when file has no extension", () => {
mockGetOriginalFileNameFromUrl.mockImplementation(() => "image-without-extension");
mockGetOriginalFileNameFromUrl.mockImplementationOnce(() => "image-without-extension");
expect(isValidImageFile("https://example.com/image-without-extension")).toBe(false);
});
test("should return false when file name ends with a dot", () => {
mockGetOriginalFileNameFromUrl.mockImplementation(() => "image.");
mockGetOriginalFileNameFromUrl.mockImplementationOnce(() => "image.");
expect(isValidImageFile("https://example.com/image.")).toBe(false);
});
test("should handle case insensitivity correctly", () => {
mockGetOriginalFileNameFromUrl.mockImplementation(() => "image.JPG");
mockGetOriginalFileNameFromUrl.mockImplementationOnce(() => "image.JPG");
expect(isValidImageFile("https://example.com/image.JPG")).toBe(true);
});
});
apps/web/modules/storage/utils.ts
+35 -30
View File
@@ -1,39 +1,28 @@
import { responses } from "@/app/lib/api/response";
import { logger } from "@formbricks/logger";
import { StorageError, StorageErrorCode } from "@formbricks/storage";
import { TResponseData } from "@formbricks/types/responses";
import { TAllowedFileExtension, ZAllowedFileExtension, mimeTypes } from "@formbricks/types/storage";
import { TSurveyQuestion, TSurveyQuestionTypeEnum } from "@formbricks/types/surveys/types";
export const getOriginalFileNameFromUrl = (fileURL: string) => {
try {
const fileNameFromURL = fileURL.startsWith("/storage/")
? fileURL.split("/").pop()
: new URL(fileURL).pathname.split("/").pop();
const lastSegment = fileURL.startsWith("/storage/")
? fileURL
: (new URL(fileURL).pathname.split("/").pop() ?? "");
const fileNameFromURL = lastSegment.split(/[?#]/)[0];
const fileExt = fileNameFromURL?.split(".").pop() ?? "";
const originalFileName = fileNameFromURL?.split("--fid--")[0] ?? "";
const fileId = fileNameFromURL?.split("--fid--")[1] ?? "";
const [namePart, fidPart] = fileNameFromURL.split("--fid--");
if (!fidPart) return namePart ? decodeURIComponent(namePart) : "";
if (!fileId) {
const fileName = originalFileName ? decodeURIComponent(originalFileName || "") : "";
return fileName;
}
const dotIdx = fileNameFromURL.lastIndexOf(".");
const hasExt = dotIdx > fileNameFromURL.indexOf("--fid--");
const ext = hasExt ? fileNameFromURL.slice(dotIdx + 1) : "";
const fileName = originalFileName ? decodeURIComponent(`${originalFileName}.${fileExt}` || "") : "";
return fileName;
return decodeURIComponent(ext ? `${namePart}.${ext}` : namePart);
} catch (error) {
logger.error(error, "Error parsing file URL");
}
};
export const getFileNameWithIdFromUrl = (fileURL: string) => {
try {
const fileNameFromURL = fileURL.startsWith("/storage/")
? fileURL.split("/").pop()
: new URL(fileURL).pathname.split("/").pop();
return fileNameFromURL ? decodeURIComponent(fileNameFromURL || "") : "";
} catch (error) {
logger.error(error, "Error parsing file URL");
logger.error({ error, fileURL }, "Error parsing file URL");
return "";
}
};
@@ -72,15 +61,10 @@ export const validateSingleFile = (
fileUrl: string,
allowedFileExtensions?: TAllowedFileExtension[]
): boolean => {
console.log("validateSingleFile", fileUrl);
const fileName = getOriginalFileNameFromUrl(fileUrl);
console.log("fileName", fileName);
if (!fileName) return false;
const extension = fileName.split(".").pop();
console.log("extension", extension);
if (!extension) return false;
console.log("allowedFileExtensions", allowedFileExtensions);
console.log("includes", allowedFileExtensions?.includes(extension as TAllowedFileExtension));
return !allowedFileExtensions || allowedFileExtensions.includes(extension as TAllowedFileExtension);
};
@@ -112,3 +96,24 @@ export const isValidImageFile = (fileUrl: string): boolean => {
const imageExtensions = ["png", "jpeg", "jpg", "webp", "heic"];
return imageExtensions.includes(extension);
};
export const getErrorResponseFromStorageError = (
error: StorageError,
details?: Record<string, string>
): Response => {
switch (error.code) {
case StorageErrorCode.FileNotFoundError:
return responses.notFoundResponse("file", details?.fileName ?? null, true);
case StorageErrorCode.InvalidInput:
return responses.badRequestResponse("Invalid input", details, true);
case StorageErrorCode.S3ClientError:
return responses.internalServerErrorResponse("Internal server error", true);
case StorageErrorCode.S3CredentialsError:
return responses.internalServerErrorResponse("Internal server error", true);
case StorageErrorCode.Unknown:
return responses.internalServerErrorResponse("Internal server error", true);
default: {
return responses.internalServerErrorResponse("Internal server error", true);
}
}
};