- Shipped
- September 23, 2026 at 2:35 AM UTC
- Author
- Kamo
- Commit
- d1a8762
/download loaded the whole file into a byte[] through ImageService.downloadDocument before serving it; /bulk-download built the whole ZIP (or merged PDF) as one byte[] in DocumentPrepareService before responding; uploads run up to 3 GB against this service's ~1.4 GB heap, and /bulk-download's 100-id cap had no size limit behind it — 100 uncapped documents could ask for hundreds of gigabytes in a single request. buildZip also wrote Img.fileName straight into a ZipEntry with no sanitization, so a document renamed to something like **************** (fileName is user-controlled — free-text, changed via /update/filename/{imgId}) produced an archive whose entry, opened by an extractor that does not itself guard against zip-slip, writes outside the target directory on whichever OS is doing the extracting. - /download now streams via MinIOStorageService.openRange + StreamingResponseBody, exactly like /stream already did, instead of buffering the whole file. (Drops ImageService.downloadDocument's dl-first/dl-last bookkeeping side effect — confirmed dead by grep: nothing in the Java backend ever reads Img.dlFirstDate/dlLastDate, only ever writes them. The separate ImgLogDownload audit row via recordDownloadEvent, which IS read back by the download-history panel, is unchanged.) - /bulk-download dedupes requested ids before the 100-id cap and the per-item work. - /bulk-download refuses outright (400) once the selected documents' combined Img.fileSize passes 500 MB, rather than attempting the merge/zip and risking an OOM — refused, not silently truncated, because completeness is the point of a bulk EXPORT. - buildZip sanitizes every entry name to its final path segment (splitting on both / and \, since the extractor may be on an OS this server does not control), closing the zip-slip gap. NOT DONE, reported as a known gap: buildZip/mergePdfs still build their result as one byte[] before responding rather than streaming it directly to the HTTP response — the 500 MB cap now bounds that to a known ceiling (down from unbounded), but it is not eliminated. Converting them to write into a caller-supplied OutputStream would let /bulk-download stream too; deferred because it changes DocumentPrepareService's public signatures, rippling into three existing test files (mocks currently stub the byte[]-returning methods), for a MEDIUM-severity, already-capped remainder — a deliberate scope call, not an oversight. New tests: **************** (7 cases for sanitizeZipEntryName) and three cases added to ImagingControllerMediaAssocTest (streaming-not-buffering, id dedupe, the size cap). Mutation-checked: reverting ImagingController.java to its pre-fix state turns the three new controller cases red; reverting sanitizeZipEntryName to `return name;` turns 5 of the 7 zip-slip cases red (the 6th, an already-safe ordinary filename, is correctly unaffected either way). Full suite: 741 tests green (was 731; +10 new).