Skip to content

Commit 9e9541b

Browse files
authored
Merge pull request #189 from bcgov/AI-1239
Enforce 100MB limit for classifier uploads
2 parents 0f5ab25 + e1c6ed6 commit 9e9541b

6 files changed

Lines changed: 146 additions & 2 deletions

File tree

apps/backend-services/src/azure/azure.controller.spec.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
ForbiddenException,
55
InternalServerErrorException,
66
NotFoundException,
7+
PayloadTooLargeException,
78
} from "@nestjs/common";
89
import { Request } from "express";
910
import type { AuditService } from "@/audit/audit.service";
@@ -212,6 +213,22 @@ describe("AzureController", () => {
212213
),
213214
).rejects.toThrow(NotFoundException);
214215
});
216+
it("should throw PayloadTooLargeException when a file exceeds 100MB", async () => {
217+
classifierService.findClassifierModel.mockResolvedValue({ id: "1" });
218+
const oversizedFile = {
219+
...mockFile,
220+
originalname: "large.pdf",
221+
size: 100 * 1024 * 1024 + 1,
222+
};
223+
await expect(
224+
controller.uploadClassifierDocuments(
225+
[oversizedFile],
226+
{ name: "c1", label: "l1" },
227+
"g1",
228+
),
229+
).rejects.toThrow(PayloadTooLargeException);
230+
expect(storageService.write).not.toHaveBeenCalled();
231+
});
215232
});
216233

