- Notifications
You must be signed in to change notification settings - Fork 0
fix(upload): align client precheck with optional public max MB#1069
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
e64dbc0
feat(upload): reject over-limit files in the browser before transferr…
BigSimmo 8e0dd7d
Merge branch 'main' into claude/upload-size-precheck-123366
BigSimmo eea934a
Merge branch 'main' into claude/upload-size-precheck-123366
BigSimmo 43b5e74
Merge remote-tracking branch 'origin/claude/upload-size-precheck-123366'
BigSimmo d51e2fb
Merge branch 'main' into claude/upload-size-precheck-123366
BigSimmo beee1fc
Merge remote-tracking branch 'origin/claude/upload-size-precheck-123366'
BigSimmo c947c92
fix(upload): align client precheck with optional public max MB
BigSimmo c023b92
style(upload): prettier-format DocumentManagerPanel precheck changes
BigSimmo ab53ff0
Merge origin/main into the upload pre-check branch
BigSimmo 0900a1e
Merge branch 'main' into claude/upload-size-precheck-123366
BigSimmo f087632
Merge branch 'main' into claude/upload-size-precheck-123366
BigSimmo ee6d038
Merge branch 'main' into claude/upload-size-precheck-123366
BigSimmo 65022b4
Merge branch 'main' into claude/upload-size-precheck-123366
BigSimmo c04b859
Merge branch 'main' into claude/upload-size-precheck-123366
BigSimmo f884f80
Merge branch 'main' into claude/upload-size-precheck-123366
BigSimmo aedf11e
Merge remote-tracking branch 'origin/main' into run-pr/1069-fix
BigSimmo 2f3b2b8
fix(upload): wire public max MB through Docker builds and pin DOM tests
BigSimmo 7e2adc7
Merge remote-tracking branch 'origin/claude/upload-size-precheck-1233…
BigSimmo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
20 changes: 10 additions & 10 deletions
20 src/components/clinical-dashboard/DocumentManagerPanel.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -8,16 +8,36 @@ | ||
| * | ||
| * The server's effective limit is `env.MAX_UPLOAD_MB`, whose schema caps it at | ||
| * `MAX_UPLOAD_MB_CEILING` — an operator can configure a *lower* limit, never a | ||
| * higher one. A browser-side pre-check at the ceiling therefore only ever | ||
| * rejects a file the server was certain to reject too; anything under it is | ||
| * still sent, and the server remains the authority. | ||
| * higher one. The browser pre-check reads optional `NEXT_PUBLIC_MAX_UPLOAD_MB` | ||
| * (clamped to the same ceiling) so the UI can match a lowered server limit; | ||
| * when that public env is unset it falls back to the ceiling. The server | ||
| * remains the authority for anything that still reaches `/api/upload`. | ||
| */ | ||
| /** Hard ceiling for a single uploaded file, in MB. */ | ||
| export const MAX_UPLOAD_MB_CEILING = 150; | ||
| const BYTES_PER_MB = 1024 * 1024; | ||
| /** | ||
| * Effective client-side upload limit in MB. | ||
| * Reads `NEXT_PUBLIC_MAX_UPLOAD_MB` when set to a positive integer, clamps to | ||
| * the ceiling, and otherwise uses the ceiling (safe default when operators | ||
| * have not mirrored a lowered `MAX_UPLOAD_MB`). | ||
| */ | ||
| export function getClientMaxUploadMb(): number { | ||
| const raw = process.env.NEXT_PUBLIC_MAX_UPLOAD_MB?.trim(); | ||
BigSimmo marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if (!raw) return MAX_UPLOAD_MB_CEILING; | ||
| const parsed = Number(raw); | ||
| if (!Number.isInteger(parsed) || parsed < 1) return MAX_UPLOAD_MB_CEILING; | ||
| return Math.min(parsed, MAX_UPLOAD_MB_CEILING); | ||
| } | ||
| /** True when a file exceeds the effective client pre-check limit. */ | ||
| export function exceedsClientUploadSize(sizeInBytes: number): boolean { | ||
| return sizeInBytes > getClientMaxUploadMb() * BYTES_PER_MB; | ||
| } | ||
| /** True when a file is larger than any limit the server can be configured to accept. */ | ||
| export function exceedsUploadSizeCeiling(sizeInBytes: number): boolean { | ||
| return sizeInBytes > MAX_UPLOAD_MB_CEILING * BYTES_PER_MB; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -9,8 +9,10 @@ import { MAX_UPLOAD_MB_CEILING } from "@/lib/upload-limits"; | ||
| // pre-check an over-ceiling file is transferred in full before the server | ||
| // answers 413 — on a large guideline PDF over a clinic connection that is a | ||
| // long wait for a guaranteed rejection. These tests pin that the pre-check | ||
| // fires locally, that it does NOT swallow files the server might still accept, | ||
| // and that a mixed batch still uploads its valid files. | ||
| // fires locally (using NEXT_PUBLIC_MAX_UPLOAD_MB when set, else the ceiling), | ||
| // that it does NOT swallow files the effective client limit still accepts, | ||
| // and that a mixed batch still uploads its valid files. The server remains | ||
| // the authority via env.MAX_UPLOAD_MB for anything that reaches /api/upload. | ||
| type OpenedRequest = { method: string; url: string }; | ||
| @@ -74,10 +76,15 @@ beforeEach(() => { | ||
| FakeXhr.lastStatus = 200; | ||
| FakeXhr.lastResponse = JSON.stringify({ document: { id: "doc-1" }, job: { id: "job-1" } }); | ||
| vi.stubGlobal("XMLHttpRequest", FakeXhr); | ||
| vi.unstubAllEnvs(); | ||
BigSimmo marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // Pin empty so default-limit assertions stay deterministic when the shell/CI | ||
| // already exports NEXT_PUBLIC_MAX_UPLOAD_MB (vi.unstubAllEnvs restores it). | ||
| vi.stubEnv("NEXT_PUBLIC_MAX_UPLOAD_MB", ""); | ||
| }); | ||
| afterEach(() => { | ||
| vi.unstubAllGlobals(); | ||
| vi.unstubAllEnvs(); | ||
| vi.restoreAllMocks(); | ||
| }); | ||
| @@ -126,4 +133,18 @@ describe("upload size pre-check", () => { | ||
| // Exactly one request: the oversized file never reached the network. | ||
| expect(opened).toHaveLength(1); | ||
| }); | ||
| it("honours a lowered NEXT_PUBLIC_MAX_UPLOAD_MB for the hint and pre-check", async () => { | ||
| vi.stubEnv("NEXT_PUBLIC_MAX_UPLOAD_MB", "50"); | ||
| const { onUploaded } = renderPanel(); | ||
| expect(screen.getByText("PDF only, up to 50 MB per file.")).toBeVisible(); | ||
| selectFiles([fileOfSize("mid-guideline.pdf", 51)]); | ||
| submit(); | ||
| expect(await screen.findByText(/mid-guideline\.pdf/)).toHaveTextContent("File exceeds 50 MB upload limit."); | ||
| expect(opened).toHaveLength(0); | ||
| expect(onUploaded).not.toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.