mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 22:10:34 +00:00
🐛 Harden SVG sanitization: workers, limits and error mapping
Address the review findings on the DOMPurify SVG sanitization. F1: sanitizeSvg was synchronous and blocked the service's only event loop: a ~30 MiB SVG (the old cap) froze media-processor for ~20 s and peaked at ~3.4 GiB. The sanitizer now runs in a worker_threads pool (sanitizeSvgSync is the CPU-bound core), each worker has a resourceLimits heap cap so an OOM kills the worker and not the process, and a stuck job is terminated by a timeout and respawned. The default SVG cap drops to 2 MiB and is enforced in both modes (the backend validates the declared and the real size on upload, and the real bytes on import), so a file is accepted or rejected the same way with and without media-processor. F2: an unexpected sanitizer failure is logged and reported as 503 svg-sanitization-failed instead of 400 invalid-svg-file, so a service fault is not blamed on the user's file and no internal detail leaks in the hint. F4: the remote backend rejects a 2xx response that is not an SVG instead of storing it. Also pin the backend/service wire contract with a test that posts the exact multipart the JVM builds, and pin the data: URI behavior. AI-assisted-by: deepseek-v4.1-flash
This commit is contained in:
parent
6961767881
commit
fc500d1102
@ -24,6 +24,18 @@ Rules:
|
||||
- Fail closed: when the remote backend is enabled but the service fails, the
|
||||
exception propagates and the upload/import fails; there is no silent fallback
|
||||
to the weaker local filter.
|
||||
- SVG has its own size cap, `:media-svg-max-file-size` (default 2MiB, env
|
||||
`PENPOT_MEDIA_SVG_MAX_FILE_SIZE`), lower than `:media-max-file-size`. It is
|
||||
enforced in both modes and reports `:svg-too-large`, the same code
|
||||
media-processor returns, so the same file is accepted or rejected regardless of
|
||||
the mode. The upload path checks the declared `:size` early
|
||||
(`app.media.validation/validate-media-size!`) and the real bytes later
|
||||
(`process-main-image`), so a client cannot lie about the size; the import path
|
||||
checks the real bytes (`sanitize-imported-svg`). Keep it aligned with
|
||||
`PENPOT_MEDIA_PROCESSOR_SVG_MAX_SIZE` (`mem:media-processor/core`).
|
||||
- The remote defmethod rejects a 2xx response that is not an SVG (it must start
|
||||
with `<svg`), so a misconfigured proxy cannot get a non-SVG body stored as
|
||||
`image/svg+xml`.
|
||||
- Text in, text out: the sanitizer takes and returns the SVG as a UTF-8 string.
|
||||
- `:app.tasks.import-binfile/job-def` carries `::http.client/client` and
|
||||
`::setup/shared-keys` (the RPC cfg already does), so the import job — not only
|
||||
|
||||
@ -27,6 +27,8 @@ media-processor/
|
||||
│ ├── upload.ts # Multer configuration, getFileBuffer helper
|
||||
│ ├── upload-storage.ts # Hybrid storage engine (memory < threshold, disk >= threshold)
|
||||
│ ├── logger.ts # Pino logger setup
|
||||
│ ├── svg-pool.ts # worker_threads pool for SVG sanitization
|
||||
│ ├── svg-worker.ts # worker entry point (runs sanitizeSvgSync)
|
||||
│ ├── middleware/
|
||||
│ │ ├── auth.ts # Timing-safe shared key authentication
|
||||
│ │ ├── error-handler.ts # ProcessingError class, centralized error handling
|
||||
@ -57,7 +59,8 @@ media-processor/
|
||||
|
||||
### Resource Limits
|
||||
- Image: max pixels, max width/height enforced before processing
|
||||
- SVG: max input size enforced before parsing (default 30MB, `PENPOT_MEDIA_PROCESSOR_SVG_MAX_SIZE`); an oversized input is rejected with 413, never truncated
|
||||
- SVG: max input size enforced before parsing (default 2MB, `PENPOT_MEDIA_PROCESSOR_SVG_MAX_SIZE`); an oversized input is rejected with 413, never truncated
|
||||
- SVG workers: `PENPOT_MEDIA_PROCESSOR_SVG_WORKERS` (default 2; 0 runs inline), `PENPOT_MEDIA_PROCESSOR_SVG_WORKER_MAX_OLD_MB` (default 512, per-worker V8 old-generation cap) and `PENPOT_MEDIA_PROCESSOR_SVG_TIMEOUT` (default 30000 ms)
|
||||
- Font: prlimit wraps FontForge processes with memory (AS) and CPU time limits
|
||||
- Concurrency: p-queue limits concurrent requests (default 10)
|
||||
- Upload: hybrid storage — memory for files < 10MB, disk for larger; configurable via `PENPOT_MEDIA_PROCESSOR_MEMORY_THRESHOLD`
|
||||
@ -82,6 +85,18 @@ media-processor/
|
||||
- `POST /api/svg/sanitize` (multipart field `file`) returns the sanitized SVG
|
||||
bytes with `Content-Type: image/svg+xml`; it is the remote backend of
|
||||
`app.media/sanitize-svg` in the JVM.
|
||||
- The route never runs the sanitizer on the main thread: `sanitizeSvg` delegates
|
||||
to a pool of `worker_threads` workers (`svg-pool.ts` / `svg-worker.ts`), so a
|
||||
heavy SVG cannot block the service's event loop (`/api/health` included).
|
||||
`sanitizeSvgSync` is the CPU-bound core and stays synchronous. With
|
||||
`PENPOT_MEDIA_PROCESSOR_SVG_WORKERS=0` the pool is disabled and the sanitizer
|
||||
runs inline (tests and an escape hatch).
|
||||
- The pool is created at boot, queues when every worker is busy, and applies
|
||||
`resourceLimits: { maxOldGenerationSizeMb }` per worker: a worker that runs out
|
||||
of memory dies with an error instead of the kernel killing the process, and the
|
||||
pool maps that to a 503 and respawns it. A stuck job is terminated by a timeout
|
||||
and its worker respawned. Worst-case memory is roughly workers ×
|
||||
maxOldGenerationSizeMb.
|
||||
- DOMPurify is given `USE_PROFILES: {svg: true, svgFilters: true}` and
|
||||
`NAMESPACE: "http://www.w3.org/2000/svg"`, so it parses the document as XML/SVG
|
||||
the way a browser parses a standalone `.svg`. Its default allowlist fails closed
|
||||
@ -95,7 +110,20 @@ media-processor/
|
||||
bails on them; stripping the DOCTYPE also keeps its entities undeclared). An
|
||||
input that does not produce an `<svg` root is rejected with 400
|
||||
`invalid-svg-file`.
|
||||
- Errors: `400 invalid-svg-file` (not a well-formed SVG), `413 svg-too-large`.
|
||||
- Error mapping: a parse/validation failure is 400 `invalid-svg-file`; anything
|
||||
else (a bug, an OOM, a DOMPurify regression) is logged and reported as 503
|
||||
`svg-sanitization-failed`, never as an invalid file.
|
||||
- The 2MB cap is deliberately lower than the backend's `:media-max-file-size`
|
||||
(30MiB): the jsdom parse cost and memory do not scale with bytes the way
|
||||
sharp's does, so a large SVG is a denial-of-service vector. The backend enforces
|
||||
the same cap (`:media-svg-max-file-size`, `mem:backend/media-sanitization`) so
|
||||
both modes behave the same; keep the two knobs aligned.
|
||||
- Errors: `400 invalid-svg-file` (not a well-formed SVG), `413 svg-too-large`,
|
||||
`503 svg-sanitization-failed` / `svg-timeout` / `svg-worker-failed`.
|
||||
- Tests: `svg.test.ts` (service corpus, route, and the exact multipart the JVM
|
||||
backend builds) and `svg-pool.test.ts` (queue, timeout, failure/respawn with a
|
||||
fake worker). The contract test pins the wire format but does not run the JVM
|
||||
against a live service — that gap is known and accepted.
|
||||
|
||||
## Commands
|
||||
|
||||
|
||||
@ -1024,10 +1024,22 @@
|
||||
path: the local filter, or DOMPurify in media-processor when remote
|
||||
media processing is enabled (`app.media/sanitize-svg`).
|
||||
|
||||
SVG has its own, lower size cap (`:media-svg-max-file-size`), the same
|
||||
the upload path and media-processor apply, so a package cannot smuggle
|
||||
an SVG too large for the sanitizer. Over the cap raises `:svg-too-large`.
|
||||
|
||||
Raises a `:validation` exception when the SVG cannot be parsed, the
|
||||
same error the upload path reports."
|
||||
[system object ^bytes raw]
|
||||
(when (svg-object? object)
|
||||
(let [max-size (cf/get :media-svg-max-file-size)
|
||||
size (alength raw)]
|
||||
(when (> size max-size)
|
||||
(ex/raise :type :restriction
|
||||
:code :svg-too-large
|
||||
:hint (str/ffmt "the imported svg size % is greater than the maximum %"
|
||||
size
|
||||
max-size))))
|
||||
(let [sanitized (media/sanitize-svg system (String. ^bytes raw "UTF-8"))
|
||||
bytes (.getBytes ^String sanitized "UTF-8")]
|
||||
{:bytes bytes
|
||||
|
||||
@ -83,6 +83,11 @@
|
||||
:jobs-request-timeout (ct/duration {:minutes 2})
|
||||
|
||||
:media-max-file-size (* 1024 1024 30) ; 30MiB
|
||||
;; SVG has its own, lower cap: parsing it with jsdom in media-processor
|
||||
;; costs memory and time out of proportion to its byte size, so a large SVG
|
||||
;; is a denial-of-service vector for the service. Keep it aligned with
|
||||
;; PENPOT_MEDIA_PROCESSOR_SVG_MAX_SIZE (see mem:backend/media-sanitization).
|
||||
:media-svg-max-file-size (* 1024 1024 2) ; 2MiB
|
||||
:font-max-file-size (* 1024 1024 30) ; 30MiB
|
||||
|
||||
:font-process-mem 512 ;; 512 MiB address space ceiling
|
||||
@ -163,6 +168,7 @@
|
||||
[:auto-file-snapshot-timeout {:optional true} ::ct/duration]
|
||||
|
||||
[:media-max-file-size {:optional true} ::sm/int]
|
||||
[:media-svg-max-file-size {:optional true} ::sm/int]
|
||||
[:font-max-file-size {:optional true} ::sm/int]
|
||||
|
||||
;; Font processing resource limits (PENPOT_FONT_PROCESS_*)
|
||||
|
||||
@ -226,7 +226,15 @@
|
||||
:mtype "image/svg+xml"})
|
||||
body (:body resp)]
|
||||
(try
|
||||
(assoc params :content (slurp body :encoding "UTF-8"))
|
||||
;; Defense in depth: the service guarantees an `<svg` root, but a
|
||||
;; misconfigured proxy or a future endpoint change must not let a non-SVG
|
||||
;; body be stored as `image/svg+xml`.
|
||||
(let [cleaned (str/trim (slurp body :encoding "UTF-8"))]
|
||||
(when-not (str/starts-with? cleaned "<svg")
|
||||
(ex/raise :type :internal
|
||||
:code :invalid-sanitized-svg
|
||||
:hint "media-processor returned a non-SVG response"))
|
||||
(assoc params :content cleaned))
|
||||
(finally
|
||||
(.close body)))))
|
||||
|
||||
|
||||
@ -42,12 +42,19 @@
|
||||
|
||||
upload))
|
||||
|
||||
(def ^:private svg-mtype "image/svg+xml")
|
||||
|
||||
(defn validate-media-size!
|
||||
"Validate the size of a media `upload`. SVG uses its own, lower cap
|
||||
(`:media-svg-max-file-size`) and reports `:svg-too-large`, the same code
|
||||
media-processor returns; every other type uses `:media-max-file-size`."
|
||||
[upload]
|
||||
(let [max-size (cf/get :media-max-file-size)]
|
||||
(let [svg? (= svg-mtype (:mtype upload))
|
||||
max-size (if svg? (cf/get :media-svg-max-file-size) (cf/get :media-max-file-size))
|
||||
code (if svg? :svg-too-large :media-max-file-size-reached)]
|
||||
(when (> (:size upload) max-size)
|
||||
(ex/raise :type :restriction
|
||||
:code :media-max-file-size-reached
|
||||
:code code
|
||||
:hint (str/ffmt "the uploaded file size % is greater than the maximum %"
|
||||
(:size upload)
|
||||
max-size)))
|
||||
|
||||
@ -129,11 +129,19 @@
|
||||
(let [path (:path info)
|
||||
mtype (:mtype info)
|
||||
path (if (= mtype "image/svg+xml")
|
||||
(let [content (slurp path)
|
||||
sanitized (media/sanitize-svg cfg content)
|
||||
temp-path (tmp/tempfile :prefix "penpot-svg-" :suffix ".svg" :min-age "5m")]
|
||||
(spit (str temp-path) sanitized)
|
||||
temp-path)
|
||||
(let [content (slurp path)
|
||||
size (alength (.getBytes ^String content "UTF-8"))
|
||||
max-size (cf/get :media-svg-max-file-size)]
|
||||
;; The declared size is checked earlier; this is the real
|
||||
;; one, so a client cannot smuggle a bigger SVG past the cap.
|
||||
(when (> size max-size)
|
||||
(ex/raise :type :restriction
|
||||
:code :svg-too-large
|
||||
:hint (str "the svg size " size " is greater than the maximum " max-size)))
|
||||
(let [sanitized (media/sanitize-svg cfg content)
|
||||
temp-path (tmp/tempfile :prefix "penpot-svg-" :suffix ".svg" :min-age "5m")]
|
||||
(spit (str temp-path) sanitized)
|
||||
temp-path))
|
||||
path)
|
||||
hash (sto/calculate-hash path)
|
||||
data (-> (sto/content path)
|
||||
|
||||
@ -118,6 +118,18 @@
|
||||
(bfc/sanitize-imported-svg th/*system* {:content-type "image/svg+xml" :bucket "file-media-object"}
|
||||
(utf8bytes "<svg><not-closed>"))))))
|
||||
|
||||
(t/deftest sanitize-imported-svg-rejects-over-cap
|
||||
(t/testing "an imported SVG over the cap is rejected before parsing"
|
||||
(let [raw (byte-array (inc (cf/get :media-svg-max-file-size)))
|
||||
err (try
|
||||
(bfc/sanitize-imported-svg th/*system*
|
||||
{:content-type "image/svg+xml" :bucket "file-media-object"}
|
||||
raw)
|
||||
nil
|
||||
(catch clojure.lang.ExceptionInfo e (ex-data e)))]
|
||||
(t/is (= :restriction (:type err)))
|
||||
(t/is (= :svg-too-large (:code err))))))
|
||||
|
||||
(t/deftest sanitize-imported-svg-routes-through-media-processor
|
||||
(t/testing "with remote media processing on, the import sanitizes through the service"
|
||||
(let [served "<svg xmlns=\"http://www.w3.org/2000/svg\"><circle r=\"4\"/></svg>"]
|
||||
@ -433,14 +445,18 @@
|
||||
(t/is (= [(:id file)]
|
||||
(mapv :id (th/db-query :file {:project-id (:default-project-id profile)}))))))
|
||||
|
||||
(def ^:private large-clean-svg
|
||||
(let [pad (apply str (repeat 110000 "<!--0123456789ABCDEF-->"))]
|
||||
;; Small on purpose: the test lowers `bfc/temp-file-threshold` with
|
||||
;; `with-redefs` to reach the tempfile branch, because the SVG size cap (2MiB)
|
||||
;; is now the same as the real threshold and an SVG cannot exceed it.
|
||||
(def ^:private tempfile-branch-svg
|
||||
(let [pad (apply str (repeat 100 "<!--0123456789ABCDEF-->"))]
|
||||
(str "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"64\" height=\"64\">" pad "<circle r=\"20\"/></svg>")))
|
||||
|
||||
(t/deftest import-binfile-v1-roundtrips-large-svg
|
||||
(t/testing "objects over the tempfile threshold (tempfile branch) import fine"
|
||||
(t/is (> (alength (utf8bytes large-clean-svg)) bfc/temp-file-threshold))
|
||||
(let [profile (th/create-profile* 5)
|
||||
[file _seed] (seed-file-with-svg-media profile 5 large-clean-svg "image/svg+xml")
|
||||
[served _stored] (export-import-v1 profile file)]
|
||||
(t/is (str/includes? served "circle")))))
|
||||
[file _seed] (seed-file-with-svg-media profile 5 tempfile-branch-svg "image/svg+xml")]
|
||||
(with-redefs [bfc/temp-file-threshold 1024]
|
||||
(t/is (> (alength (utf8bytes tempfile-branch-svg)) bfc/temp-file-threshold))
|
||||
(let [[served _stored] (export-import-v1 profile file)]
|
||||
(t/is (str/includes? served "circle")))))))
|
||||
|
||||
@ -662,3 +662,15 @@
|
||||
(with-redefs [cf/flags #{:remote-media-processing}
|
||||
cf/get (th/config-get-mock config-mock)]
|
||||
(t/is (= "<svg/>REMOTE" (media/sanitize-svg (mk-system) "<svg/>")))))))
|
||||
|
||||
(t/deftest sanitize-svg-rejects-non-svg-response
|
||||
(t/testing "a 2xx response that is not an SVG is rejected"
|
||||
(with-mocks [mock {:target 'app.media.remote/service-request
|
||||
:return {:status 200
|
||||
:body (ByteArrayInputStream. (.getBytes "<html>nope</html>" "UTF-8"))}}]
|
||||
(with-redefs [cf/get (th/config-get-mock config-mock)]
|
||||
(let [err (ex/try! (media.remote/process (mk-system)
|
||||
{:cmd :sanitize-svg :content "<svg/>"}))]
|
||||
(t/is (ex/error? err))
|
||||
(t/is (= :internal (:type (ex-data err))))
|
||||
(t/is (= :invalid-sanitized-svg (:code (ex-data err)))))))))
|
||||
|
||||
@ -10,6 +10,7 @@
|
||||
[app.config :as cf]
|
||||
[app.media :as media]
|
||||
[app.media.svg :as svg]
|
||||
[app.media.validation :as media.v]
|
||||
[backend-tests.helpers :as th]
|
||||
[clojure.test :as t]
|
||||
[datoteka.fs :as fs]))
|
||||
@ -221,6 +222,19 @@
|
||||
(t/is (= :validation (:type (ex-data err))))
|
||||
(t/is (= :invalid-svg-file (:code (ex-data err))))))))
|
||||
|
||||
(t/deftest validate-media-size-svg-cap
|
||||
(t/testing "SVG uses its own cap and code; other types use the general cap"
|
||||
(let [cap (cf/get :media-svg-max-file-size)]
|
||||
(t/is (pos? cap))
|
||||
(let [err (ex/try! (media.v/validate-media-size! {:mtype "image/svg+xml" :size (inc cap)}))]
|
||||
(t/is (ex/error? err))
|
||||
(t/is (= :restriction (:type (ex-data err))))
|
||||
(t/is (= :svg-too-large (:code (ex-data err)))))
|
||||
(t/is (= {:mtype "image/svg+xml" :size cap}
|
||||
(media.v/validate-media-size! {:mtype "image/svg+xml" :size cap})))
|
||||
(t/is (= {:mtype "image/jpeg" :size (inc cap)}
|
||||
(media.v/validate-media-size! {:mtype "image/jpeg" :size (inc cap)}))))))
|
||||
|
||||
(t/deftest info-invalid-image
|
||||
(t/testing "info on invalid image raises error"
|
||||
(let [path (fs/create-tempfile :prefix "penpot-test-" :suffix ".jpg")]
|
||||
|
||||
@ -1238,6 +1238,51 @@
|
||||
(t/is (= served stored))
|
||||
(t/is (= 1 (:call-count @mock))))))))))
|
||||
|
||||
(def ^:private big-upload-svg
|
||||
(str "<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"64\" height=\"64\">"
|
||||
(apply str (repeat 90000 "<rect width=\"1\" height=\"1\"/>"))
|
||||
"</svg>"))
|
||||
|
||||
(t/deftest upload-svg-over-cap-is-rejected
|
||||
(t/testing "an SVG over the svg cap is rejected on upload"
|
||||
(let [prof (th/create-profile* 1)
|
||||
file (th/create-file* 1 {:profile-id (:id prof)
|
||||
:project-id (:default-project-id prof)
|
||||
:is-shared false})
|
||||
path (write-svg-tempfile big-upload-svg)
|
||||
out (th/command! {::th/type :upload-file-media-object
|
||||
::rpc/profile-id (:id prof)
|
||||
:file-id (:id file)
|
||||
:is-local true
|
||||
:name "big.svg"
|
||||
:content {:filename "big.svg"
|
||||
:path path
|
||||
:mtype "image/svg+xml"
|
||||
:size (alength (.getBytes big-upload-svg "UTF-8"))}})]
|
||||
(t/is (th/ex-info? (:error out)))
|
||||
(t/is (th/ex-of-type? (:error out) :restriction))
|
||||
(t/is (th/ex-of-code? (:error out) :svg-too-large)))))
|
||||
|
||||
(t/deftest upload-svg-over-cap-with-lying-size-is-rejected
|
||||
(t/testing "the real size is checked even when the declared size lies"
|
||||
(let [prof (th/create-profile* 1)
|
||||
file (th/create-file* 1 {:profile-id (:id prof)
|
||||
:project-id (:default-project-id prof)
|
||||
:is-shared false})
|
||||
path (write-svg-tempfile big-upload-svg)
|
||||
out (th/command! {::th/type :upload-file-media-object
|
||||
::rpc/profile-id (:id prof)
|
||||
:file-id (:id file)
|
||||
:is-local true
|
||||
:name "big.svg"
|
||||
:content {:filename "big.svg"
|
||||
:path path
|
||||
:mtype "image/svg+xml"
|
||||
:size 10}})]
|
||||
(t/is (th/ex-info? (:error out)))
|
||||
(t/is (th/ex-of-type? (:error out) :restriction))
|
||||
(t/is (th/ex-of-code? (:error out) :svg-too-large)))))
|
||||
|
||||
(t/deftest upload-file-media-object-rejects-client-id
|
||||
(let [prof (th/create-profile* 1)
|
||||
file (th/create-file* 1 {:profile-id (:id prof)
|
||||
|
||||
@ -1,12 +1,12 @@
|
||||
import { build } from "esbuild";
|
||||
|
||||
await build({
|
||||
entryPoints: ["src/index.ts"],
|
||||
entryPoints: ["src/index.ts", "src/svg-worker.ts"],
|
||||
bundle: true,
|
||||
platform: "node",
|
||||
target: "node24",
|
||||
format: "esm",
|
||||
outfile: "dist/index.js",
|
||||
outdir: "dist",
|
||||
external: ["sharp", "pino", "pino-pretty", "pino-loki", "dompurify", "jsdom"],
|
||||
banner: {
|
||||
js: `
|
||||
|
||||
@ -12,7 +12,10 @@ const envSchema = z.object({
|
||||
PENPOT_MEDIA_PROCESSOR_IMAGE_MAX_PIXELS: z.coerce.number().int().positive().default(128_000_000),
|
||||
PENPOT_MEDIA_PROCESSOR_IMAGE_MAX_WIDTH: z.coerce.number().int().positive().default(16384),
|
||||
PENPOT_MEDIA_PROCESSOR_IMAGE_MAX_HEIGHT: z.coerce.number().int().positive().default(16384),
|
||||
PENPOT_MEDIA_PROCESSOR_SVG_MAX_SIZE: z.coerce.number().int().positive().default(31457280), // 30 MB
|
||||
PENPOT_MEDIA_PROCESSOR_SVG_MAX_SIZE: z.coerce.number().int().positive().default(2097152), // 2 MB — see mem:media-processor/core
|
||||
PENPOT_MEDIA_PROCESSOR_SVG_WORKERS: z.coerce.number().int().nonnegative().default(2), // 0 = run inline (tests/escape hatch)
|
||||
PENPOT_MEDIA_PROCESSOR_SVG_WORKER_MAX_OLD_MB: z.coerce.number().int().positive().default(512),
|
||||
PENPOT_MEDIA_PROCESSOR_SVG_TIMEOUT: z.coerce.number().int().positive().default(30000),
|
||||
PENPOT_MEDIA_PROCESSOR_FONT_PROCESS_MEM: z.coerce.number().int().positive().default(512),
|
||||
PENPOT_MEDIA_PROCESSOR_FONT_PROCESS_CPU_TIME: z.coerce.number().int().positive().default(30),
|
||||
PENPOT_MEDIA_PROCESSOR_FONT_TIMEOUT: z.coerce.number().int().positive().default(120000),
|
||||
@ -53,6 +56,9 @@ export function loadConfig(): AppConfig {
|
||||
imageMaxWidth: parsed.PENPOT_MEDIA_PROCESSOR_IMAGE_MAX_WIDTH,
|
||||
imageMaxHeight: parsed.PENPOT_MEDIA_PROCESSOR_IMAGE_MAX_HEIGHT,
|
||||
svgMaxSize: parsed.PENPOT_MEDIA_PROCESSOR_SVG_MAX_SIZE,
|
||||
svgWorkers: parsed.PENPOT_MEDIA_PROCESSOR_SVG_WORKERS,
|
||||
svgWorkerMaxOldMb: parsed.PENPOT_MEDIA_PROCESSOR_SVG_WORKER_MAX_OLD_MB,
|
||||
svgTimeout: parsed.PENPOT_MEDIA_PROCESSOR_SVG_TIMEOUT,
|
||||
fontProcessMem: parsed.PENPOT_MEDIA_PROCESSOR_FONT_PROCESS_MEM,
|
||||
fontProcessCpuTime: parsed.PENPOT_MEDIA_PROCESSOR_FONT_PROCESS_CPU_TIME,
|
||||
fontTimeout: parsed.PENPOT_MEDIA_PROCESSOR_FONT_TIMEOUT,
|
||||
|
||||
@ -13,6 +13,7 @@ import { loggingMiddleware } from "./middleware/logging.js";
|
||||
import { configureImageLimits } from "./services/image.js";
|
||||
import { configureFontLimits } from "./services/font.js";
|
||||
import { configureSvgLimits } from "./services/svg.js";
|
||||
import { configureSvgPool, shutdownSvgPool } from "./svg-pool.js";
|
||||
import { configureUploadLimits } from "./upload.js";
|
||||
import sharp from "sharp";
|
||||
|
||||
@ -43,6 +44,12 @@ configureFontLimits({
|
||||
|
||||
configureSvgLimits({ maxSize: config.svgMaxSize });
|
||||
|
||||
configureSvgPool({
|
||||
workers: config.svgWorkers,
|
||||
maxOldGenerationSizeMb: config.svgWorkerMaxOldMb,
|
||||
timeout: config.svgTimeout,
|
||||
});
|
||||
|
||||
configureUploadLimits({ maxFileSize: config.maxFileSize, memoryThreshold: config.memoryThreshold });
|
||||
|
||||
const queueMiddleware = createQueueMiddleware(config.maxConcurrentRequests);
|
||||
@ -61,4 +68,12 @@ app.listen(config.port, config.host, () => {
|
||||
logger.info(`media-processor listening on ${config.host}:${config.port}`);
|
||||
});
|
||||
|
||||
// Terminate the SVG workers on shutdown so in-flight sanitizations fail fast
|
||||
// instead of keeping the pool's timers alive until the process dies.
|
||||
for (const signal of ["SIGTERM", "SIGINT"] as const) {
|
||||
process.on(signal, () => {
|
||||
void shutdownSvgPool().finally(() => process.exit(0));
|
||||
});
|
||||
}
|
||||
|
||||
export { app };
|
||||
|
||||
@ -27,7 +27,7 @@ export function createSvgRoutes(): IRouter {
|
||||
}
|
||||
|
||||
res.locals.opMeta = `size=${req.file!.size}`;
|
||||
const clean = sanitizeSvg((await getFileBuffer(req.file!)).toString("utf8"));
|
||||
const clean = await sanitizeSvg((await getFileBuffer(req.file!)).toString("utf8"));
|
||||
|
||||
res.setHeader("Content-Type", "image/svg+xml");
|
||||
res.send(Buffer.from(clean, "utf8"));
|
||||
|
||||
@ -1,11 +1,15 @@
|
||||
import { isMainThread } from "node:worker_threads";
|
||||
import createDOMPurify, { type WindowLike } from "dompurify";
|
||||
import { JSDOM } from "jsdom";
|
||||
import { throwValidation, throwRestriction } from "./errors.js";
|
||||
import { throwValidation, throwRestriction, throwProcessing } from "./errors.js";
|
||||
import { ProcessingError } from "../middleware/error-handler.js";
|
||||
import { createLogger } from "../logger.js";
|
||||
import { getSvgPoolOptions, runInSvgPool } from "../svg-pool.js";
|
||||
|
||||
const logger = createLogger("svg");
|
||||
const SVG_NAMESPACE = "http://www.w3.org/2000/svg";
|
||||
|
||||
let svgMaxSize = 30 * 1024 * 1024;
|
||||
let svgMaxSize = 2 * 1024 * 1024;
|
||||
|
||||
export function configureSvgLimits(opts: { maxSize: number }): void {
|
||||
svgMaxSize = opts.maxSize;
|
||||
@ -20,7 +24,7 @@ export function getSvgMaxSize(): number {
|
||||
// the sanitizer return nothing, so they are stripped first. Stripping the
|
||||
// DOCTYPE is also what keeps its entities from ever being declared. A prolog
|
||||
// the stripper misses still fails closed: DOMPurify returns an empty document
|
||||
// and `sanitizeSvg` rejects it.
|
||||
// and `sanitizeSvgSync` rejects it.
|
||||
function stripProlog(svg: string): string {
|
||||
return svg
|
||||
.replace(/^\uFEFF/, "")
|
||||
@ -32,8 +36,8 @@ function stripProlog(svg: string): string {
|
||||
const SVG_ROOT = /^<svg[\s/>]/i;
|
||||
|
||||
/**
|
||||
* Sanitize an SVG document with DOMPurify over jsdom and return the clean
|
||||
* SVG as UTF-8 text.
|
||||
* Sanitize an SVG document synchronously with DOMPurify over jsdom and return
|
||||
* the clean SVG as UTF-8 text.
|
||||
*
|
||||
* DOMPurify works from an allowlist: every element and attribute it does not
|
||||
* explicitly allow is removed, so it fails closed. It is given the SVG profile
|
||||
@ -44,11 +48,16 @@ const SVG_ROOT = /^<svg[\s/>]/i;
|
||||
* DOMPurify keeps state on its window, and reusing one window in a long-lived
|
||||
* process leaks memory and degrades latency without bound.
|
||||
*
|
||||
* This is the CPU-bound core. The HTTP route does not call it directly: it runs
|
||||
* in a `worker_threads` worker through `sanitizeSvg`, so the service's event
|
||||
* loop is never blocked by it.
|
||||
*
|
||||
* Throws a `ProcessingError` (413 `svg-too-large`) when the input exceeds the
|
||||
* configured limit, and (400 `invalid-svg-file`) when the input is not a
|
||||
* well-formed SVG document.
|
||||
* configured limit, (400 `invalid-svg-file`) when the input is not a
|
||||
* well-formed SVG document, and (503 `svg-sanitization-failed`) when anything
|
||||
* else goes wrong.
|
||||
*/
|
||||
export function sanitizeSvg(svg: string): string {
|
||||
export function sanitizeSvgSync(svg: string): string {
|
||||
const size = Buffer.byteLength(svg, "utf8");
|
||||
if (size > svgMaxSize) {
|
||||
throwRestriction("svg-too-large", `SVG size ${size} exceeds the maximum of ${svgMaxSize} bytes`);
|
||||
@ -71,8 +80,30 @@ export function sanitizeSvg(svg: string): string {
|
||||
if (err instanceof ProcessingError) {
|
||||
throw err;
|
||||
}
|
||||
throwValidation("invalid-svg-file", `Failed to sanitize svg: ${(err as Error).message}`);
|
||||
// A failure of the sanitizer itself (a bug, an OOM, a DOMPurify regression)
|
||||
// is not the user's file being invalid: report it as a service failure and
|
||||
// keep the technical detail out of the hint. Inside a worker this is not
|
||||
// logged: the pool logs it on the main thread when the result comes back,
|
||||
// so a worker never initializes the (transport-spawning) logger.
|
||||
if (isMainThread) {
|
||||
logger.error({ err }, "unexpected svg sanitization failure");
|
||||
}
|
||||
throwProcessing("svg-sanitization-failed", "SVG sanitization failed unexpectedly");
|
||||
} finally {
|
||||
dom.window.close();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitize an SVG document, off the event loop when a worker pool is
|
||||
* configured. Falls back to running `sanitizeSvgSync` inline when the pool is
|
||||
* disabled (`PENPOT_MEDIA_PROCESSOR_SVG_WORKERS=0`) or not configured, which is
|
||||
* what tests use.
|
||||
*/
|
||||
export function sanitizeSvg(svg: string): Promise<string> {
|
||||
const options = getSvgPoolOptions();
|
||||
if (options === null || options.workers <= 0) {
|
||||
return Promise.resolve().then(() => sanitizeSvgSync(svg));
|
||||
}
|
||||
return runInSvgPool(svg);
|
||||
}
|
||||
|
||||
228
media-processor/src/svg-pool.ts
Normal file
228
media-processor/src/svg-pool.ts
Normal file
@ -0,0 +1,228 @@
|
||||
import { Worker } from "node:worker_threads";
|
||||
import { ProcessingError } from "./middleware/error-handler.js";
|
||||
import { createLogger } from "./logger.js";
|
||||
import type { AppError } from "./types.js";
|
||||
|
||||
const logger = createLogger("svg-pool");
|
||||
|
||||
/**
|
||||
* The slice of `worker_threads.Worker` the pool uses. Keeping it narrow lets
|
||||
* tests inject a fake worker and exercise the queue, the timeout and the
|
||||
* failure handling without real threads.
|
||||
*/
|
||||
export interface SvgWorker {
|
||||
postMessage(value: unknown): void;
|
||||
terminate(): Promise<number>;
|
||||
on(event: "message", listener: (value: unknown) => void): this;
|
||||
on(event: "error", listener: (err: Error) => void): this;
|
||||
on(event: "exit", listener: (code: number) => void): this;
|
||||
}
|
||||
|
||||
export interface SvgPoolOptions {
|
||||
workers: number;
|
||||
maxOldGenerationSizeMb: number;
|
||||
timeout: number;
|
||||
}
|
||||
|
||||
interface Job {
|
||||
svg: string;
|
||||
resolve: (clean: string) => void;
|
||||
reject: (err: Error) => void;
|
||||
timer: NodeJS.Timeout | null;
|
||||
}
|
||||
|
||||
interface PooledWorker {
|
||||
worker: SvgWorker;
|
||||
job: Job | null;
|
||||
terminating: boolean;
|
||||
}
|
||||
|
||||
interface PoolState {
|
||||
options: SvgPoolOptions;
|
||||
workers: PooledWorker[];
|
||||
queue: Job[];
|
||||
stopped: boolean;
|
||||
}
|
||||
|
||||
type SvgWorkerResult = { ok: true; clean: string } | { ok: false; statusCode: number; body: AppError };
|
||||
|
||||
let state: PoolState | null = null;
|
||||
let createWorker: () => SvgWorker = defaultWorkerFactory;
|
||||
|
||||
function workerUrl(): URL {
|
||||
// Under `tsx` the module is a `.ts` source and the worker must be loaded as
|
||||
// `.ts`; in the bundle it is the emitted `.js`. `import.meta.url` tells them
|
||||
// apart.
|
||||
const extension = import.meta.url.endsWith(".ts") ? "ts" : "js";
|
||||
return new URL(`./svg-worker.${extension}`, import.meta.url);
|
||||
}
|
||||
|
||||
function defaultWorkerFactory(): SvgWorker {
|
||||
const options = state!.options;
|
||||
return new Worker(workerUrl(), {
|
||||
// Node inherits the parent flags by default; setting it explicitly keeps
|
||||
// the dev loader (`tsx`) available in the worker.
|
||||
execArgv: process.execArgv,
|
||||
resourceLimits: { maxOldGenerationSizeMb: options.maxOldGenerationSizeMb },
|
||||
}) as unknown as SvgWorker;
|
||||
}
|
||||
|
||||
/** Replace the worker factory. Tests use it to inject a fake; `null` restores the real one. */
|
||||
export function setSvgWorkerFactory(factory: (() => SvgWorker) | null): void {
|
||||
createWorker = factory ?? defaultWorkerFactory;
|
||||
}
|
||||
|
||||
/** Create the pool. `workers <= 0` disables it (the sanitizer runs inline). */
|
||||
export function configureSvgPool(options: SvgPoolOptions): void {
|
||||
if (state !== null) {
|
||||
throw new Error("svg pool is already configured");
|
||||
}
|
||||
state = { options, workers: [], queue: [], stopped: false };
|
||||
for (let i = 0; i < options.workers; i++) {
|
||||
state.workers.push(spawnWorker());
|
||||
}
|
||||
}
|
||||
|
||||
export function getSvgPoolOptions(): SvgPoolOptions | null {
|
||||
return state?.options ?? null;
|
||||
}
|
||||
|
||||
/** Queue an SVG for sanitization in a worker. */
|
||||
export function runInSvgPool(svg: string): Promise<string> {
|
||||
const current = state;
|
||||
if (current === null || current.stopped) {
|
||||
return Promise.reject(stoppedError());
|
||||
}
|
||||
return new Promise<string>((resolve, reject) => {
|
||||
current.queue.push({ svg, resolve, reject, timer: null });
|
||||
dispatch();
|
||||
});
|
||||
}
|
||||
|
||||
/** Terminate every worker and reject whatever is pending. */
|
||||
export async function shutdownSvgPool(): Promise<void> {
|
||||
const current = state;
|
||||
if (current === null) {
|
||||
return;
|
||||
}
|
||||
current.stopped = true;
|
||||
state = null;
|
||||
|
||||
for (const job of current.queue.splice(0)) {
|
||||
job.reject(stoppedError());
|
||||
}
|
||||
|
||||
const workers = current.workers;
|
||||
current.workers = [];
|
||||
await Promise.all(
|
||||
workers.map((pooled) => {
|
||||
pooled.terminating = true;
|
||||
const job = pooled.job;
|
||||
pooled.job = null;
|
||||
if (job !== null) {
|
||||
clearJobTimer(job);
|
||||
job.reject(stoppedError());
|
||||
}
|
||||
return pooled.worker.terminate().catch(() => 0);
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
function spawnWorker(): PooledWorker {
|
||||
const worker = createWorker();
|
||||
const pooled: PooledWorker = { worker, job: null, terminating: false };
|
||||
worker.on("message", (value) => onMessage(pooled, value));
|
||||
worker.on("error", (err) => onWorkerError(pooled, err));
|
||||
worker.on("exit", (code) => onWorkerExit(pooled, code));
|
||||
return pooled;
|
||||
}
|
||||
|
||||
function dispatch(): void {
|
||||
const current = state;
|
||||
if (current === null || current.stopped) {
|
||||
return;
|
||||
}
|
||||
while (current.queue.length > 0) {
|
||||
const pooled = current.workers.find((w) => w.job === null && !w.terminating);
|
||||
if (pooled === undefined) {
|
||||
return;
|
||||
}
|
||||
const job = current.queue.shift()!;
|
||||
pooled.job = job;
|
||||
job.timer = setTimeout(() => {
|
||||
job.timer = null;
|
||||
failWorker(pooled, new ProcessingError(503, { type: "internal", code: "svg-timeout" }));
|
||||
}, current.options.timeout);
|
||||
pooled.worker.postMessage(job.svg);
|
||||
}
|
||||
}
|
||||
|
||||
function onMessage(pooled: PooledWorker, value: unknown): void {
|
||||
const job = pooled.job;
|
||||
if (job === null) {
|
||||
return;
|
||||
}
|
||||
clearJobTimer(job);
|
||||
pooled.job = null;
|
||||
|
||||
const result = value as SvgWorkerResult;
|
||||
if (result.ok) {
|
||||
job.resolve(result.clean);
|
||||
} else {
|
||||
if (result.body.code === "svg-sanitization-failed") {
|
||||
logger.error("svg worker reported an unexpected sanitization failure");
|
||||
}
|
||||
job.reject(new ProcessingError(result.statusCode, result.body));
|
||||
}
|
||||
dispatch();
|
||||
}
|
||||
|
||||
function onWorkerError(pooled: PooledWorker, err: Error): void {
|
||||
logger.error({ err }, "svg worker error");
|
||||
failWorker(pooled, new ProcessingError(503, { type: "internal", code: "svg-worker-failed" }));
|
||||
}
|
||||
|
||||
function onWorkerExit(pooled: PooledWorker, code: number): void {
|
||||
if (pooled.terminating) {
|
||||
return;
|
||||
}
|
||||
logger.error({ code }, "svg worker exited unexpectedly");
|
||||
failWorker(pooled, new ProcessingError(503, { type: "internal", code: "svg-worker-exited" }));
|
||||
}
|
||||
|
||||
/** Reject the worker's in-flight job (if any) and replace the worker. */
|
||||
function failWorker(pooled: PooledWorker, err: ProcessingError): void {
|
||||
if (pooled.terminating) {
|
||||
return;
|
||||
}
|
||||
pooled.terminating = true;
|
||||
|
||||
const job = pooled.job;
|
||||
pooled.job = null;
|
||||
if (job !== null) {
|
||||
clearJobTimer(job);
|
||||
job.reject(err);
|
||||
}
|
||||
|
||||
void pooled.worker.terminate().catch(() => 0);
|
||||
|
||||
const current = state;
|
||||
if (current !== null && !current.stopped) {
|
||||
const index = current.workers.indexOf(pooled);
|
||||
if (index !== -1) {
|
||||
current.workers[index] = spawnWorker();
|
||||
}
|
||||
dispatch();
|
||||
}
|
||||
}
|
||||
|
||||
function clearJobTimer(job: Job): void {
|
||||
if (job.timer !== null) {
|
||||
clearTimeout(job.timer);
|
||||
job.timer = null;
|
||||
}
|
||||
}
|
||||
|
||||
function stoppedError(): ProcessingError {
|
||||
return new ProcessingError(503, { type: "internal", code: "svg-pool-stopped" });
|
||||
}
|
||||
27
media-processor/src/svg-worker.ts
Normal file
27
media-processor/src/svg-worker.ts
Normal file
@ -0,0 +1,27 @@
|
||||
import { parentPort } from "node:worker_threads";
|
||||
import { sanitizeSvgSync } from "./services/svg.js";
|
||||
import { ProcessingError } from "./middleware/error-handler.js";
|
||||
|
||||
// Entry point of the SVG sanitization worker. `svg-pool` spawns this file and
|
||||
// sends the SVG to sanitize through `postMessage`; the answer is always a
|
||||
// plain, structured-cloneable object, never a class instance.
|
||||
const port = parentPort;
|
||||
if (port === null) {
|
||||
throw new Error("svg-worker must be started as a worker thread");
|
||||
}
|
||||
|
||||
port.on("message", (svg: string) => {
|
||||
try {
|
||||
port.postMessage({ ok: true, clean: sanitizeSvgSync(svg) });
|
||||
} catch (err) {
|
||||
if (err instanceof ProcessingError) {
|
||||
port.postMessage({ ok: false, statusCode: err.statusCode, body: err.errorBody });
|
||||
} else {
|
||||
port.postMessage({
|
||||
ok: false,
|
||||
statusCode: 503,
|
||||
body: { type: "internal", code: "svg-sanitization-failed" },
|
||||
});
|
||||
}
|
||||
}
|
||||
});
|
||||
@ -11,6 +11,9 @@ export interface AppConfig {
|
||||
imageMaxWidth: number;
|
||||
imageMaxHeight: number;
|
||||
svgMaxSize: number;
|
||||
svgWorkers: number;
|
||||
svgWorkerMaxOldMb: number;
|
||||
svgTimeout: number;
|
||||
fontProcessMem: number;
|
||||
fontProcessCpuTime: number;
|
||||
fontTimeout: number;
|
||||
|
||||
@ -20,6 +20,10 @@ describe("loadConfig", () => {
|
||||
expect(config.requestTimeout).toBe(180000);
|
||||
expect(config.maxFileSize).toBe(367001600);
|
||||
expect(config.memoryThreshold).toBe(10485760);
|
||||
expect(config.svgMaxSize).toBe(2097152);
|
||||
expect(config.svgWorkers).toBe(2);
|
||||
expect(config.svgWorkerMaxOldMb).toBe(512);
|
||||
expect(config.svgTimeout).toBe(30000);
|
||||
});
|
||||
|
||||
it("accepts valid config with all fields set", () => {
|
||||
@ -101,6 +105,22 @@ describe("loadConfig", () => {
|
||||
expect(() => loadConfig()).toThrow();
|
||||
});
|
||||
|
||||
it("rejects negative svg max size", () => {
|
||||
process.env.PENPOT_MEDIA_PROCESSOR_SVG_MAX_SIZE = "-1";
|
||||
expect(() => loadConfig()).toThrow();
|
||||
});
|
||||
|
||||
it("accepts svg workers=0 (inline mode)", () => {
|
||||
process.env.PENPOT_MEDIA_PROCESSOR_SVG_WORKERS = "0";
|
||||
const config = loadConfig();
|
||||
expect(config.svgWorkers).toBe(0);
|
||||
});
|
||||
|
||||
it("rejects negative svg worker heap", () => {
|
||||
process.env.PENPOT_MEDIA_PROCESSOR_SVG_WORKER_MAX_OLD_MB = "-1";
|
||||
expect(() => loadConfig()).toThrow();
|
||||
});
|
||||
|
||||
it("rejects negative font process mem", () => {
|
||||
process.env.PENPOT_MEDIA_PROCESSOR_FONT_PROCESS_MEM = "-512";
|
||||
expect(() => loadConfig()).toThrow();
|
||||
|
||||
170
media-processor/test/svg-pool.test.ts
Normal file
170
media-processor/test/svg-pool.test.ts
Normal file
@ -0,0 +1,170 @@
|
||||
import { describe, it, expect, afterEach, vi } from "vitest";
|
||||
import { EventEmitter } from "node:events";
|
||||
import {
|
||||
configureSvgPool,
|
||||
shutdownSvgPool,
|
||||
runInSvgPool,
|
||||
setSvgWorkerFactory,
|
||||
type SvgWorker,
|
||||
} from "../src/svg-pool.js";
|
||||
import { ProcessingError } from "../src/middleware/error-handler.js";
|
||||
|
||||
class FakeWorker extends EventEmitter {
|
||||
public readonly posted: unknown[] = [];
|
||||
public terminated = false;
|
||||
|
||||
public postMessage(value: unknown): void {
|
||||
this.posted.push(value);
|
||||
}
|
||||
|
||||
public async terminate(): Promise<number> {
|
||||
this.terminated = true;
|
||||
this.emit("exit", 0);
|
||||
return 0;
|
||||
}
|
||||
|
||||
public reply(value: unknown): void {
|
||||
this.emit("message", value);
|
||||
}
|
||||
|
||||
public fail(err: Error): void {
|
||||
this.emit("error", err);
|
||||
}
|
||||
|
||||
public exitUnexpectedly(code = 1): void {
|
||||
this.emit("exit", code);
|
||||
}
|
||||
}
|
||||
|
||||
let created: FakeWorker[] = [];
|
||||
|
||||
function installFakeFactory(): void {
|
||||
created = [];
|
||||
setSvgWorkerFactory(() => {
|
||||
const worker = new FakeWorker();
|
||||
created.push(worker);
|
||||
return worker as unknown as SvgWorker;
|
||||
});
|
||||
}
|
||||
|
||||
async function rejection(promise: Promise<unknown>): Promise<ProcessingError> {
|
||||
try {
|
||||
await promise;
|
||||
} catch (err) {
|
||||
return err as ProcessingError;
|
||||
}
|
||||
throw new Error("expected the promise to reject");
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
await shutdownSvgPool();
|
||||
setSvgWorkerFactory(null);
|
||||
});
|
||||
|
||||
describe("svg pool: dispatch", () => {
|
||||
it("runs jobs on idle workers and queues the rest", async () => {
|
||||
installFakeFactory();
|
||||
configureSvgPool({ workers: 2, maxOldGenerationSizeMb: 512, timeout: 1000 });
|
||||
|
||||
const first = runInSvgPool("<svg>1</svg>");
|
||||
const second = runInSvgPool("<svg>2</svg>");
|
||||
expect(created).toHaveLength(2);
|
||||
expect(created[0].posted).toEqual(["<svg>1</svg>"]);
|
||||
expect(created[1].posted).toEqual(["<svg>2</svg>"]);
|
||||
|
||||
const third = runInSvgPool("<svg>3</svg>");
|
||||
expect(created[0].posted).toHaveLength(1);
|
||||
|
||||
created[0].reply({ ok: true, clean: "A" });
|
||||
await expect(first).resolves.toBe("A");
|
||||
expect(created[0].posted).toEqual(["<svg>1</svg>", "<svg>3</svg>"]);
|
||||
|
||||
created[1].reply({ ok: true, clean: "B" });
|
||||
created[0].reply({ ok: true, clean: "C" });
|
||||
await expect(second).resolves.toBe("B");
|
||||
await expect(third).resolves.toBe("C");
|
||||
});
|
||||
|
||||
it("rejects with the error body the worker reports", async () => {
|
||||
installFakeFactory();
|
||||
configureSvgPool({ workers: 1, maxOldGenerationSizeMb: 512, timeout: 1000 });
|
||||
|
||||
const job = runInSvgPool("<svg/>");
|
||||
created[0].reply({
|
||||
ok: false,
|
||||
statusCode: 400,
|
||||
body: { type: "validation", code: "invalid-svg-file" },
|
||||
});
|
||||
|
||||
const err = await rejection(job);
|
||||
expect(err).toBeInstanceOf(ProcessingError);
|
||||
expect(err.statusCode).toBe(400);
|
||||
expect(err.errorBody.code).toBe("invalid-svg-file");
|
||||
});
|
||||
});
|
||||
|
||||
describe("svg pool: worker failure", () => {
|
||||
it("rejects the job and respawns when a worker errors", async () => {
|
||||
installFakeFactory();
|
||||
configureSvgPool({ workers: 1, maxOldGenerationSizeMb: 512, timeout: 1000 });
|
||||
|
||||
const job = runInSvgPool("<svg/>");
|
||||
created[0].fail(new Error("boom"));
|
||||
|
||||
const err = await rejection(job);
|
||||
expect(err.statusCode).toBe(503);
|
||||
expect(err.errorBody.code).toBe("svg-worker-failed");
|
||||
expect(created).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("rejects the job and respawns when a worker exits unexpectedly", async () => {
|
||||
installFakeFactory();
|
||||
configureSvgPool({ workers: 1, maxOldGenerationSizeMb: 512, timeout: 1000 });
|
||||
|
||||
const job = runInSvgPool("<svg/>");
|
||||
created[0].exitUnexpectedly();
|
||||
|
||||
const err = await rejection(job);
|
||||
expect(err.statusCode).toBe(503);
|
||||
expect(err.errorBody.code).toBe("svg-worker-exited");
|
||||
expect(created).toHaveLength(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe("svg pool: timeout", () => {
|
||||
it("terminates and respawns a worker that does not answer in time", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
installFakeFactory();
|
||||
configureSvgPool({ workers: 1, maxOldGenerationSizeMb: 512, timeout: 1000 });
|
||||
|
||||
const job = runInSvgPool("<svg/>");
|
||||
const settled = rejection(job);
|
||||
await vi.advanceTimersByTimeAsync(1000);
|
||||
|
||||
const err = await settled;
|
||||
expect(err.statusCode).toBe(503);
|
||||
expect(err.errorBody.code).toBe("svg-timeout");
|
||||
expect(created[0].terminated).toBe(true);
|
||||
expect(created).toHaveLength(2);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("svg pool: shutdown", () => {
|
||||
it("rejects pending jobs and terminates the workers", async () => {
|
||||
installFakeFactory();
|
||||
configureSvgPool({ workers: 1, maxOldGenerationSizeMb: 512, timeout: 1000 });
|
||||
|
||||
const job = runInSvgPool("<svg/>");
|
||||
const settled = rejection(job);
|
||||
await shutdownSvgPool();
|
||||
|
||||
const err = await settled;
|
||||
expect(err.statusCode).toBe(503);
|
||||
expect(err.errorBody.code).toBe("svg-pool-stopped");
|
||||
expect(created[0].terminated).toBe(true);
|
||||
});
|
||||
});
|
||||
@ -1,7 +1,7 @@
|
||||
import { describe, it, expect, beforeAll, afterAll, afterEach } from "vitest";
|
||||
import request from "supertest";
|
||||
import express from "express";
|
||||
import { sanitizeSvg, configureSvgLimits } from "../src/services/svg.js";
|
||||
import { sanitizeSvg, sanitizeSvgSync, configureSvgLimits } from "../src/services/svg.js";
|
||||
import { createSvgRoutes } from "../src/routes/svg.js";
|
||||
import { errorHandler, ProcessingError } from "../src/middleware/error-handler.js";
|
||||
import { timeoutMiddleware } from "../src/middleware/timeout.js";
|
||||
@ -10,7 +10,7 @@ import { createQueueMiddleware } from "../src/middleware/queue.js";
|
||||
import { configureUploadLimits } from "../src/upload.js";
|
||||
|
||||
const SVG_NS = "http://www.w3.org/2000/svg";
|
||||
const DEFAULT_MAX_SIZE = 30 * 1024 * 1024;
|
||||
const DEFAULT_MAX_SIZE = 2 * 1024 * 1024;
|
||||
|
||||
function svg(inner: string, attrs = `xmlns="${SVG_NS}" width="100" height="100"`): string {
|
||||
return `<svg ${attrs}>${inner}</svg>`;
|
||||
@ -105,7 +105,7 @@ const MALICIOUS: Array<{ name: string; svg: string; forbidden: RegExp }> = [
|
||||
|
||||
describe("sanitizeSvg: removes dangerous content", () => {
|
||||
it.each(MALICIOUS)("removes $name", ({ svg: dirty, forbidden }) => {
|
||||
const clean = sanitizeSvg(dirty);
|
||||
const clean = sanitizeSvgSync(dirty);
|
||||
expect(clean).toMatch(/^<svg[\s/>]/i);
|
||||
expect(clean).not.toMatch(forbidden);
|
||||
expectNoDangerousOutput(clean);
|
||||
@ -130,7 +130,7 @@ describe("sanitizeSvg: keeps legitimate SVG", () => {
|
||||
`<text x="1" y="9"><tspan>hi</tspan></text>`,
|
||||
`xmlns="${SVG_NS}" viewBox="0 0 10 10"`
|
||||
);
|
||||
const clean = sanitizeSvg(dirty);
|
||||
const clean = sanitizeSvgSync(dirty);
|
||||
expect(clean).toMatch(/^<svg[\s/>]/i);
|
||||
expect(clean).toContain("<linearGradient");
|
||||
expect(clean).toContain("<stop");
|
||||
@ -143,17 +143,17 @@ describe("sanitizeSvg: keeps legitimate SVG", () => {
|
||||
});
|
||||
|
||||
it("keeps a safe relative/absolute href", () => {
|
||||
const clean = sanitizeSvg(svg(`<image href="https://example.com/a.png" width="5" height="5"/>`));
|
||||
const clean = sanitizeSvgSync(svg(`<image href="https://example.com/a.png" width="5" height="5"/>`));
|
||||
expect(clean).toContain('href="https://example.com/a.png"');
|
||||
});
|
||||
|
||||
it("accepts a document with a leading UTF-8 BOM", () => {
|
||||
const clean = sanitizeSvg(`\uFEFF${svg(`<rect width="5" height="5"/>`)}`);
|
||||
const clean = sanitizeSvgSync(`\uFEFF${svg(`<rect width="5" height="5"/>`)}`);
|
||||
expect(clean).toMatch(/^<svg[\s/>]/i);
|
||||
});
|
||||
|
||||
it("accepts a document with an XML declaration and leading whitespace", () => {
|
||||
const clean = sanitizeSvg(
|
||||
const clean = sanitizeSvgSync(
|
||||
`<?xml version="1.0" encoding="UTF-8"?>\n<svg xmlns="${SVG_NS}"><rect width="5" height="5"/></svg>`
|
||||
);
|
||||
expect(clean).toMatch(/^<svg[\s/>]/i);
|
||||
@ -162,14 +162,14 @@ describe("sanitizeSvg: keeps legitimate SVG", () => {
|
||||
|
||||
it("accepts a document with a DOCTYPE", () => {
|
||||
const dirty = `<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">${svg(`<rect width="5" height="5"/>`)}`;
|
||||
const clean = sanitizeSvg(dirty);
|
||||
const clean = sanitizeSvgSync(dirty);
|
||||
expect(clean).toMatch(/^<svg[\s/>]/i);
|
||||
expect(clean).toContain("<rect");
|
||||
});
|
||||
|
||||
it("does not resolve entities declared in a DOCTYPE internal subset", () => {
|
||||
const dirty = `<!DOCTYPE svg [<!ENTITY xxe SYSTEM "file:///etc/passwd">]>${svg(`<rect width="5" height="5"/>`)}`;
|
||||
const clean = sanitizeSvg(dirty);
|
||||
const clean = sanitizeSvgSync(dirty);
|
||||
expect(clean).not.toContain("passwd");
|
||||
expect(clean).not.toContain("&xxe;");
|
||||
});
|
||||
@ -190,7 +190,7 @@ describe("sanitizeSvg: rejects invalid input", () => {
|
||||
];
|
||||
|
||||
it.each(INVALID)("rejects $name with invalid-svg-file", ({ svg: dirty }) => {
|
||||
expectProcessingError(() => sanitizeSvg(dirty), 400, "invalid-svg-file");
|
||||
expectProcessingError(() => sanitizeSvgSync(dirty), 400, "invalid-svg-file");
|
||||
});
|
||||
});
|
||||
|
||||
@ -204,21 +204,74 @@ describe("sanitizeSvg: limits and stability", () => {
|
||||
it("rejects an input larger than the configured maximum", () => {
|
||||
configureSvgLimits({ maxSize: 100 });
|
||||
const big = svg(`<rect width="5" height="5"/>${"<!-- pad -->".repeat(20)}`);
|
||||
expectProcessingError(() => sanitizeSvg(big), 413, "svg-too-large");
|
||||
expectProcessingError(() => sanitizeSvgSync(big), 413, "svg-too-large");
|
||||
});
|
||||
|
||||
it("stays stable over many consecutive calls", () => {
|
||||
const dirty = svg(`<rect width="5" height="5"/>`);
|
||||
for (let i = 0; i < 150; i++) {
|
||||
expect(sanitizeSvg(dirty)).toMatch(/^<svg[\s/>]/i);
|
||||
expect(sanitizeSvgSync(dirty)).toMatch(/^<svg[\s/>]/i);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// sanitizeSvg: async wrapper and internal failures
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("sanitizeSvg: async wrapper", () => {
|
||||
it("runs inline when no pool is configured", async () => {
|
||||
const clean = await sanitizeSvg(svg(`<script>alert("xss")</script><rect width="5" height="5"/>`));
|
||||
expect(clean).toMatch(/^<svg[\s/>]/i);
|
||||
expect(clean).not.toMatch(/<script/i);
|
||||
});
|
||||
});
|
||||
|
||||
describe("sanitizeSvgSync: internal failures", () => {
|
||||
it("maps an unexpected failure to 503 svg-sanitization-failed", () => {
|
||||
// A Uint8Array passes the byte-length check and then breaks `stripProlog`
|
||||
// (it has no `.replace`), which is not a ProcessingError.
|
||||
const err = captureError(() => sanitizeSvgSync(new Uint8Array([60, 115]) as unknown as string));
|
||||
expect(err).toBeInstanceOf(ProcessingError);
|
||||
const processingError = err as ProcessingError;
|
||||
expect(processingError.statusCode).toBe(503);
|
||||
expect(processingError.errorBody.code).toBe("svg-sanitization-failed");
|
||||
expect(processingError.errorBody.hint ?? "").not.toContain("replace");
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// sanitizeSvg: cross-mode corpus
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("sanitizeSvgSync: corpus decisions", () => {
|
||||
it("keeps a data: URI in an image href (pins the cross-mode behavior)", () => {
|
||||
const dirty = svg(`<image href="data:image/png;base64,iVBORw0KGgo=" width="5" height="5"/>`);
|
||||
const clean = sanitizeSvgSync(dirty);
|
||||
// Observed with DOMPurify 3.4.x and the svg profile: data: URIs survive.
|
||||
expect(clean).toContain("data:image/png;base64");
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// POST /api/svg/sanitize
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
// Build the multipart body exactly like the JVM backend does in
|
||||
// `app.media.remote/build-multipart-stream`: field `file`, filename `file`,
|
||||
// the part content type as the declared mtype, and the `----PenpotBoundary<ms>`
|
||||
// boundary. This pins the wire contract from the service side.
|
||||
function backendMultipartBody(boundary: string, svgText: string): string {
|
||||
return (
|
||||
`--${boundary}\r\n` +
|
||||
`Content-Disposition: form-data; name="file"; filename="file"\r\n` +
|
||||
`Content-Type: image/svg+xml\r\n` +
|
||||
`\r\n` +
|
||||
svgText +
|
||||
`\r\n--${boundary}--\r\n`
|
||||
);
|
||||
}
|
||||
|
||||
function createTestApp() {
|
||||
const app = express();
|
||||
// Low threshold to exercise the disk-storage path of the upload.
|
||||
@ -302,4 +355,35 @@ describe("POST /api/svg/sanitize", () => {
|
||||
configureSvgLimits({ maxSize: DEFAULT_MAX_SIZE });
|
||||
}
|
||||
});
|
||||
|
||||
it("accepts the multipart body the JVM backend builds", async () => {
|
||||
const boundary = `----PenpotBoundary${Date.now()}`;
|
||||
const body = backendMultipartBody(boundary, svg(`<script>alert("xss")</script><rect width="5" height="5"/>`));
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/svg/sanitize")
|
||||
.set("x-shared-key", "test-key")
|
||||
.set("Content-Type", `multipart/form-data; boundary=${boundary}`)
|
||||
.send(Buffer.from(body, "utf8"));
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.headers["content-type"]).toContain("image/svg+xml");
|
||||
const out = Buffer.isBuffer(res.body) ? res.body.toString("utf8") : res.text;
|
||||
expect(out).toMatch(/^<svg[\s/>]/i);
|
||||
expect(out).not.toMatch(/<script/i);
|
||||
});
|
||||
|
||||
it("returns the JSON error shape the JVM backend translates", async () => {
|
||||
const boundary = `----PenpotBoundary${Date.now()}`;
|
||||
const body = backendMultipartBody(boundary, "this is not an svg");
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/svg/sanitize")
|
||||
.set("x-shared-key", "test-key")
|
||||
.set("Content-Type", `multipart/form-data; boundary=${boundary}`)
|
||||
.send(Buffer.from(body, "utf8"));
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body).toEqual(expect.objectContaining({ type: "validation", code: "invalid-svg-file" }));
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user