217234
describe("deleteClassifierDocuments", () => {

apps/backend-services/src/azure/azure.controller.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,13 @@ import {
1212
NotFoundException,
1313
Param,
1414
Patch,
15+
PayloadTooLargeException,
1516
Post,
1617
Query,
1718
Req,
1819
UploadedFile,
1920
UploadedFiles,
21+
UseFilters,
2022
UseInterceptors,
2123
} from "@nestjs/common";
2224
import { FileInterceptor, FilesInterceptor } from "@nestjs/platform-express";
@@ -77,6 +79,7 @@ import {
7779
buildBlobPrefixPath,
7880
OperationCategory,
7981
} from "@/blob-storage/storage-path-builder";
82+
import { MulterExceptionFilter } from "@/filters/multer-exception.filter";
8083
import { GroupRole } from "@/generated/edge";
8184
import { AppLoggerService } from "@/logging/app-logger.service";
8285

@@ -229,7 +232,14 @@ export class AzureController {
229232
summary: "Upload training documents",
230233
description: "Upload training documents for a classifier.",
231234
})
232-
@UseInterceptors(FilesInterceptor("files"))
235+
@UseInterceptors(
236+
FilesInterceptor("files", 100, {
237+
limits: {
238+
fileSize: 100 * 1024 * 1024, // 100MB per file
239+
},
240+
}),
241+
)
242+
@UseFilters(MulterExceptionFilter)
233243
@ApiConsumes("multipart/form-data")
234244
@ApiQuery({ name: "group_id", required: true, description: "Group ID" })
235245
@ApiBody({
@@ -269,6 +279,15 @@ export class AzureController {
269279
throw new NotFoundException("No existing record of classifier model.");
270280
}
271281

282+
const maxFileSize = 100 * 1024 * 1024; // 100MB
283+
for (const file of files) {
284+
if (file.size > maxFileSize) {
285+
throw new PayloadTooLargeException(
286+
`File ${file.originalname} exceeds maximum size of 100MB`,
287+
);
288+
}
289+
}
290+
272291
const uploadResults: string[] = [];
273292
for (const file of files) {
274293
const key = buildBlobFilePath(
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { PayloadTooLargeException } from "@nestjs/common";
2+
import { MulterError } from "multer";
3+
import { MulterExceptionFilter } from "./multer-exception.filter";
4+
5+
describe("MulterExceptionFilter", () => {
6+
const filter = new MulterExceptionFilter();
7+
8+
it("should return 413 for LIMIT_FILE_SIZE", () => {
9+
const json = jest.fn();
10+
const status = jest.fn().mockReturnValue({ json });
11+
const host = {
12+
switchToHttp: () => ({
13+
getResponse: () => ({ status }),
14+
}),
15+
};
16+
17+
filter.catch(new MulterError("LIMIT_FILE_SIZE", "files"), host as never);
18+
19+
expect(status).toHaveBeenCalledWith(413);
20+
expect(json).toHaveBeenCalledWith(
21+
new PayloadTooLargeException(
22+
"File exceeds maximum allowed size",
23+
).getResponse(),
24+
);
25+
});
26+
27+
it("should return 400 for other multer errors", () => {
28+
const json = jest.fn();
29+
const status = jest.fn().mockReturnValue({ json });
30+
const host = {
31+
switchToHttp: () => ({
32+
getResponse: () => ({ status }),
33+
}),
34+
};
35+
36+
filter.catch(
37+
new MulterError("LIMIT_UNEXPECTED_FILE", "files"),
38+
host as never,
39+
);
40+
41+
expect(status).toHaveBeenCalledWith(400);
42+
expect(json).toHaveBeenCalledWith({
43+
statusCode: 400,
44+
message: "Unexpected field",
45+
error: "Bad Request",
46+
});
47+
});
48+
});
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import {
2+
ArgumentsHost,
3+
Catch,
4+
ExceptionFilter,
5+
PayloadTooLargeException,
6+
} from "@nestjs/common";
7+
import { Response } from "express";
8+
import { MulterError } from "multer";
9+
10+
@Catch(MulterError)
11+
export class MulterExceptionFilter implements ExceptionFilter {
12+
catch(exception: MulterError, host: ArgumentsHost): void {
13+
const response = host.switchToHttp().getResponse<Response>();
14+
15+
if (exception.code === "LIMIT_FILE_SIZE") {
16+
const body = new PayloadTooLargeException(
17+
"File exceeds maximum allowed size",
18+
).getResponse();
19+
response.status(413).json(body);
20+
return;
21+
}
22+
23+
response.status(400).json({
24+
statusCode: 400,
25+
message: exception.message,
26+
error: "Bad Request",
27+
});
28+
}
29+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# Classifier document upload limits
2+
3+
`POST /api/azure/classifier/documents` accepts multipart training-document uploads for classifiers.
4+
5+
## File size limit
6+
7+
Each uploaded file may be up to **100 MB**, matching the dataset version upload endpoint in `apps/backend-services/src/benchmark/dataset.controller.ts`.
8+
9+
The route configures multer via `FilesInterceptor` with `limits.fileSize: 100 * 1024 * 1024`. Files larger than 100 MB receive **HTTP 413 Payload Too Large** (via `MulterExceptionFilter` for multer rejections and an explicit controller check as a safeguard).
10+
11+
## Load testing context
12+
13+
Load testing previously reported HTTP 500 for uploads above ~64 KB because the classifier route used `FilesInterceptor("files")` without an explicit limit. The `blob-storage` k6 scenario (256 KB blobs) showed a **20.6 %** failure rate until this limit was aligned with the dataset upload endpoint.
14+
15+
See also:
16+
17+
- [`docs-md/LOAD_TEST_REPORT_2026-05.md`](./LOAD_TEST_REPORT_2026-05.md) — Finding 3
18+
- [`docs-md/LOAD_TESTING.md`](./LOAD_TESTING.md) — blob storage scenario
19+
20+
## Verification
21+
22+
```bash
23+
# Expect HTTP 201 (classifier must exist; replace group/name/label as needed)
24+
curl -H "x-api-key: <api-key>" \
25+
-F "files=@/path/to/1mb.pdf" \
26+
-F "name=<classifier>" \
27+
-F "label=<label>" \
28+
"http://localhost:3002/api/azure/classifier/documents?group_id=<group>"
29+
```
30+
31+
Files over 100 MB should return **413**, not 500.

docs-md/openshift-deployment/MANUAL_LOAD_TEST_INSTANCE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ The patch lasts for the life of the ConfigMap; re-running **`oc-deploy-instance.
159159
The matrix runs in [`tools/load-testing/test-matrix.csv`](../../tools/load-testing/test-matrix.csv) surfaced two reproducible issues that are not specific to the load-test instance — they are **product limits** that any operator will hit. They are documented here so the next person running the suite knows what they are seeing and is not blocked by them.
160160

161161
1. **Backend pod becomes unavailable under concurrent large uploads.** Running `payload-sizes` at **`large` (5 MB raw PDF) × 5 VUs** drove the **`backend-services`** pod into a restart that returned **`HTTP 502`** (router→dead pod) followed by **`HTTP 503`** (router→no healthy upstream) for ~30–60 s. **`smoke`** and **`datasets`** were both unreachable during the window. **`large × 1 VU`** runs cleanly. The most likely cause is per-request memory pressure from `JSON.parse` + `Buffer.from(b64,'base64')` + `pdf-lib` normalization at the same time, exceeding the pod's `memory.limit`. Confirm with **`oc describe pod <instance>-backend-services-<id>`** and look for **`Last State: Terminated · Reason: OOMKilled · Exit Code: 137`** after a failing run. Workaround for the load-test instance: raise the deployment **`resources.limits.memory`**. Real fix (in source): stream the upload through pdf-lib instead of holding the full buffer twice, or cap concurrent uploads at the controller via a semaphore.
162-
2. **Classifier-document upload fails with HTTP 500 above ~64 KB.** The endpoint **`POST /api/azure/classifier/documents`** in **[`apps/backend-services/src/azure/azure.controller.ts`](../../apps/backend-services/src/azure/azure.controller.ts:223)** uses **`@UseInterceptors(FilesInterceptor("files"))`** with no explicit limits. Files **≤ 64 KB** return 201; **≥ 96 KB** consistently return **`HTTP 500 {"statusCode":500,"message":"Internal server error"}`** in under 250 ms. The list and delete endpoints on the same controller (`GET`/`DELETE /api/azure/classifier/documents`) are unaffected. The OCR upload at **`POST /api/upload`** is unaffected because it is JSON-bodied (governed by **`BODY_LIMIT`**), not multipart. Compare with **[`apps/backend-services/src/benchmark/dataset.controller.ts:222`](../../apps/backend-services/src/benchmark/dataset.controller.ts)** where the dataset multipart endpoint explicitly sets **`limits: { fileSize: 100 * 1024 * 1024 }`** — the classifier endpoint should do the same. Without **`oc`** access to read the backend log this could not be confirmed end-to-end, but the failure is fast, deterministic, and size-correlated, which fits a multer **`limits.fileSize`** rejection. Fix: add **`@UseInterceptors(FilesInterceptor("files", { limits: { fileSize: 100 * 1024 * 1024 } }))`** on the classifier upload route, or register a global **`MulterModule.register`** with a project-wide default that matches **`BODY_LIMIT`**.
162+
2. **Classifier-document upload fails with HTTP 500 above ~64 KB** *(fixed)*. The endpoint **`POST /api/azure/classifier/documents`** now sets an explicit **`limits.fileSize: 100 * 1024 * 1024`** on its `FilesInterceptor`, matching **[`apps/backend-services/src/benchmark/dataset.controller.ts:222`](../../apps/backend-services/src/benchmark/dataset.controller.ts)**. Oversized files return **HTTP 413** via **`MulterExceptionFilter`**. See **[`docs-md/CLASSIFIER_DOCUMENT_UPLOAD.md`](../CLASSIFIER_DOCUMENT_UPLOAD.md)**.
163163
3. **In-cluster MinIO fills up after a few sustained upload runs.** After roughly **~1,500** documents uploaded across **`upload-ocr`** and **`payload-sizes`** runs at 256 KB–5 MB tiers, **`POST /api/upload`** starts returning **`HTTP 400 {"message":"Failed to write blob \"<group>/ocr/<id>/original.pdf\": Storage backend has reached its minimum free drive threshold. Please delete a few objects to proceed."}`**. The latency on the failed call is ~4 s (MinIO retries before erroring), and the error is upstream of the backend — it surfaces directly from the AWS S3 SDK call inside **[`apps/backend-services/src/blob-storage/minio-blob-storage.service.ts:86`](../../apps/backend-services/src/blob-storage/minio-blob-storage.service.ts)**. The MinIO PVC sizing comes from **[`deployments/openshift/kustomize/components/minio/pvc.yml`](../../deployments/openshift/kustomize/components/minio/pvc.yml)** with the size templated by **`scripts/lib/generate-overlay.sh`** (default `5Gi`, override with **`--minio-pvc-size`** or `MINIO_PVC_SIZE` in **`dev.env`**). The storage class **`netapp-file-standard`** has **`allowVolumeExpansion=true`**, so an existing PVC can be grown online with no pod restart and no data loss:
164164

165165
```bash

0 commit comments

Comments
 (0)