Refuse SVG sources and never save an unknown input as JPEG (closes #68) #240

Merged
clawbot merged 5 commits from issue-68-refuse-svg into next 2026-10-13 07:13:18 +02:00
Collaborator

pixa no longer accepts SVG sources, and orig no longer turns a source it cannot name into a JPEG.

  • The upstream fetch no longer accepts image/svg+xml. libvips renders SVG, so an SVG source was rasterized, and orig of one came back as image/jpeg.
  • Before decoding, the image processor names the input's format from its first bytes with govips' DetermineImageType, which is how govips picks the libvips loader, and refuses with ErrUnsupportedInputFormat any input whose first bytes it does not name as one of the six source formats. An SVG whose first bytes pass the first-bytes check for AVIF (an XML comment holding ftypavis) is refused there, before librsvg reads it. A damaged image in a source format is not: it is decoded and answers 500.
  • Both image routes answer 415 unsupported source format for that, and when the upstream's Content-Type is not one of the source formats.
  • formatFromString refuses a format it cannot name, where it returned JPEG; it makes the refusal above, and orig is the format it named.
  • README.md drops SVG from the source formats and lists the 415.

Each fix follows its failing tests in commit order. No existing test is changed.

Disclosures:

  • SVG detection stays in internal/magic, unused on the request path, for #168.
  • Judgement call: every upstream Content-Type outside the accepted list now answers 415 where it answered 500.
  • An SVG failing the first-bytes check for the type it is sent under still answers 500.
  • An AVIF image sequence (brand avis), whose first bytes govips names as no format, now answers 415 where it answered 500; README.md does not name that case.

Closes #68

Model: opus-5-5

pixa no longer accepts SVG sources, and `orig` no longer turns a source it cannot name into a JPEG. - The upstream fetch no longer accepts `image/svg+xml`. libvips renders SVG, so an SVG source was rasterized, and `orig` of one came back as `image/jpeg`. - Before decoding, the image processor names the input's format from its first bytes with govips' `DetermineImageType`, which is how govips picks the libvips loader, and refuses with `ErrUnsupportedInputFormat` any input whose first bytes it does not name as one of the six source formats. An SVG whose first bytes pass the first-bytes check for AVIF (an XML comment holding `ftypavis`) is refused there, before librsvg reads it. A damaged image in a source format is not: it is decoded and answers 500. - Both image routes answer 415 `unsupported source format` for that, and when the upstream's `Content-Type` is not one of the source formats. - `formatFromString` refuses a format it cannot name, where it returned JPEG; it makes the refusal above, and `orig` is the format it named. - `README.md` drops SVG from the source formats and lists the 415. Each fix follows its failing tests in commit order. No existing test is changed. Disclosures: - SVG detection stays in `internal/magic`, unused on the request path, for https://git.eeqj.de/sneak/pixa/issues/168. - Judgement call: every upstream `Content-Type` outside the accepted list now answers 415 where it answered 500. - An SVG failing the first-bytes check for the type it is sent under still answers 500. - An AVIF image sequence (brand `avis`), whose first bytes govips names as no format, now answers 415 where it answered 500; `README.md` does not name that case. Closes https://git.eeqj.de/sneak/pixa/issues/68 Model: opus-5-5
clawbot added the needs-review label 2026-10-13 04:43:15 +02:00
clawbot self-assigned this 2026-10-13 04:43:15 +02:00
clawbot added 2 commits 2026-10-13 04:43:15 +02:00
Failing tests, ahead of the fix: an upstream answering image/svg+xml is
answered 415 on the image route, also for orig; both image routes answer
415 for an unaccepted upstream content type and for orig of an input the
processor cannot name; and the processor refuses orig of a TIFF instead
of saving it as JPEG.

Model: opus-5-5
The fetcher no longer accepts image/svg+xml, so an SVG source gets the
refusal every unaccepted upstream type gets, which both image routes now
answer with 415 where they answered 500. orig of an input the processor
cannot name is ErrUnsupportedInputFormat, also answered 415, where it
was saved as JPEG. README.md drops SVG from the source formats and lists
the 415.

Model: opus-5-5
Author
Collaborator

FAIL: needs-rework

  1. internal/imageprocessor/imageprocessor.go, Process: an SVG still gets rasterized. An upstream can answer Content-Type: image/avif with an SVG document that starts with an XML comment holding ftypavis, so that bytes 4 to 11 read ftypavis. It passes the fetcher, and it passes the first-bytes check, because internal/magic accepts the AVIF brand avis. govips does not read avis as AVIF, so it loads the bytes as SVG and librsvg renders them. A png or jpeg request then gets 200 with the rendered image, and orig gets 415 only after rendering. So untrusted XML still reaches librsvg, which is what #68 removes. Acceptable: before decoding, the processor refuses any input that libvips would not load as one of the six source formats, SVG included, and both routes answer that with the same 415. A new test sends this input and gets 415, for a resized format and for orig.

  2. PR body and the new TODO.md entry: "An SVG sent under another image type still fails the first-bytes check and answers 500" is wrong for the input in 1. So is the PR body's claim that no accepted source reaches formatFromString without a name. Acceptable: after the fix in 1, both texts say what the tree does.

Judgement call: the test commit does not compile on its own, because it names ErrUnsupportedInputFormat before the fix adds it. I counted it as failing first, since each new test fails on its own assertion once that error exists.
Judgement call: I took 415 for every upstream Content-Type outside the accepted list as right, in line with the 415 the issue asks for SVG.

Model: opus-5-5

FAIL: needs-rework 1. `internal/imageprocessor/imageprocessor.go`, `Process`: an SVG still gets rasterized. An upstream can answer `Content-Type: image/avif` with an SVG document that starts with an XML comment holding `ftypavis`, so that bytes 4 to 11 read `ftypavis`. It passes the fetcher, and it passes the first-bytes check, because `internal/magic` accepts the AVIF brand `avis`. govips does not read `avis` as AVIF, so it loads the bytes as SVG and librsvg renders them. A `png` or `jpeg` request then gets 200 with the rendered image, and `orig` gets 415 only after rendering. So untrusted XML still reaches librsvg, which is what https://git.eeqj.de/sneak/pixa/issues/68 removes. Acceptable: before decoding, the processor refuses any input that libvips would not load as one of the six source formats, SVG included, and both routes answer that with the same 415. A new test sends this input and gets 415, for a resized format and for `orig`. 2. PR body and the new `TODO.md` entry: "An SVG sent under another image type still fails the first-bytes check and answers 500" is wrong for the input in 1. So is the PR body's claim that no accepted source reaches `formatFromString` without a name. Acceptable: after the fix in 1, both texts say what the tree does. Judgement call: the test commit does not compile on its own, because it names `ErrUnsupportedInputFormat` before the fix adds it. I counted it as failing first, since each new test fails on its own assertion once that error exists. Judgement call: I took 415 for every upstream `Content-Type` outside the accepted list as right, in line with the 415 the issue asks for SVG. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-13 05:09:52 +02:00
clawbot added 2 commits 2026-10-13 05:20:58 +02:00
An SVG starting with an XML comment holding "ftypavis" passes the
first-bytes check for AVIF; resized as PNG, it is answered 200 with the
SVG rendered. The new server test asks for it through both image routes,
resized and as orig, and wants 415 for each. The new processor test
checks that an image in each of the six source formats loads and is saved
again in its own format.

Model: opus-5-5
govips picks the libvips loader for the input with DetermineImageType.
The image processor now names the format from that type before decoding,
and refuses with ErrUnsupportedInputFormat, answered 415, any input it
cannot name as one of the six source formats. An SVG whose first bytes
pass the first-bytes check for AVIF therefore never reaches librsvg.
orig is the format named there. README.md and TODO.md say so.

Model: opus-5-5
clawbot force-pushed issue-68-refuse-svg from 69dd0143e8 to 6241c6a425 2026-10-13 05:20:58 +02:00 Compare
Author
Collaborator

Fixed: before decoding, the image processor names the input's format from govips' DetermineImageType, by which govips picks the libvips loader, and refuses with ErrUnsupportedInputFormat, answered 415 by both routes, any input that is not one of the six source formats. New tests send the SVG with ftypavis as image/avif through both routes, resized as PNG and as orig, and load each of the six formats.
Fixed: the PR body and the TODO.md entry now say that such an SVG gets 415 from the image processor, that an SVG failing the first-bytes check answers 500, and that formatFromString makes the refusal before decoding.

Model: opus-5-5

Fixed: before decoding, the image processor names the input's format from govips' `DetermineImageType`, by which govips picks the libvips loader, and refuses with `ErrUnsupportedInputFormat`, answered 415 by both routes, any input that is not one of the six source formats. New tests send the SVG with `ftypavis` as `image/avif` through both routes, resized as PNG and as `orig`, and load each of the six formats. Fixed: the PR body and the `TODO.md` entry now say that such an SVG gets 415 from the image processor, that an SVG failing the first-bytes check answers 500, and that `formatFromString` makes the refusal before decoding. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-10-13 05:50:47 +02:00
Author
Collaborator

FAIL: needs-rework

  1. README.md (the 415 in the /v1/image/ route entry, and the paragraph on source images) and the doc comment of ErrUnsupportedInputFormat in internal/imageprocessor/imageprocessor.go: they promise the refusal and 415 for "an image libvips would not load as one of" the source formats. That also reads as a damaged image in an accepted format, such as a truncated JPEG served as image/jpeg, which libvips does not load and which answers 500. The refusal covers only input whose first bytes libvips reads as some other format. Acceptable: these three texts say that, for example "an image whose first bytes libvips reads as another format, such as an SVG whose first bytes pass the check for AVIF", and none of them promises 415 for a damaged image.

  2. internal/imageprocessor/orig_format_internal_test.go, doc comment of TestImageProcessor_OrigOfUnnamedFormatRefused: it says the processor decodes TIFF but cannot name it. Since the rework, TIFF is refused before it is decoded, whatever format is asked for. Acceptable: the comment says TIFF is refused before decoding.

Judgement call: TestImageProcessor_LoadsEachSourceFormat passes without the fix; I took it as a check that the refusal leaves the six formats loading, not as a test that must fail first.
Judgement call: the first test commit still does not compile on its own; as in the earlier review, I counted its tests as failing first, since each fails on its own assertion once ErrUnsupportedInputFormat exists.

Model: opus-5-5

FAIL: needs-rework 1. `README.md` (the 415 in the `/v1/image/` route entry, and the paragraph on source images) and the doc comment of `ErrUnsupportedInputFormat` in `internal/imageprocessor/imageprocessor.go`: they promise the refusal and 415 for "an image libvips would not load as one of" the source formats. That also reads as a damaged image in an accepted format, such as a truncated JPEG served as `image/jpeg`, which libvips does not load and which answers 500. The refusal covers only input whose first bytes libvips reads as some other format. Acceptable: these three texts say that, for example "an image whose first bytes libvips reads as another format, such as an SVG whose first bytes pass the check for AVIF", and none of them promises 415 for a damaged image. 2. `internal/imageprocessor/orig_format_internal_test.go`, doc comment of `TestImageProcessor_OrigOfUnnamedFormatRefused`: it says the processor decodes TIFF but cannot name it. Since the rework, TIFF is refused before it is decoded, whatever format is asked for. Acceptable: the comment says TIFF is refused before decoding. Judgement call: `TestImageProcessor_LoadsEachSourceFormat` passes without the fix; I took it as a check that the refusal leaves the six formats loading, not as a test that must fail first. Judgement call: the first test commit still does not compile on its own; as in the earlier review, I counted its tests as failing first, since each fails on its own assertion once `ErrUnsupportedInputFormat` exists. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-13 06:19:26 +02:00
clawbot added 1 commit 2026-10-13 06:24:26 +02:00
README.md, the doc comment of ErrUnsupportedInputFormat and the TODO.md
entry say the 415 is for input whose first bytes libvips reads as
another format, and no longer read as refusing a damaged image in a
source format, which is decoded and answers 500. The doc comment of
TestImageProcessor_OrigOfUnnamedFormatRefused says TIFF is refused
before it is decoded.

Model: opus-5-5
Author
Collaborator

Fixed: README.md (the 415 in the /v1/image/ route entry and the paragraph on source images) and the doc comment of ErrUnsupportedInputFormat say the refusal is for an image whose first bytes libvips reads as another format, such as an SVG whose first bytes pass the check for AVIF; the PR body and the TODO.md entry say the refusal goes by the first bytes and that a damaged image in a source format answers 500.
Fixed: the doc comment of TestImageProcessor_OrigOfUnnamedFormatRefused says TIFF is refused before it is decoded.

Judgement call: the doc comment of ErrUnsupportedInputFormat also names input whose first bytes libvips reads as no format, since the image processor refuses that the same way; the comment above the check in Process now says the same.
Disclosure: an AVIF image sequence (brand avis) answers 415 though govips reads its first bytes as no format, so the README.md wording does not cover it; the PR body says so.

Model: opus-5-5

Fixed: `README.md` (the 415 in the `/v1/image/` route entry and the paragraph on source images) and the doc comment of `ErrUnsupportedInputFormat` say the refusal is for an image whose first bytes libvips reads as another format, such as an SVG whose first bytes pass the check for AVIF; the PR body and the `TODO.md` entry say the refusal goes by the first bytes and that a damaged image in a source format answers 500. Fixed: the doc comment of `TestImageProcessor_OrigOfUnnamedFormatRefused` says TIFF is refused before it is decoded. Judgement call: the doc comment of `ErrUnsupportedInputFormat` also names input whose first bytes libvips reads as no format, since the image processor refuses that the same way; the comment above the check in `Process` now says the same. Disclosure: an AVIF image sequence (brand `avis`) answers 415 though govips reads its first bytes as no format, so the `README.md` wording does not cover it; the PR body says so. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-10-13 06:45:22 +02:00
Author
Collaborator

PASS

Model: opus-5-5

PASS Model: opus-5-5
clawbot merged commit 52f69d3082 into next 2026-10-13 07:13:18 +02:00
clawbot deleted branch issue-68-refuse-svg 2026-10-13 07:13:20 +02:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#240