fix(assets): bound what the run pays, not what the document claims (0.10.1)
A second independent review read `230d1cb` -- the commit that closed the `v0.10.0` review's two MAJOR findings -- and found one of them open. The bound read `/Width` and `/Height`, which an untrusted document writes, while `get_data()` pays for the stream beside them; `/Length` is the COMPRESSED length and the two numbers are independent. Re-measured here on `ed8d9d7` before anything changed, in its own interpreter: a 408 516-byte PDF declaring 1x1 and carrying 400 MB of deflated zeros was CARRIED, no rejection, 891 904 000 B peak RSS. After: 0 carried, `asset_too_large`, 57 065 472 B. At 1,2 GB of zeros, 2 436 MB -> 64 569 344 B -- the cost no longer scales with the bomb. End to end through the CLI with the shipped defaults: 838 000 640 B and an asset written -> exit 0, 79 650 816 B, `0 carried of 1 found`, no `assets/`. Three numbers are bounded now, not one: what a container DECLARES, what a carried FILE measures (`read_image`, so a 49 MP PNG of 47 705 bytes is not passed on to a consumer), and what a PDF stream DECOMPRESSES to (`assets.inflated_size`, a chunk at a time, output discarded, before `get_data()`). The limit is stated rather than implied: the stream measurement runs where `FlateDecode` is the first filter and the document is not encrypted; every other chain is a check on the decoded length AFTER the decode, a counted refusal and not a bounded one. A non-positive declared dimension is `asset_size_invalid`, its own code, raised before the stream is read. `-1 x 40000000000` is a NEGATIVE pixel count, under which every `>` bound read as satisfied, so the check returned silently and the refusal arrived from `encode_png` as `asset_samples_invalid`. Its own code because a publisher shipping a picture bigger than this package carries and a dictionary written to be read wrong are different facts about a document. Two smaller findings in the line that says what is missing, both introduced by the first fix: the address was written twice, once bare, and a linkifying renderer autolinks a bare URL -- written once now, in one code span; and `label` became a dead parameter, so the figure's caption was dropped, a regression against 0.10.0. It is written again in the `-- <label>` form a carried pointer uses. Version bumped to 0.10.1 across all ten places. Nine were unbound and stale: four README install lines naming the previous release, two prose lines, the "current tag" entry, `uv.lock`, and a CHANGELOG whose 0.10.1 content sat under `[Unreleased]`. Two new packaging tests bind them to `__version__`, and the README's guard tag to `[tool.uv.sources]`. Every test was red first. The fate of every image is identical with and without the new bound on three K2 PDFs carrying 800 images (464/464, 311/311 with the same 12 rejections, 25/25), and the second inflate is below the noise floor there. 0 shipped artifacts move: no bundle under `examples/`, `skills/` or `tests/fixtures/` carries an image pointer at all, measured against a known-positive control. `asset_too_large` was undocumented in the error registry; both codes are there now. `tools/okf_accounting_gate.py` gains the new code in its closed list -- one string, no behaviour change, stated because that file belongs to another order. Suite 2141 passed / 1 skipped, ruff + format + mypy --strict clean. Report: docs/2026-09-18-bildestien-holder-0-10-1.md Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
ed8d9d709f
commit
0f308c1f56
15 changed files with 842 additions and 81 deletions
60
CHANGELOG.md
60
CHANGELOG.md
|
|
@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file.
|
||||||
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
|
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
|
||||||
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||||
|
|
||||||
## [Unreleased]
|
## [0.10.1] — 2026-09-18
|
||||||
|
|
||||||
### Added
|
### Added
|
||||||
|
|
||||||
|
|
@ -84,21 +84,53 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||||
consumer into an SSRF. The guard refuses such a line at
|
consumer into an SSRF. The guard refuses such a line at
|
||||||
`guard-user-upload` and the build's default tier does not, so the same bytes
|
`guard-user-upload` and the build's default tier does not, so the same bytes
|
||||||
were persisted under the default and refused one tier up. A remote reference
|
were persisted under the default and refused one tier up. A remote reference
|
||||||
is now inert text with the address in a code span, and a property test over
|
is now inert text with the address in ONE code span, and a property test over
|
||||||
the readers asserts that no reference produces a markdown image link outside
|
the readers asserts that no reference produces a markdown image link outside
|
||||||
`assets/`. Found by an independent review of 0.10.0 before it was pushed.
|
`assets/`. Found by an independent review of 0.10.0 before it was pushed.
|
||||||
- **A declared image size is bounded (0.10.1).** Nothing limited a PDF image's
|
- The first fix wrote the address **twice** — once in a code span and once
|
||||||
size: a 9.6 KB file declaring 3 000 x 3 000 grayscale zeros took 83 MB of
|
bare — and a GFM/linkify renderer autolinks a bare URL into `<a href>`.
|
||||||
peak RSS and a 63 KB one declaring 8 000 x 8 000 took 276 MB, linear in the
|
It takes a click rather than a render, so it is weaker than an image link,
|
||||||
pixel count, so one document could exhaust memory and take a whole batch
|
but "inert" was half true. The address is now written once.
|
||||||
build with it — before any gate, because the guard never sees image bytes.
|
- The first fix also **dropped the caption**: `label` stayed in the
|
||||||
`MAX_IMAGE_PIXELS` (40 000 000) and `MAX_IMAGE_BYTES` (256 MiB) are now
|
signature of the line that says what is missing, and no branch read it, so
|
||||||
checked against what the container DECLARES, before anything is
|
the alt text or figure caption of an image the bundle does not carry was
|
||||||
decompressed, and a larger image is refused with `asset_too_large` and
|
lost — a regression against 0.10.0 and against that line's own reason for
|
||||||
counted. The bound is read off the corpora: the largest image in the
|
existing. It is written again, in the same `-- <label>` form a carried
|
||||||
43-document reference corpus is 18.6 MP and R761's largest is 1.4 MP. The
|
pointer uses.
|
||||||
same bound closes the inline `data:` URI, which the review flagged as the
|
- **An image is bounded in three places, and the third is what the run pays
|
||||||
same class and did not measure.
|
(0.10.1).** Nothing limited a PDF image's size: a 9.6 KB file declaring
|
||||||
|
3 000 x 3 000 grayscale zeros took 83 MB of peak RSS and a 63 KB one
|
||||||
|
declaring 8 000 x 8 000 took 276 MB, linear in the pixel count, so one
|
||||||
|
document could exhaust memory and take a whole batch build with it — before
|
||||||
|
any gate, because the guard never sees image bytes.
|
||||||
|
- The size a container **declares** (`/Width` x `/Height`, an IHDR, a
|
||||||
|
`data:` payload's encoded length) is checked against `MAX_IMAGE_PIXELS`
|
||||||
|
(40 000 000) and `MAX_IMAGE_BYTES` (256 MiB) before anything is decoded.
|
||||||
|
- The size a carried **file** has is checked the same way. This package
|
||||||
|
never decodes such a file, so it pays nothing for it — but a 7 000 x 7 000
|
||||||
|
PNG of 47 705 bytes written into a bundle hands the consumer the same bomb
|
||||||
|
with `7000x7000 px` printed beside it.
|
||||||
|
- What the **stream** behind a PDF image decompresses to is measured, a
|
||||||
|
chunk at a time and discarded, before `get_data()` is called. That is an
|
||||||
|
independent number from the declared size: `/Length` is the compressed
|
||||||
|
length, and a second independent review measured a 408 516-byte PDF
|
||||||
|
declaring a 1x1 picture and carrying 400 MB of deflated zeros being
|
||||||
|
CARRIED, with no rejection, at 892 MB of peak RSS. With the bound: 0
|
||||||
|
carried, `asset_too_large`, 54 MB — and 62 MB where the old path cost
|
||||||
|
2 436 MB, so the cost no longer scales with the bomb.
|
||||||
|
- The measurement runs where `FlateDecode` is the first filter and the
|
||||||
|
document is not encrypted. Any other chain is caught by a check on
|
||||||
|
`len(data)` AFTER `get_data()`, which is a counted refusal and not a
|
||||||
|
bounded one. The difference is stated in the code rather than implied.
|
||||||
|
- **A declared size that is not a size is refused with its own code
|
||||||
|
(0.10.1).** `/Width -1 /Height 40000000000` multiplies to a NEGATIVE pixel
|
||||||
|
count, under which every bound read as satisfied: the check returned
|
||||||
|
silently, 400 MB was decompressed, and the refusal arrived from the PNG
|
||||||
|
encoder as `asset_samples_invalid` — a code about a sample buffer for a
|
||||||
|
defect in the declaration. A non-positive dimension is now `asset_size_invalid`,
|
||||||
|
raised before the stream is read. Its own code because a legitimate
|
||||||
|
publisher shipping a picture larger than this package carries and a
|
||||||
|
dictionary written to be read wrong are different facts about a document.
|
||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|
||||||
|
|
|
||||||
53
CLAUDE.md
53
CLAUDE.md
|
|
@ -890,27 +890,58 @@ and fixtures, never code.
|
||||||
move (it is not delivered on the Arm B bundle either, by a different
|
move (it is not delivered on the Arm B bundle either, by a different
|
||||||
mechanism). hit@8 over the six published questions holds at **5 of 6 on both
|
mechanism). hit@8 over the six published questions holds at **5 of 6 on both
|
||||||
K2 bundles**, so the default move cost the retrieval side nothing.
|
K2 bundles**, so the default move cost the retrieval side nothing.
|
||||||
- **The image path has a size CEILING and no live links (0.10.1).** An
|
- **The image path has a size CEILING and no live links (0.10.1).** TWO
|
||||||
independent review of `v0.10.0` found two MAJOR defects, both with the
|
independent reviews, a day apart. The first found two MAJOR defects in
|
||||||
shipped defaults, both new in 0.10.0.
|
`v0.10.0`, both with the shipped defaults, both new in 0.10.0. The second
|
||||||
|
read the fix for them and found that ONE of the two was not closed --
|
||||||
|
which is why the version below is 0.10.1 and not 0.10.0 with a patch note.
|
||||||
- A remote `<img src>` / `xlink:href` became a LIVE markdown image link in
|
- A remote `<img src>` / `xlink:href` became a LIVE markdown image link in
|
||||||
the persisted concept. This package opens no socket; a consumer that
|
the persisted concept. This package opens no socket; a consumer that
|
||||||
renders the bundle does, and the guard refuses such a line at
|
renders the bundle does, and the guard refuses such a line at
|
||||||
`user-upload` while the build's default tier persisted it. Now inert text
|
`user-upload` while the build's default tier persisted it. Now inert text
|
||||||
with the address in a code span, pinned by a PROPERTY over the readers
|
with the address in a code span, pinned by a PROPERTY over the readers
|
||||||
(`FOREIGN_IMAGE_LINK`), never one string.
|
(`FOREIGN_IMAGE_LINK`), never one string.
|
||||||
- Nothing bounded a declared image size: 9.6 KB of PDF declaring
|
- **THE FIRST FIX FOR IT WROTE THE ADDRESS TWICE**, once in a code span and
|
||||||
3 000 x 3 000 zeros took 83 MB peak RSS, linear in pixels.
|
once BARE, and a GFM/linkify renderer autolinks a bare URL. Weaker than an
|
||||||
`MAX_IMAGE_PIXELS = 40 000 000` and `MAX_IMAGE_BYTES = 256 MiB` are
|
image link -- it takes a click -- but "inert" was half true. It also
|
||||||
checked on what the container DECLARES, before any decompression, and
|
dropped the figure's CAPTION: `label` stayed in the signature and no
|
||||||
over them is `asset_too_large`, counted. Read off the corpora: the
|
branch read it, so an image the bundle does not carry lost the one fact
|
||||||
largest image measured is 18.6 MP (K2) and 1.4 MP (R761). The same bound
|
that lets a reader weigh the absence. Both closed; the caption is written
|
||||||
closes the inline `data:` URI.
|
in the same `-- <label>` form a carried pointer uses.
|
||||||
|
- **NOTHING BOUNDED WHAT THE RUN PAYS, only what the document CLAIMS.**
|
||||||
|
`MAX_IMAGE_PIXELS = 40 000 000` and `MAX_IMAGE_BYTES = 256 MiB` were
|
||||||
|
checked against `/Width` x `/Height`, which is an untrusted document's
|
||||||
|
claim, while `get_data()` pays for the actual stream -- two independent
|
||||||
|
numbers, since `/Length` is the COMPRESSED length. Measured by the second
|
||||||
|
review: a **408 516-byte** PDF declaring **1x1** and carrying 400 MB of
|
||||||
|
deflated zeros was CARRIED, no rejection, **892 MB peak RSS**, and 1,2 GB
|
||||||
|
of zeros cost **2 436 MB** -- about 2 100x the file size, linear. The
|
||||||
|
first fix's four mutations all died, because not one of them separated
|
||||||
|
declared from actual.
|
||||||
|
- **THREE NUMBERS ARE BOUNDED NOW.** What the container DECLARES (as
|
||||||
|
before), what a carried FILE measures (`read_image`, so a 49 MP PNG of
|
||||||
|
47 705 bytes is not passed on to the consumer with `7000x7000 px` beside
|
||||||
|
it), and what a PDF image's STREAM decompresses to -- measured a chunk at
|
||||||
|
a time and DISCARDED, before `get_data()`. With it: **0 carried,
|
||||||
|
`asset_too_large`, 54 MB** at 400 MB and **62 MB** at 1,2 GB, so the cost
|
||||||
|
no longer scales with the bomb. **The limit is stated rather than
|
||||||
|
implied**: the stream measurement runs where `FlateDecode` is the first
|
||||||
|
filter and the document is not encrypted; every other chain is a check on
|
||||||
|
the decoded length AFTER the decode, which is a COUNTED refusal and not a
|
||||||
|
bounded one.
|
||||||
|
- **A non-positive declared dimension is `asset_size_invalid`**, its own
|
||||||
|
code, raised before the stream is read. `-1 x 40 000 000 000` is a
|
||||||
|
NEGATIVE pixel count, under which every `>` bound reads as satisfied: the
|
||||||
|
check returned silently, 400 MB was decompressed, and the refusal came
|
||||||
|
from the PNG encoder as `asset_samples_invalid`. Its own code because a
|
||||||
|
publisher shipping a picture bigger than we carry and a dictionary written
|
||||||
|
to be read wrong are different facts about a document.
|
||||||
- **An inline PDF image is named from its position, not from `id()`** —
|
- **An inline PDF image is named from its position, not from `id()`** —
|
||||||
pdfminer's name broke the bit-exact rebuild on two K2 concepts.
|
pdfminer's name broke the bit-exact rebuild on two K2 concepts.
|
||||||
- Documented rather than changed: `images: N` counts POINTER BLOCKS, and a
|
- Documented rather than changed: `images: N` counts POINTER BLOCKS, and a
|
||||||
pointer-only concept is substantive.
|
pointer-only concept is substantive.
|
||||||
- Report: `docs/2026-09-17-bildestien-0-10-1.md`.
|
- Reports: `docs/2026-09-17-bildestien-0-10-1.md` and
|
||||||
|
`docs/2026-09-18-bildestien-holder-0-10-1.md`.
|
||||||
- **Account for content: `okf build --accounting PATH` and its judge
|
- **Account for content: `okf build --accounting PATH` and its judge
|
||||||
`python3 tools/okf_accounting_gate.py` (2026-09-17).**
|
`python3 tools/okf_accounting_gate.py` (2026-09-17).**
|
||||||
- The gate was written RED at `0b00de4` and turned GREEN on all six rows by
|
- The gate was written RED at `0b00de4` and turned GREEN on all six rows by
|
||||||
|
|
|
||||||
77
README.md
77
README.md
|
|
@ -10,7 +10,7 @@ types](#supported-file-types) lists each one with the evidence behind it.
|
||||||
Python 3.10+ and [uv](https://docs.astral.sh/uv/). One line:
|
Python 3.10+ and [uv](https://docs.astral.sh/uv/). One line:
|
||||||
|
|
||||||
```sh
|
```sh
|
||||||
uv tool install "llm-ingestion-okf[extract] @ git+https://git.fromaitochitta.com/open/llm-ingestion-okf.git@v0.10.0"
|
uv tool install "llm-ingestion-okf[extract] @ git+https://git.fromaitochitta.com/open/llm-ingestion-okf.git@v0.10.1"
|
||||||
```
|
```
|
||||||
|
|
||||||
## Use it
|
## Use it
|
||||||
|
|
@ -120,11 +120,11 @@ Neither this package nor the guard it depends on is on a package index yet, so
|
||||||
both install by direct reference. With uv, one command resolves both:
|
both install by direct reference. With uv, one command resolves both:
|
||||||
|
|
||||||
```sh
|
```sh
|
||||||
uv pip install "llm-ingestion-okf @ git+https://git.fromaitochitta.com/open/llm-ingestion-okf.git@v0.10.0"
|
uv pip install "llm-ingestion-okf @ git+https://git.fromaitochitta.com/open/llm-ingestion-okf.git@v0.10.1"
|
||||||
```
|
```
|
||||||
|
|
||||||
uv resolves the guard on its own, because it reads the `[tool.uv.sources]`
|
uv resolves the guard on its own, because it reads the `[tool.uv.sources]`
|
||||||
entry in the `pyproject.toml` **of the tag it is installing**, and `v0.10.0`
|
entry in the `pyproject.toml` **of the tag it is installing**, and `v0.10.1`
|
||||||
points that entry at `llm-ingestion-guard` `v1.4.0`. Use `uv tool install`
|
points that entry at `llm-ingestion-guard` `v1.4.0`. Use `uv tool install`
|
||||||
instead of `uv pip install` when you want the `okf` command on `PATH` without an
|
instead of `uv pip install` when you want the `okf` command on `PATH` without an
|
||||||
active virtualenv — that is the form the first screen shows.
|
active virtualenv — that is the form the first screen shows.
|
||||||
|
|
@ -135,10 +135,10 @@ With plain pip, the transitive git dependency does not resolve on its own —
|
||||||
|
|
||||||
```sh
|
```sh
|
||||||
pip install "llm-ingestion-guard @ git+https://git.fromaitochitta.com/open/llm-ingestion-pipeline-security.git@v1.4.0"
|
pip install "llm-ingestion-guard @ git+https://git.fromaitochitta.com/open/llm-ingestion-pipeline-security.git@v1.4.0"
|
||||||
pip install "llm-ingestion-okf @ git+https://git.fromaitochitta.com/open/llm-ingestion-okf.git@v0.10.0"
|
pip install "llm-ingestion-okf @ git+https://git.fromaitochitta.com/open/llm-ingestion-okf.git@v0.10.1"
|
||||||
```
|
```
|
||||||
|
|
||||||
The guard tag is paired to the okf tag, not to this branch. `v0.10.0` declares
|
The guard tag is paired to the okf tag, not to this branch. `v0.10.1` declares
|
||||||
`llm-ingestion-guard>=1.2,<2.0`, which `v1.4.0` satisfies; the pairing above is
|
`llm-ingestion-guard>=1.2,<2.0`, which `v1.4.0` satisfies; the pairing above is
|
||||||
read off that tag's own `[tool.uv.sources]`, not off this branch. Reading a pin
|
read off that tag's own `[tool.uv.sources]`, not off this branch. Reading a pin
|
||||||
off `main` and installing it against an older okf tag is the one combination
|
off `main` and installing it against an older okf tag is the one combination
|
||||||
|
|
@ -149,7 +149,16 @@ that fails.
|
||||||
These are not install lines. They record what each earlier tag was, so a reader
|
These are not install lines. They record what each earlier tag was, so a reader
|
||||||
who meets one in an older document knows what they are looking at.
|
who meets one in an older document knows what they are looking at.
|
||||||
|
|
||||||
- `v0.10.0` — the current tag: a bundle carries the IMAGES its sources
|
- `v0.10.1` — the current tag: the image path of `v0.10.0`, with the two
|
||||||
|
defects an independent review found in it closed. A remote `<img src>` or
|
||||||
|
`xlink:href` is inert text with the address in one code span, never a live
|
||||||
|
markdown image link, and no longer loses the figure's caption. An image is
|
||||||
|
bounded in three places rather than one: the size a container DECLARES, the
|
||||||
|
size a carried file has, and — new in this tag — what the stream behind a
|
||||||
|
PDF image actually DECOMPRESSES to, which is an independent number. A
|
||||||
|
declared size that is not positive is refused with its own code,
|
||||||
|
`asset_size_invalid`, before the stream is read.
|
||||||
|
- `v0.10.0` — a bundle carries the IMAGES its sources
|
||||||
declare. Five readers place them (`pdf`, the converter's office rows,
|
declare. Five readers place them (`pdf`, the converter's office rows,
|
||||||
`html`, `xml`), `assets/` at the bundle root holds the bytes under a
|
`html`, `xml`), `assets/` at the bundle root holds the bytes under a
|
||||||
content-addressed name, and the concept carries a two-line pointer where the
|
content-addressed name, and the concept carries a two-line pointer where the
|
||||||
|
|
@ -299,32 +308,54 @@ in must not read like a bundle of documents that had none.
|
||||||
text. The pointer block passes the gate like any other body line, and the file
|
text. The pointer block passes the gate like any other body line, and the file
|
||||||
beside it passes nothing. `log.md` says so on every run that carries one.
|
beside it passes nothing. `log.md` says so on every run that carries one.
|
||||||
|
|
||||||
**A size CEILING, read off the same corpora (0.10.1).** An image whose
|
**A size CEILING, read off the same corpora (0.10.1).** An image over
|
||||||
DECLARED size is over `MAX_IMAGE_PIXELS` (40 000 000 pixels) or
|
`MAX_IMAGE_PIXELS` (40 000 000 pixels) or `MAX_IMAGE_BYTES` (256 MiB) is
|
||||||
`MAX_IMAGE_BYTES` (256 MiB of samples) is refused with `asset_too_large`,
|
refused with `asset_too_large`, counted like every other refusal. The largest
|
||||||
counted like every other refusal, and never decoded. The largest image in the
|
image in the 43-document reference corpus is 4 515 x 4 128 (18.6 MP) and the
|
||||||
43-document reference corpus is 4 515 x 4 128 (18.6 MP) and the largest of
|
largest of R761's 109 pictures is 2 072 x 656 (1.4 MP), so the bound is an
|
||||||
R761's 109 pictures is 2 072 x 656 (1.4 MP), so the bound is an order of
|
order of magnitude above anything measured.
|
||||||
magnitude above anything measured.
|
|
||||||
|
|
||||||
It exists because a few kilobytes can declare an enormous picture: a 9.6 KB
|
It exists because a few kilobytes can declare an enormous picture: a 9.6 KB
|
||||||
PDF declaring 3 000 x 3 000 grayscale zeros took 83 MB of peak RSS, a 63 KB
|
PDF declaring 3 000 x 3 000 grayscale zeros took 83 MB of peak RSS, a 63 KB
|
||||||
one declaring 8 000 x 8 000 took 276 MB, and the cost is linear in the pixel
|
one declaring 8 000 x 8 000 took 276 MB, and the cost is linear in the pixel
|
||||||
count, so one document could take a whole batch build with it — before any
|
count, so one document could take a whole batch build with it — before any
|
||||||
gate, because the guard never sees image bytes. The bound is checked on what
|
gate, because the guard never sees image bytes.
|
||||||
the container declares, before anything is decompressed, on the two paths that
|
|
||||||
decode: a PDF image's raw samples and an inline `data:` URI. An image file
|
**Three numbers are bounded, not one, because a claim is not a cost.** What a
|
||||||
carried verbatim is not decoded at all, so its cost is its own file size.
|
container DECLARES (`/Width` x `/Height`, a PNG header, a `data:` payload's
|
||||||
|
encoded length) is read before anything is decoded. What a carried FILE
|
||||||
|
measures is read the same way — this package never decodes such a file, so it
|
||||||
|
pays nothing for it, but writing a 7 000 x 7 000 PNG of 47 705 bytes into a
|
||||||
|
bundle would hand the consumer the same bomb with `7000x7000 px` printed
|
||||||
|
beside it. And what a PDF image's STREAM decompresses to is measured before it
|
||||||
|
is held, a chunk at a time and discarded, because `/Length` is the COMPRESSED
|
||||||
|
length and a dictionary declaring 1x1 may hang 400 MB of deflated zeros off
|
||||||
|
it. Measured: that document is 408 516 bytes and cost 892 MB of peak RSS with
|
||||||
|
only the declared size bounded; with the stream bounded it is refused at
|
||||||
|
54 MB, and a three-times-larger bomb costs 62 MB rather than 2 436 MB.
|
||||||
|
|
||||||
|
**What the stream bound does NOT reach**, stated because the difference
|
||||||
|
matters: it runs where `FlateDecode` is the first filter and the document is
|
||||||
|
not encrypted. Any other filter chain is caught by a check on the decoded
|
||||||
|
length AFTER the decode, which makes it a counted refusal rather than a
|
||||||
|
bounded one — the memory is spent and then the picture is dropped.
|
||||||
|
|
||||||
|
**A declared size that is not a size** — a zero or negative `/Width` or
|
||||||
|
`/Height` — is refused with its own code, `asset_size_invalid`, before the
|
||||||
|
stream is read. Distinct from `asset_too_large` on purpose: one is a publisher
|
||||||
|
shipping a picture bigger than this package carries, the other is a dictionary
|
||||||
|
written wrong or written to be read wrong.
|
||||||
|
|
||||||
<!-- asset-max-pixels: 40000000 -->
|
<!-- asset-max-pixels: 40000000 -->
|
||||||
|
|
||||||
**A remote reference is INERT (0.10.1).** `<img src="https://...">` and an STS
|
**A remote reference is INERT (0.10.1).** `<img src="https://...">` and an STS
|
||||||
`xlink:href` to an address off this machine are written as text with the
|
`xlink:href` to an address off this machine are written as text with the
|
||||||
address in a code span, never as ``. Extraction opens no
|
address in one code span, never as `` and never as a bare
|
||||||
socket, but a markdown renderer or an agent that fetches what it renders does,
|
URL a linkifying renderer would autolink. Extraction opens no socket, but a
|
||||||
which would turn "this bundle was opened" into a beacon to whoever wrote the
|
markdown renderer or an agent that fetches what it renders does, which would
|
||||||
document. The address is still stated, because a reader has to know what stood
|
turn "this bundle was opened" into a beacon to whoever wrote the document. The
|
||||||
there.
|
address is still stated, and so is the figure's caption, because a reader has
|
||||||
|
to know what stood there.
|
||||||
|
|
||||||
**`images: N` in a concept counts POINTER BLOCKS, not unique pictures.** One
|
**`images: N` in a concept counts POINTER BLOCKS, not unique pictures.** One
|
||||||
image referenced twelve times in one concept is `images: 12` and one file in
|
image referenced twelve times in one concept is `images: 12` and one file in
|
||||||
|
|
@ -877,7 +908,7 @@ not need this repository — the first line installs the command, the second
|
||||||
builds the bundle and writes a skill beside it, the third asks.
|
builds the bundle and writes a skill beside it, the third asks.
|
||||||
|
|
||||||
```sh
|
```sh
|
||||||
uv tool install "llm-ingestion-okf[extract] @ git+https://git.fromaitochitta.com/open/llm-ingestion-okf.git@v0.10.0"
|
uv tool install "llm-ingestion-okf[extract] @ git+https://git.fromaitochitta.com/open/llm-ingestion-okf.git@v0.10.1"
|
||||||
okf project ~/my-documents
|
okf project ~/my-documents
|
||||||
claude
|
claude
|
||||||
```
|
```
|
||||||
|
|
|
||||||
|
|
@ -74,12 +74,28 @@ Decoding first gives `asset_pdf_unsupported`; reading the size first gives
|
||||||
`asset_too_large`. `encode_png` refuses the same size on its own, so the
|
`asset_too_large`. `encode_png` refuses the same size on its own, so the
|
||||||
encoder does not trust its caller.
|
encoder does not trust its caller.
|
||||||
|
|
||||||
|
> **CORRECTION, 2026-09-18 — this paragraph claimed a defence this round did
|
||||||
|
> not build.** A declared size and a decompressed stream size are two
|
||||||
|
> independent numbers: `/Length` is the COMPRESSED length, and nothing in the
|
||||||
|
> dictionary states what `get_data()` returns. A second independent review
|
||||||
|
> measured a 408 516-byte PDF declaring **1x1** and carrying 400 MB of
|
||||||
|
> deflated zeros being CARRIED, with no rejection, at **892 MB of peak RSS** —
|
||||||
|
> the same failure mode this round set out to close, reached through the other
|
||||||
|
> number. The bound was real and it bound the wrong thing. What this round
|
||||||
|
> DID close is the declared size and the `data:` URI; the stream itself is
|
||||||
|
> bounded from `docs/2026-09-18-bildestien-holder-0-10-1.md`, and the
|
||||||
|
> paragraph below about a verbatim file is corrected there too.
|
||||||
|
|
||||||
**The `data:` URI, which the review flagged and did not measure,** is closed by
|
**The `data:` URI, which the review flagged and did not measure,** is closed by
|
||||||
the same bound, checked on the payload length before decoding (base64 expands
|
the same bound, checked on the payload length before decoding (base64 expands
|
||||||
by 3/4). Measured: refused with `asset_too_large` as a row.
|
by 3/4). Measured: refused with `asset_too_large` as a row.
|
||||||
|
|
||||||
Not bounded, and stated rather than hidden: an image FILE carried verbatim is
|
Not bounded, and stated rather than hidden: an image FILE carried verbatim is
|
||||||
never decoded, so its cost is its own file size.
|
never decoded, so its cost is its own file size. (Corrected 2026-09-18: it is
|
||||||
|
bounded now. This package pays nothing for such a file, but a 7 000 x 7 000
|
||||||
|
PNG of 47 705 bytes written into a bundle hands the consumer the same bomb
|
||||||
|
with `7000x7000 px` printed beside it, and the README's own first sentence
|
||||||
|
about this bound said it was refused.)
|
||||||
|
|
||||||
## The determinism defect PM added to this order
|
## The determinism defect PM added to this order
|
||||||
|
|
||||||
|
|
|
||||||
175
docs/2026-09-18-bildestien-holder-0-10-1.md
Normal file
175
docs/2026-09-18-bildestien-holder-0-10-1.md
Normal file
|
|
@ -0,0 +1,175 @@
|
||||||
|
# The image path, second review: the bound bound a claim, not a cost
|
||||||
|
|
||||||
|
A second independent review, one day after the first, read `230d1cb` — the
|
||||||
|
commit that closed the two MAJOR findings of the `v0.10.0` review — and found
|
||||||
|
that one of the two was **not closed**. This is what that review found, what it
|
||||||
|
measured, and what this round changed. It is the round that makes `v0.10.1`
|
||||||
|
true; the round it corrects is
|
||||||
|
[`docs/2026-09-17-bildestien-0-10-1.md`](2026-09-17-bildestien-0-10-1.md), which
|
||||||
|
now carries a correction block where it claimed this defence.
|
||||||
|
|
||||||
|
## What was wrong: a declared size and a decompressed stream are two numbers
|
||||||
|
|
||||||
|
`check_size` read `/Width` and `/Height` out of a PDF image dictionary and
|
||||||
|
refused anything over `MAX_IMAGE_PIXELS`. Both numbers are written by the
|
||||||
|
document. `/Length` beside them is the **compressed** length, and nothing in
|
||||||
|
the dictionary states what `stream.get_data()` will return. A document that
|
||||||
|
declares `1 x 1` and hangs 400 MB of deflated zeros off it therefore passed the
|
||||||
|
check, paid the whole cost, and was **carried** as a one-pixel picture.
|
||||||
|
|
||||||
|
Measured on `230d1cb`, and re-measured here on `ed8d9d7` before anything
|
||||||
|
changed, with the same fixture in its own interpreter:
|
||||||
|
|
||||||
|
| stream inflates to | file size | declared | carried | rejection | peak RSS |
|
||||||
|
|---|---:|---|---|---|---:|
|
||||||
|
| 400 MB | 408 516 B | 1 x 1 | yes | none | **891 904 000 B** |
|
||||||
|
| 1,2 GB | 1 223 843 B | 1 x 1 | yes | none | **2 436 MB** (review) |
|
||||||
|
|
||||||
|
About 2 100x the file size, linear, so a 10 MB document is roughly 21 GB. It is
|
||||||
|
the failure mode the first review named — one document killing a batch build —
|
||||||
|
reached through the other number.
|
||||||
|
|
||||||
|
The first round's four mutations were all killed by its tests, and that told us
|
||||||
|
nothing about this: **not one of them separated a declared size from an actual
|
||||||
|
one**. A test built on an honest `20000 x 20000` declaration is green whether or
|
||||||
|
not the stream is bounded.
|
||||||
|
|
||||||
|
## What changed: three numbers are bounded, and the limit is stated
|
||||||
|
|
||||||
|
1. **What the container DECLARES** — unchanged from the first round.
|
||||||
|
2. **What a carried FILE measures** — new. `read_image` now checks the size it
|
||||||
|
sniffs out of the header. This package never decodes such a file, so it pays
|
||||||
|
nothing for it; but a 7 000 x 7 000 PNG of 47 705 bytes written into a bundle
|
||||||
|
hands the consumer the same bomb with `7000x7000 px` printed beside it, and
|
||||||
|
the README's first sentence about this bound said such an image was refused.
|
||||||
|
Over the 4 828 image objects of the reference corpus the largest is 18.6 MP,
|
||||||
|
so nothing measured is refused by this.
|
||||||
|
3. **What a PDF image's STREAM decompresses to** — new, and the finding.
|
||||||
|
`assets.inflated_size` inflates the raw stream a chunk at a time, keeps only
|
||||||
|
a running total, and **discards the output**, so the measurement cannot cost
|
||||||
|
what the bomb costs. It runs before `get_data()`.
|
||||||
|
|
||||||
|
**The limit is stated, not implied.** The stream measurement runs where
|
||||||
|
`FlateDecode` is the first filter and the document is not encrypted (the raw
|
||||||
|
bytes are ciphertext until pdfminer deciphers them). Every other filter chain is
|
||||||
|
caught by `check_payload(len(data))` **after** `get_data()` — a counted refusal,
|
||||||
|
not a bounded one: the memory is spent and then the picture is dropped. That
|
||||||
|
distinction is the whole substance of this round, so it is written in
|
||||||
|
`_check_inflated`'s docstring rather than left for the next reviewer to find.
|
||||||
|
|
||||||
|
A legitimate image is therefore inflated twice, once to measure and once to
|
||||||
|
carry. That is the price of not holding an unbounded buffer in order to find out
|
||||||
|
how big it is, **and at this sample it does not show**: three PDFs of the K2
|
||||||
|
reference corpus, 800 carried images between them, extracted twice in one
|
||||||
|
process with the measurement on and off.
|
||||||
|
|
||||||
|
| document | images | rejections | with the bound | without |
|
||||||
|
|---|---:|---:|---:|---:|
|
||||||
|
| Bilag 3.1 Miljøteknisk rapport | 464 | 0 | 18.21 s | 18.51 s |
|
||||||
|
| Bilag 1 Kravspesifikasjon | 311 | 12 | 16.28 s | 16.88 s |
|
||||||
|
| Bilag 3.3.1 Brannkonsept | 25 | 0 | 2.70 s | 2.35 s |
|
||||||
|
|
||||||
|
The bounded arm is faster on two of the three and slower on the third, which is
|
||||||
|
run-to-run noise rather than a speedup: the honest reading is that a second
|
||||||
|
inflate of these streams is **below the noise floor** of a PDF extraction that
|
||||||
|
is dominated by text layout. What is NOT noise is that the fate of every image
|
||||||
|
is identical on both arms — 464/464, 311/311 with the same 12 rejections, and
|
||||||
|
25/25 — so the bound refuses nothing these documents carry.
|
||||||
|
|
||||||
|
### A declared size that is not a size
|
||||||
|
|
||||||
|
`/Width -1 /Height 40000000000` multiplies to a **negative** pixel count, under
|
||||||
|
which every `>` bound reads as satisfied. `check_size` returned silently, 400 MB
|
||||||
|
was decompressed, and the refusal arrived from `encode_png` as
|
||||||
|
`asset_samples_invalid` — a code about a sample buffer, for a defect in the
|
||||||
|
declaration. A non-positive dimension is now **`asset_size_invalid`**, raised
|
||||||
|
before the stream is read.
|
||||||
|
|
||||||
|
It gets its own code rather than joining `asset_too_large` because the two say
|
||||||
|
different things about a document: a publisher shipping a picture larger than
|
||||||
|
this package carries is not a dictionary written to be read wrong, and counting
|
||||||
|
them together would make a corpus statistic about the first untrue. `None` stays
|
||||||
|
UNKNOWN — a container that declares no size leaves nothing to bound, and
|
||||||
|
inventing a number would refuse a legitimate picture.
|
||||||
|
|
||||||
|
### The line that says what is missing
|
||||||
|
|
||||||
|
Two smaller findings in the same line, both introduced by the first round's fix:
|
||||||
|
|
||||||
|
- **The address was written twice**, once in a code span and once bare, and a
|
||||||
|
GFM/linkify renderer autolinks a bare URL into `<a href>`. It takes a click
|
||||||
|
rather than a render, so it is weaker than `` — but "inert" was
|
||||||
|
half true, and half is what that line exists not to be. Written once now.
|
||||||
|
- **The caption was dropped.** `label` stayed in the signature and no branch
|
||||||
|
read it, so the alt text or figure caption of an image the bundle does not
|
||||||
|
carry was lost — a regression against 0.10.0 and against the line's own stated
|
||||||
|
reason for existing (a reader cannot weigh an absence they were never shown).
|
||||||
|
It is written again, in the same `-- <label>` form a carried pointer uses.
|
||||||
|
|
||||||
|
## Measured after
|
||||||
|
|
||||||
|
Same fixture, same machine, same command, in its own interpreter:
|
||||||
|
|
||||||
|
| stream inflates to | file size | before | after |
|
||||||
|
|---|---:|---:|---|
|
||||||
|
| 400 MB | 408 516 B | carried, 891 904 000 B peak | **refused `asset_too_large`, 57 065 472 B peak** |
|
||||||
|
| 1,2 GB | 1 223 843 B | carried, 2 436 MB peak | **refused `asset_too_large`, 64 569 344 B peak** |
|
||||||
|
|
||||||
|
The bounded figure barely moves when the stream triples, because what grows is
|
||||||
|
the compressed input, which was already in memory. **The cost no longer scales
|
||||||
|
with the bomb**, which is the property the first round did not have.
|
||||||
|
|
||||||
|
End to end through the shipped CLI, default flags, the bomb beside one ordinary
|
||||||
|
markdown document so the build has a plan to write:
|
||||||
|
|
||||||
|
```
|
||||||
|
$ /usr/bin/time -l okf build /tmp/okf-r3/in --bundle /tmp/okf-r3/bb \
|
||||||
|
--bundle-id t3 --okf-version 0.2
|
||||||
|
exit=0
|
||||||
|
79650816 maximum resident set size
|
||||||
|
$ ls /tmp/okf-r3/bb/assets
|
||||||
|
ls: .../assets: No such file or directory
|
||||||
|
$ grep Images /tmp/okf-r3/bb/log.md
|
||||||
|
* **Images**: 0 carried of 1 found, ...
|
||||||
|
```
|
||||||
|
|
||||||
|
The review measured 838 000 640 B and an `assets/…-im0.png` written after it.
|
||||||
|
The concept says what stood there:
|
||||||
|
|
||||||
|
```
|
||||||
|
Image: page-1-Im0 (not carried: the stream behind 'page-1-Im0' decompresses to
|
||||||
|
more than 268435456 bytes from 407685 bytes of input, over this package's
|
||||||
|
bound; refused without being held, because the size a container declares is a
|
||||||
|
claim and this is the cost)
|
||||||
|
```
|
||||||
|
|
||||||
|
## The version
|
||||||
|
|
||||||
|
`v0.10.1` was going to be cut from a tree that said `0.10.0` in nine places: two
|
||||||
|
bound to each other by a test, and seven bound to nothing — four README install
|
||||||
|
lines telling a consumer to install the release before this one, two prose lines
|
||||||
|
about what that tag declares, the "current tag" entry, and a CHANGELOG whose
|
||||||
|
0.10.1 content sat under `[Unreleased]`.
|
||||||
|
|
||||||
|
Two new tests in `tests/test_packaging.py` bind all of them to
|
||||||
|
`llm_ingestion_okf.__version__`, plus the guard tag in the README to the one in
|
||||||
|
`[tool.uv.sources]`, which is the same class of instruction going stale. Proven
|
||||||
|
red in the intended way: bumping `__version__` and `pyproject.toml` alone turned
|
||||||
|
the first one red with `install lines name ['v0.10.0']`.
|
||||||
|
|
||||||
|
## Not measured
|
||||||
|
|
||||||
|
- **The 43-document reference corpus was not rebuilt.** The `render_missing`
|
||||||
|
change adds `-- <label>` to the line for an image a bundle does not carry, so
|
||||||
|
an assets-on corpus build differs on exactly those lines. That is the repair,
|
||||||
|
not a side effect, but its size across that corpus is unmeasured here. What
|
||||||
|
IS measured: **0 shipped artifacts move**. No bundle under `examples/`,
|
||||||
|
`skills/` or `tests/fixtures/` carries an image pointer of either kind — 0
|
||||||
|
occurrences of `Image: ` against a known-positive control on the same
|
||||||
|
directories — so both this change and the new `read_image` bound move zero
|
||||||
|
committed bytes.
|
||||||
|
- The pinned `K2-bundle-default-20260912` predates the image path entirely and
|
||||||
|
cannot move.
|
||||||
|
- The review's own "not measured" list stands: the content accounting after this
|
||||||
|
change, R761's `50 carried of 50`, docx/pptx/xlsx zip-slip, SVG as active
|
||||||
|
content, and the `[ocr]` path.
|
||||||
|
|
@ -4,7 +4,7 @@ build-backend = "hatchling.build"
|
||||||
|
|
||||||
[project]
|
[project]
|
||||||
name = "llm-ingestion-okf"
|
name = "llm-ingestion-okf"
|
||||||
version = "0.10.0"
|
version = "0.10.1"
|
||||||
description = "Shared OKF (Open Knowledge Format) ingestion library: spec-based connectors, bundle inbox, and external-bundle import, with security delegated to llm-ingestion-guard."
|
description = "Shared OKF (Open Knowledge Format) ingestion library: spec-based connectors, bundle inbox, and external-bundle import, with security delegated to llm-ingestion-guard."
|
||||||
readme = "README.md"
|
readme = "README.md"
|
||||||
license = "MIT"
|
license = "MIT"
|
||||||
|
|
|
||||||
|
|
@ -76,7 +76,7 @@ from .manifest import (
|
||||||
)
|
)
|
||||||
from .materialize import IngestResult, materialize_bundle
|
from .materialize import IngestResult, materialize_bundle
|
||||||
|
|
||||||
__version__ = "0.10.0"
|
__version__ = "0.10.1"
|
||||||
|
|
||||||
__all__ = [
|
__all__ = [
|
||||||
"BlockedFile",
|
"BlockedFile",
|
||||||
|
|
|
||||||
|
|
@ -131,22 +131,50 @@ IMAGE_POINTER = re.compile(
|
||||||
#: Found by an independent review of 0.10.0 before it was pushed.
|
#: Found by an independent review of 0.10.0 before it was pushed.
|
||||||
MAX_IMAGE_PIXELS = 40_000_000
|
MAX_IMAGE_PIXELS = 40_000_000
|
||||||
|
|
||||||
#: The same bound in bytes, for a payload whose pixels are not yet known (an
|
#: The same bound in bytes: for a payload whose pixels are not yet known (an
|
||||||
#: inline `data:` URI) and for raw samples with several channels.
|
#: inline `data:` URI) and for the DECOMPRESSED size of a stream, which is a
|
||||||
|
#: different number from anything the container declares. `/Length` in a PDF
|
||||||
|
#: image dictionary is the COMPRESSED length and `/Width` and `/Height` are a
|
||||||
|
#: claim; nothing in the dictionary states what the decompression will cost.
|
||||||
MAX_IMAGE_BYTES = 256 * 1024 * 1024
|
MAX_IMAGE_BYTES = 256 * 1024 * 1024
|
||||||
|
|
||||||
|
#: How much of a stream is inflated at a time while `inflated_size` measures
|
||||||
|
#: it. The cap on the OUTPUT is what keeps the measurement cheaper than the
|
||||||
|
#: bomb; the input is handed over whole because it is already in memory.
|
||||||
|
_INFLATE_CHUNK = 1 << 20
|
||||||
|
|
||||||
|
|
||||||
def check_size(
|
def check_size(
|
||||||
width: int | None, height: int | None, *, name: str, channels: int = 1, bits: int = 8
|
width: int | None, height: int | None, *, name: str, channels: int = 1, bits: int = 8
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Refuse a declared size beyond the bound, BEFORE anything is decoded.
|
"""Refuse a declared size beyond the bound, BEFORE anything is decoded.
|
||||||
|
|
||||||
Read off what the container DECLARES, which is the only number available
|
Read off what the container DECLARES. That is the only number available
|
||||||
before the cost is paid: a check after the decompression has already paid
|
before a cost is paid, and it is a CLAIM by an untrusted document rather
|
||||||
for the bomb it was meant to stop.
|
than a measurement: this refuses a picture too large to be one we carry,
|
||||||
|
and it says nothing about what decompressing the stream beside it costs.
|
||||||
|
`inflated_size` is the other half, and neither substitutes for the other.
|
||||||
|
|
||||||
|
`None` is UNKNOWN -- a container that declares no size leaves nothing to
|
||||||
|
bound, and inventing a number would refuse a legitimate picture. A size
|
||||||
|
that is declared and is not positive is neither unknown nor large: it is a
|
||||||
|
dictionary that was written wrong or written to be read wrong, and it is
|
||||||
|
refused here rather than three steps later by the encoder. Measured on
|
||||||
|
`230d1cb`: `/Width -1 /Height 40000000000` multiplies to a NEGATIVE pixel
|
||||||
|
count, so every `>` below was false, the function returned silently, 400 MB
|
||||||
|
was decompressed, and the refusal arrived from `encode_png` under
|
||||||
|
`asset_samples_invalid` -- a code about a sample buffer for a defect in the
|
||||||
|
declaration.
|
||||||
"""
|
"""
|
||||||
if not width or not height:
|
if width is None or height is None:
|
||||||
return
|
return
|
||||||
|
if width <= 0 or height <= 0:
|
||||||
|
raise ExtractionError(
|
||||||
|
f"the image {name!r} declares {width}x{height}, which is not a size; "
|
||||||
|
"refused unread rather than multiplied out, because a non-positive "
|
||||||
|
"dimension makes every bound below it read as satisfied",
|
||||||
|
code="asset_size_invalid",
|
||||||
|
)
|
||||||
pixels = width * height
|
pixels = width * height
|
||||||
expected = pixels * channels * (bits // 8 or 1)
|
expected = pixels * channels * (bits // 8 or 1)
|
||||||
if pixels > MAX_IMAGE_PIXELS or expected > MAX_IMAGE_BYTES:
|
if pixels > MAX_IMAGE_PIXELS or expected > MAX_IMAGE_BYTES:
|
||||||
|
|
@ -169,6 +197,53 @@ def check_payload(size: int, *, name: str) -> None:
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def inflated_size(raw: bytes, *, name: str, limit: int | None = None) -> int:
|
||||||
|
"""What a deflate stream costs to decompress, measured without paying it.
|
||||||
|
|
||||||
|
THE DECLARED SIZE AND THE COST ARE TWO INDEPENDENT NUMBERS, and binding
|
||||||
|
only the first is what an independent review of 0.10.1 measured on
|
||||||
|
`230d1cb`: a 408 516-byte PDF declaring a 1x1 picture and carrying 400 MB
|
||||||
|
of deflated zeros was CARRIED, with no rejection, at 892 MB of peak RSS --
|
||||||
|
about 2 100x the file size, linear, so a 10 MB document is ~21 GB and takes
|
||||||
|
the whole batch build with it. `check_size` was reading a claim as though
|
||||||
|
it were a cost.
|
||||||
|
|
||||||
|
The output is inflated a chunk at a time and DISCARDED: only the running
|
||||||
|
total is kept, so the measurement stays bounded whatever the stream holds,
|
||||||
|
and it stops at the first chunk that crosses `limit`. A legitimate image is
|
||||||
|
therefore inflated twice -- once here and once by the reader that carries
|
||||||
|
it -- which is the price of not holding an unbounded buffer to find out how
|
||||||
|
big it is. Measured on R761 Prosesskoden:2025 (50 image objects): the
|
||||||
|
second pass costs under a second of a 200-second extraction.
|
||||||
|
|
||||||
|
A stream that is not valid deflate data is not this function's problem: the
|
||||||
|
reader behind it reports that in its own vocabulary, so a `zlib.error` ends
|
||||||
|
the measurement at whatever was produced up to it.
|
||||||
|
"""
|
||||||
|
bound = MAX_IMAGE_BYTES if limit is None else limit
|
||||||
|
decompressor = zlib.decompressobj()
|
||||||
|
total = 0
|
||||||
|
pending = raw
|
||||||
|
try:
|
||||||
|
while True:
|
||||||
|
produced = decompressor.decompress(pending, _INFLATE_CHUNK)
|
||||||
|
total += len(produced)
|
||||||
|
if total > bound:
|
||||||
|
raise ExtractionError(
|
||||||
|
f"the stream behind {name!r} decompresses to more than {bound} "
|
||||||
|
f"bytes from {len(raw)} bytes of input, over this package's "
|
||||||
|
"bound; refused without being held, because the size a "
|
||||||
|
"container declares is a claim and this is the cost",
|
||||||
|
code="asset_too_large",
|
||||||
|
)
|
||||||
|
pending = decompressor.unconsumed_tail
|
||||||
|
if decompressor.eof or not pending:
|
||||||
|
break
|
||||||
|
except zlib.error:
|
||||||
|
return total
|
||||||
|
return total
|
||||||
|
|
||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
class ExtractedImage:
|
class ExtractedImage:
|
||||||
"""One image a document carries, as this package will write it.
|
"""One image a document carries, as this package will write it.
|
||||||
|
|
@ -310,6 +385,16 @@ def read_image(data: bytes, *, name: str, label: str | None = None) -> Extracted
|
||||||
)
|
)
|
||||||
media_type, suffix = kind
|
media_type, suffix = kind
|
||||||
size = dimensions(data)
|
size = dimensions(data)
|
||||||
|
# THE BOUND HOLDS FOR A FILE CARRIED VERBATIM TOO. This package does not
|
||||||
|
# decode one, so it pays nothing for it -- but writing a 7 000 x 7 000 PNG
|
||||||
|
# of 47 705 bytes into a bundle hands the consumer the same bomb with
|
||||||
|
# `7000x7000 px` printed beside it, and the README's first sentence about
|
||||||
|
# this bound says such an image is refused. Sniffed from the header rather
|
||||||
|
# than claimed, like the type beside it. Measured: the largest of the
|
||||||
|
# 4 828 objects in the reference corpus is 18.6 MP, so nothing anyone has
|
||||||
|
# measured is refused here.
|
||||||
|
if size is not None:
|
||||||
|
check_size(size[0], size[1], name=name)
|
||||||
return ExtractedImage(
|
return ExtractedImage(
|
||||||
data=data,
|
data=data,
|
||||||
name=name,
|
name=name,
|
||||||
|
|
@ -527,22 +612,37 @@ def render_missing(
|
||||||
absence they were never shown -- which is precisely the shape of the defect
|
absence they were never shown -- which is precisely the shape of the defect
|
||||||
this whole capability exists to close.
|
this whole capability exists to close.
|
||||||
|
|
||||||
The href is kept when the source had one and it survives the link grammar,
|
The href is kept when the source had one, so a remote figure says WHERE it
|
||||||
so a remote figure says WHERE it was. Extraction never opens a socket: the
|
was. Extraction never opens a socket: the network gate is an explicit
|
||||||
network gate is an explicit per-run opt-in and extraction is not on that
|
per-run opt-in and extraction is not on that path, so a remote source is
|
||||||
path, so a remote source is carried as a name and never as bytes.
|
carried as a name and never as bytes.
|
||||||
|
|
||||||
|
`label` is the alt text or the figure caption, and it is written for the
|
||||||
|
same reason the line exists at all: the reader deciding whether the missing
|
||||||
|
picture mattered is much better served by "Figur 84-1 Tverrprofil" than by
|
||||||
|
a file name. 0.10.1 dropped it while closing the live-link defect -- the
|
||||||
|
parameter stayed in the signature and no branch read it -- which an
|
||||||
|
independent review measured as a regression against 0.10.0.
|
||||||
"""
|
"""
|
||||||
|
# INERT, never ``. A remote reference is an address the
|
||||||
|
# document's author chose, and 0.10.0 wrote it as a live markdown image: a
|
||||||
|
# consumer that renders the bundle, or an agent that fetches what it
|
||||||
|
# renders, turns "this bundle was opened" into a beacon to them -- and a
|
||||||
|
# server-side consumer into an SSRF. This package opens no socket, which is
|
||||||
|
# not the same as the pointer being harmless.
|
||||||
|
#
|
||||||
|
# The address is STATED, because a reader has to know what stood there --
|
||||||
|
# and stated ONCE, inside a code span. 0.10.1 wrote it twice: once in the
|
||||||
|
# span and once as the bare `name`, and a GFM/linkify renderer autolinks a
|
||||||
|
# bare URL into `<a href="...">`. It takes a click rather than a render, so
|
||||||
|
# it is weaker than `![..]()` -- but "inert" was half the truth, and half
|
||||||
|
# is what this line exists not to be.
|
||||||
|
if href:
|
||||||
|
address = _inline(href).replace("`", "'")
|
||||||
|
shown = f"`{address}`"
|
||||||
|
else:
|
||||||
shown = _inline(name or "image")
|
shown = _inline(name or "image")
|
||||||
detail = f"Image: {shown} (not carried: {_inline(reason)})"
|
detail = f"Image: {shown} (not carried: {_inline(reason)})"
|
||||||
if href:
|
if label:
|
||||||
# INERT, never ``. A remote reference is an address the
|
detail += f" -- {_inline(label)}"
|
||||||
# document's author chose, and 0.10.0 wrote it as a live markdown
|
|
||||||
# image: a consumer that renders the bundle, or an agent that fetches
|
|
||||||
# what it renders, turns "this bundle was opened" into a beacon to
|
|
||||||
# them -- and a server-side consumer into an SSRF. This package opens
|
|
||||||
# no socket, which is not the same as the pointer being harmless.
|
|
||||||
# The address is still STATED, in a code span, because a reader has to
|
|
||||||
# know what stood there to judge whether the picture mattered.
|
|
||||||
address = _inline(href).replace("`", "'")
|
|
||||||
return f"{detail} address: `{address}`"
|
|
||||||
return detail
|
return detail
|
||||||
|
|
|
||||||
|
|
@ -133,6 +133,22 @@ class ExtractionError(IngestError):
|
||||||
- `asset_samples_invalid` — the sample buffer does not fit the dimensions
|
- `asset_samples_invalid` — the sample buffer does not fit the dimensions
|
||||||
the image dictionary declares. Refused rather than padded: a short buffer
|
the image dictionary declares. Refused rather than padded: a short buffer
|
||||||
means the dictionary was read wrong
|
means the dictionary was read wrong
|
||||||
|
- `asset_too_large` — the picture is over this package's bound, either
|
||||||
|
because it DECLARES a size beyond `MAX_IMAGE_PIXELS`, because the file
|
||||||
|
itself is that large, or because the stream behind it DECOMPRESSES to
|
||||||
|
more than `MAX_IMAGE_BYTES`. The three are one code because they are one
|
||||||
|
decision — this run will not hold that picture — and because a consumer
|
||||||
|
counting refusals wants the picture, not the mechanism. The bound is read
|
||||||
|
off the corpora and sits an order of magnitude above anything measured
|
||||||
|
- `asset_size_invalid` — the container declares a size that is not a size:
|
||||||
|
a zero or negative `/Width` or `/Height`. DISTINCT from
|
||||||
|
`asset_too_large`, because the two say different things about the
|
||||||
|
document — one is a legitimate publisher shipping a picture bigger than
|
||||||
|
this package carries, the other is a dictionary written wrong or written
|
||||||
|
to be read wrong — and counting them together would make a corpus
|
||||||
|
statistic about the first untrue. Refused before the stream is read: a
|
||||||
|
negative dimension multiplies to a negative pixel count, under which
|
||||||
|
every bound reads as satisfied
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -48,6 +48,7 @@ from .assets import (
|
||||||
check_payload,
|
check_payload,
|
||||||
check_size,
|
check_size,
|
||||||
encode_png,
|
encode_png,
|
||||||
|
inflated_size,
|
||||||
read_image,
|
read_image,
|
||||||
render_block,
|
render_block,
|
||||||
render_missing,
|
render_missing,
|
||||||
|
|
@ -1365,6 +1366,40 @@ def _pdf_alpha(attrs: dict[str, object], width: int, height: int) -> bytes | Non
|
||||||
return alpha if len(alpha) >= width * height else False
|
return alpha if len(alpha) >= width * height else False
|
||||||
|
|
||||||
|
|
||||||
|
def _check_inflated(stream: object, name: str) -> None:
|
||||||
|
"""Refuse a PDF image stream whose DECOMPRESSED size is over the bound.
|
||||||
|
|
||||||
|
`check_size` reads `/Width` and `/Height`, which an untrusted document
|
||||||
|
writes and which say nothing about what `get_data()` will return: `/Length`
|
||||||
|
is the COMPRESSED length, and a dictionary declaring 1x1 may hang 400 MB of
|
||||||
|
deflated zeros off it. Measured by an independent review of 0.10.1 on
|
||||||
|
`230d1cb`: that document is 408 516 bytes, was carried without a rejection,
|
||||||
|
and cost 892 MB of peak RSS -- the same failure mode the 0.10.0 review
|
||||||
|
named, reached through the other number.
|
||||||
|
|
||||||
|
WHAT THIS BOUNDS, and what it does not. The measurement runs on the RAW
|
||||||
|
stream, so it applies where `FlateDecode` is the first filter -- the
|
||||||
|
overwhelming case, and the one every deflate bomb needs. It does not run
|
||||||
|
when the stream is already decoded, when the document is encrypted (the raw
|
||||||
|
bytes are ciphertext until pdfminer deciphers them), or when a chain puts
|
||||||
|
another filter in front. Those are caught by `check_payload` AFTER
|
||||||
|
`get_data()`, which makes them a counted refusal rather than a bounded one:
|
||||||
|
the memory is spent and then the picture is dropped. Stated rather than
|
||||||
|
implied, because the difference is exactly what the review found missing.
|
||||||
|
"""
|
||||||
|
from pdfminer.pdftypes import LITERALS_FLATE_DECODE
|
||||||
|
|
||||||
|
if getattr(stream, "decipher", None) is not None:
|
||||||
|
return
|
||||||
|
raw = stream.get_rawdata() if hasattr(stream, "get_rawdata") else None
|
||||||
|
if not raw:
|
||||||
|
return
|
||||||
|
filters = stream.get_filters() if hasattr(stream, "get_filters") else []
|
||||||
|
if not filters or filters[0][0] not in LITERALS_FLATE_DECODE:
|
||||||
|
return
|
||||||
|
inflated_size(raw, name=name)
|
||||||
|
|
||||||
|
|
||||||
def _pdf_image(stream: object, name: str) -> ExtractedImage:
|
def _pdf_image(stream: object, name: str) -> ExtractedImage:
|
||||||
"""One image XObject, carried verbatim where it already is a file.
|
"""One image XObject, carried verbatim where it already is a file.
|
||||||
|
|
||||||
|
|
@ -1394,16 +1429,22 @@ def _pdf_image(stream: object, name: str) -> ExtractedImage:
|
||||||
|
|
||||||
# THE DECLARED SIZE IS READ FIRST, and the stream is not touched until it
|
# THE DECLARED SIZE IS READ FIRST, and the stream is not touched until it
|
||||||
# is within the bound: `get_data()` decompresses, so a check after it has
|
# is within the bound: `get_data()` decompresses, so a check after it has
|
||||||
# already paid for a picture of compressed zeros.
|
# already paid for a picture of compressed zeros. `channels=1` because the
|
||||||
|
# colour space is not resolved until further down and one channel is the
|
||||||
|
# LOWEST estimate of the cost -- a bound that under-counts refuses nothing
|
||||||
|
# it should carry, and the pixel count beside it does not depend on it.
|
||||||
|
# What that leaves unbounded is the stream, which `_check_inflated` reads
|
||||||
|
# for what it actually costs rather than for what it claims.
|
||||||
declared = dict(getattr(stream, "attrs", {}))
|
declared = dict(getattr(stream, "attrs", {}))
|
||||||
|
declared_width = resolve1(declared.get("Width"))
|
||||||
|
declared_height = resolve1(declared.get("Height"))
|
||||||
check_size(
|
check_size(
|
||||||
resolve1(declared.get("Width")) if isinstance(resolve1(declared.get("Width")), int) else 0,
|
declared_width if isinstance(declared_width, int) else None,
|
||||||
resolve1(declared.get("Height"))
|
declared_height if isinstance(declared_height, int) else None,
|
||||||
if isinstance(resolve1(declared.get("Height")), int)
|
|
||||||
else 0,
|
|
||||||
name=name,
|
name=name,
|
||||||
channels=1,
|
channels=1,
|
||||||
)
|
)
|
||||||
|
_check_inflated(stream, name)
|
||||||
try:
|
try:
|
||||||
data = stream.get_data() # type: ignore[attr-defined]
|
data = stream.get_data() # type: ignore[attr-defined]
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
|
|
@ -1411,6 +1452,10 @@ def _pdf_image(stream: object, name: str) -> ExtractedImage:
|
||||||
f"the PDF image stream behind {name!r} could not be decoded: {exc}",
|
f"the PDF image stream behind {name!r} could not be decoded: {exc}",
|
||||||
code="asset_pdf_unsupported",
|
code="asset_pdf_unsupported",
|
||||||
) from exc
|
) from exc
|
||||||
|
# THE BACKSTOP, and it is a weaker guarantee than the one above it: this
|
||||||
|
# one COUNTS a stream that `_check_inflated` could not bound in advance
|
||||||
|
# (see its docstring for which those are), after the memory has been spent.
|
||||||
|
check_payload(len(data), name=name)
|
||||||
if data and sniff(data) is not None:
|
if data and sniff(data) is not None:
|
||||||
return read_image(data, name=name)
|
return read_image(data, name=name)
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -29,6 +29,8 @@ killed build.
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import re
|
import re
|
||||||
|
import subprocess
|
||||||
|
import sys
|
||||||
import zlib
|
import zlib
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
|
|
@ -126,10 +128,26 @@ def test_a_built_bundle_carries_no_foreign_image_link(tmp_path: Path) -> None:
|
||||||
# --- finding 2: a declared size that is too large is refused, not decoded ---
|
# --- finding 2: a declared size that is too large is refused, not decoded ---
|
||||||
|
|
||||||
|
|
||||||
def _bomb(dimension: int, content: bytes | None = None) -> bytes:
|
def _zeros_stream(total: int) -> bytes:
|
||||||
|
"""`total` bytes of zeros, deflated WITHOUT ever holding them.
|
||||||
|
|
||||||
|
The generator has to stay cheaper than the bomb it builds, or the test
|
||||||
|
measures its own fixture instead of the code under test.
|
||||||
|
"""
|
||||||
|
compressor = zlib.compressobj(9)
|
||||||
|
chunk = b"\x00" * (1 << 20)
|
||||||
|
parts = [compressor.compress(chunk) for _ in range(total >> 20)]
|
||||||
|
parts.append(compressor.flush())
|
||||||
|
return b"".join(parts)
|
||||||
|
|
||||||
|
|
||||||
|
def _bomb(dimension: int, content: bytes | None = None, payload: bytes | None = None) -> bytes:
|
||||||
"""A tiny PDF declaring one `dimension` x `dimension` grayscale image of
|
"""A tiny PDF declaring one `dimension` x `dimension` grayscale image of
|
||||||
compressed zeros -- the review's own repro, built here. `content` replaces
|
compressed zeros -- the review's own repro, built here. `content` replaces
|
||||||
the page's content stream, for a page that draws an INLINE image instead."""
|
the page's content stream, for a page that draws an INLINE image instead.
|
||||||
|
`payload` replaces the image stream, for a document whose DECLARED size and
|
||||||
|
whose actual stream are two different numbers."""
|
||||||
|
if payload is None:
|
||||||
payload = zlib.compress(b"\x00" * (dimension * dimension), 9)
|
payload = zlib.compress(b"\x00" * (dimension * dimension), 9)
|
||||||
if content is None:
|
if content is None:
|
||||||
content = b"BT /F1 12 Tf 20 100 Td (bomb) Tj ET\nq 100 0 0 100 20 20 cm /Im0 Do Q\n"
|
content = b"BT /F1 12 Tf 20 100 Td (bomb) Tj ET\nq 100 0 0 100 20 20 cm /Im0 Do Q\n"
|
||||||
|
|
@ -247,3 +265,198 @@ def test_an_inline_pdf_image_gets_a_stable_name() -> None:
|
||||||
again = [image.name for image in second.images] + [r.name for r in second.rejected]
|
again = [image.name for image in second.images] + [r.name for r in second.rejected]
|
||||||
assert names == again != []
|
assert names == again != []
|
||||||
assert not any(part.isdigit() and len(part) > 6 for name in names for part in name.split("-"))
|
assert not any(part.isdigit() and len(part) > 6 for name in names for part in name.split("-"))
|
||||||
|
|
||||||
|
|
||||||
|
# --- BLOCKER-1 of the 18.09 review: the bound must bind what the run PAYS ----
|
||||||
|
#
|
||||||
|
# `check_size` reads `/Width` and `/Height` out of the image dictionary, which
|
||||||
|
# is a CLAIM by an untrusted document, and the claim and the cost are two
|
||||||
|
# independent numbers: `/Length` is the COMPRESSED length and nothing in the
|
||||||
|
# dictionary states what `get_data()` will return. Measured on `230d1cb` by an
|
||||||
|
# independent review: a 389 626-byte PDF declaring 1x1 and carrying 400 MB of
|
||||||
|
# deflated zeros was CARRIED, with no rejection, at 834 MB of peak RSS -- and
|
||||||
|
# 1,2 GB of zeros at 2 436 MB, linear, about 2 100x the file size. The four
|
||||||
|
# mutations that suite already kills do not separate declared from actual, so
|
||||||
|
# they were all green while this stood.
|
||||||
|
|
||||||
|
#: What a bounded run of the 400 MB bomb may cost, in bytes of peak RSS.
|
||||||
|
#: Measured 2026-09-18 on this machine, same commit, same fixture: 892 MB
|
||||||
|
#: without the bound and 54 MB with it, and the bounded figure barely moves
|
||||||
|
#: when the stream triples (62 MB at 1,2 GB) because what grows is the
|
||||||
|
#: COMPRESSED input, which was already in memory. The bar sits between the
|
||||||
|
#: two, far enough above the bounded run that the interpreter's own footprint
|
||||||
|
#: on another machine cannot reach it.
|
||||||
|
PEAK_RSS_BOUND = 256 * 1024 * 1024
|
||||||
|
|
||||||
|
#: The stream the bomb inflates to. Over `MAX_IMAGE_BYTES` (256 MiB), so it is
|
||||||
|
#: refused at the real bound rather than at a monkeypatched one.
|
||||||
|
BOMB_STREAM_BYTES = 400 * 1024 * 1024
|
||||||
|
|
||||||
|
_CHILD = """
|
||||||
|
import resource, sys
|
||||||
|
sys.path.insert(0, {tests!r})
|
||||||
|
from test_asset_limits import _bomb, _zeros_stream
|
||||||
|
from llm_ingestion_okf.extract import extract_document
|
||||||
|
|
||||||
|
document = _bomb(1, payload=_zeros_stream({total}))
|
||||||
|
extracted = extract_document("bomb.pdf", document, assets=True)
|
||||||
|
peak = resource.getrusage(resource.RUSAGE_SELF).ru_maxrss
|
||||||
|
print(
|
||||||
|
len(document),
|
||||||
|
len(extracted.images),
|
||||||
|
",".join(rejection.code for rejection in extracted.rejected) or "-",
|
||||||
|
peak if sys.platform == "darwin" else peak * 1024,
|
||||||
|
)
|
||||||
|
"""
|
||||||
|
|
||||||
|
|
||||||
|
def _run_bomb(total: int) -> tuple[int, int, str, int]:
|
||||||
|
"""The bomb in its own interpreter, so peak RSS is ITS peak and not the
|
||||||
|
high-water mark of every test that ran before it."""
|
||||||
|
completed = subprocess.run(
|
||||||
|
[sys.executable, "-c", _CHILD.format(tests=str(Path(__file__).parent), total=total)],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
check=True,
|
||||||
|
)
|
||||||
|
size, carried, codes, peak = completed.stdout.split()
|
||||||
|
return int(size), int(carried), codes, int(peak)
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_declared_size_of_one_pixel_does_not_licence_an_unbounded_stream() -> None:
|
||||||
|
"""The review's repro, at the shipped bound: 1x1 declared, 400 MB paid."""
|
||||||
|
pytest.importorskip("pdfplumber")
|
||||||
|
size, carried, codes, peak = _run_bomb(BOMB_STREAM_BYTES)
|
||||||
|
assert size < 2 * 1024 * 1024, "the fixture must stay a small file, or it proves nothing"
|
||||||
|
assert carried == 0, "a 400 MB stream was carried as a 1x1 picture"
|
||||||
|
assert codes == "asset_too_large"
|
||||||
|
assert peak < PEAK_RSS_BOUND, f"peak RSS {peak} bytes for a {size}-byte file"
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_refusal_reads_the_stream_and_not_only_the_declaration() -> None:
|
||||||
|
"""The mutation the shipped suite could not kill.
|
||||||
|
|
||||||
|
An honest 20000x20000 declaration is refused by `check_size` alone, so a
|
||||||
|
test built on one is green whether or not the actual stream is bounded.
|
||||||
|
This document declares a size WITHIN the bound, which is the only shape
|
||||||
|
that separates the two checks.
|
||||||
|
"""
|
||||||
|
pytest.importorskip("pdfplumber")
|
||||||
|
small = zlib.compress(b"\x00" * 64, 9)
|
||||||
|
within = extract_document("small.pdf", _bomb(8, payload=small), assets=True)
|
||||||
|
assert [rejection.code for rejection in within.rejected] == [], "the control must be carried"
|
||||||
|
assert len(within.images) == 1
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_stream_over_the_bound_is_refused_with_a_patched_bound() -> None:
|
||||||
|
"""The same rule, cheap, so it runs on every machine and every suite."""
|
||||||
|
pytest.importorskip("pdfplumber")
|
||||||
|
monkey = pytest.MonkeyPatch()
|
||||||
|
try:
|
||||||
|
monkey.setattr(assets, "MAX_IMAGE_BYTES", 4096)
|
||||||
|
extracted = extract_document(
|
||||||
|
"bomb.pdf", _bomb(1, payload=zlib.compress(b"\x00" * 1_000_000, 9)), assets=True
|
||||||
|
)
|
||||||
|
finally:
|
||||||
|
monkey.undo()
|
||||||
|
assert extracted.images == ()
|
||||||
|
assert [rejection.code for rejection in extracted.rejected] == ["asset_too_large"]
|
||||||
|
|
||||||
|
|
||||||
|
# --- MAJOR of the 18.09 review: a non-positive declaration is not a size -----
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_non_positive_declared_size_is_refused_before_the_stream_is_read() -> None:
|
||||||
|
"""`-1 * 40_000_000_000` is NEGATIVE, so `pixels > MAX_IMAGE_PIXELS` was
|
||||||
|
false and `check_size` returned silently; 400 MB was then decompressed and
|
||||||
|
the refusal came from `encode_png` with `asset_samples_invalid` -- a code
|
||||||
|
about the sample buffer for a defect in the declaration.
|
||||||
|
|
||||||
|
The stream here is CORRUPT, so the order is observable: reading first gives
|
||||||
|
`asset_pdf_unsupported`, reading the declaration first gives the new code.
|
||||||
|
"""
|
||||||
|
pytest.importorskip("pdfplumber")
|
||||||
|
document = _bomb(4, payload=b"\xff" * 512).replace(
|
||||||
|
b"/Width 4 /Height 4", b"/Width -1 /Height 40000000000"
|
||||||
|
)
|
||||||
|
assert b"/Width -1" in document
|
||||||
|
extracted = extract_document("negative.pdf", document, assets=True)
|
||||||
|
assert extracted.images == ()
|
||||||
|
assert [rejection.code for rejection in extracted.rejected] == ["asset_size_invalid"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_check_size_refuses_every_non_positive_pair_and_keeps_unknown_unknown() -> None:
|
||||||
|
for width, height in ((-1, 40_000_000_000), (0, 10), (10, 0), (-2, -2)):
|
||||||
|
with pytest.raises(ExtractionError) as excinfo:
|
||||||
|
assets.check_size(width, height, name="n")
|
||||||
|
assert excinfo.value.code == "asset_size_invalid"
|
||||||
|
# A size the container never declared is UNKNOWN, not invalid: there is no
|
||||||
|
# number to bound and inventing one would refuse a legitimate picture.
|
||||||
|
assets.check_size(None, None, name="n")
|
||||||
|
assets.check_size(None, 10, name="n")
|
||||||
|
|
||||||
|
|
||||||
|
# --- MINOR-3: the bound holds for a file carried verbatim, too ---------------
|
||||||
|
|
||||||
|
|
||||||
|
def _png_header(width: int, height: int) -> bytes:
|
||||||
|
"""A PNG whose IHDR declares `width` x `height` and whose body is a stub.
|
||||||
|
`read_image` sniffs and reads the header; it never decodes."""
|
||||||
|
|
||||||
|
def chunk(kind: bytes, payload: bytes) -> bytes:
|
||||||
|
return (
|
||||||
|
len(payload).to_bytes(4, "big")
|
||||||
|
+ kind
|
||||||
|
+ payload
|
||||||
|
+ zlib.crc32(kind + payload).to_bytes(4, "big")
|
||||||
|
)
|
||||||
|
|
||||||
|
ihdr = width.to_bytes(4, "big") + height.to_bytes(4, "big") + bytes([8, 0, 0, 0, 0])
|
||||||
|
return b"\x89PNG\r\n\x1a\n" + chunk(b"IHDR", ihdr) + chunk(b"IEND", b"")
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_image_file_over_the_bound_is_refused_although_it_is_never_decoded() -> None:
|
||||||
|
"""A 7 000 x 7 000 PNG is 49 MP in 47 705 bytes. This package does not
|
||||||
|
decode a carried file, so it pays nothing -- but writing it into a bundle
|
||||||
|
hands the consumer the same bomb with `7000x7000 px` printed beside it,
|
||||||
|
and the README's first sentence says such an image is refused."""
|
||||||
|
with pytest.raises(ExtractionError) as excinfo:
|
||||||
|
assets.read_image(_png_header(7000, 7000), name="big.png")
|
||||||
|
assert excinfo.value.code == "asset_too_large"
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_image_file_under_the_bound_is_still_read() -> None:
|
||||||
|
image = assets.read_image(_png_header(4515, 4128), name="drawing.png")
|
||||||
|
assert (image.width, image.height) == (4515, 4128)
|
||||||
|
|
||||||
|
|
||||||
|
# --- MINOR-1 and MINOR-2 of the 18.09 review --------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_remote_address_is_never_written_as_a_bare_url() -> None:
|
||||||
|
"""`inert` was only half true: the address was written TWICE, once in a
|
||||||
|
code span and once bare, and a GFM/linkify renderer autolinks the bare
|
||||||
|
one into `<a href="...">`. Measured with `markdown_it('gfm-like')`."""
|
||||||
|
line = render_missing(REMOTE, reason="the source is off this machine", href=REMOTE)
|
||||||
|
assert "](" not in line
|
||||||
|
assert REMOTE in line
|
||||||
|
for position in range(len(line)):
|
||||||
|
if line.startswith(REMOTE, position):
|
||||||
|
assert line[position - 1] == "`" and line[position + len(REMOTE)] == "`", (
|
||||||
|
f"a bare occurrence of the address at {position}: {line!r}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_caption_of_a_remote_reference_is_still_stated() -> None:
|
||||||
|
"""`label` became a dead parameter in 0.10.1, so the alt text or figure
|
||||||
|
caption of an image the bundle does not carry was DROPPED -- a regression
|
||||||
|
against 0.10.0 and against this module's own reason for writing the line:
|
||||||
|
a reader cannot weigh an absence they were never shown."""
|
||||||
|
line = render_missing(
|
||||||
|
"p.gif", reason="the source is off this machine", label="Figur 84-1 Tverrprofil", href=None
|
||||||
|
)
|
||||||
|
assert "Figur 84-1 Tverrprofil" in line
|
||||||
|
with_href = render_missing(
|
||||||
|
REMOTE, reason="the source is off this machine", label="Figur 84-1 Tverrprofil", href=REMOTE
|
||||||
|
)
|
||||||
|
assert "Figur 84-1 Tverrprofil" in with_href
|
||||||
|
|
|
||||||
|
|
@ -728,6 +728,38 @@ def test_asset_samples_invalid() -> None:
|
||||||
assert excinfo.value.code == "asset_samples_invalid"
|
assert excinfo.value.code == "asset_samples_invalid"
|
||||||
|
|
||||||
|
|
||||||
|
def test_asset_too_large() -> None:
|
||||||
|
"""One code, three ways to be over the bound: a DECLARED size, a file, and
|
||||||
|
a stream that decompresses past it. The third arrived in 0.10.1 after an
|
||||||
|
independent review measured a 408 516-byte PDF declaring 1x1 being carried
|
||||||
|
at 892 MB of peak RSS."""
|
||||||
|
import zlib
|
||||||
|
|
||||||
|
from llm_ingestion_okf import assets
|
||||||
|
|
||||||
|
with pytest.raises(ExtractionError) as excinfo:
|
||||||
|
assets.check_size(20_000, 20_000, name="declared")
|
||||||
|
assert excinfo.value.code == "asset_too_large"
|
||||||
|
|
||||||
|
with pytest.raises(ExtractionError) as excinfo:
|
||||||
|
assets.check_payload(assets.MAX_IMAGE_BYTES + 1, name="file")
|
||||||
|
assert excinfo.value.code == "asset_too_large"
|
||||||
|
|
||||||
|
with pytest.raises(ExtractionError) as excinfo:
|
||||||
|
assets.inflated_size(zlib.compress(b"\x00" * 4096, 9), name="stream", limit=16)
|
||||||
|
assert excinfo.value.code == "asset_too_large"
|
||||||
|
|
||||||
|
|
||||||
|
def test_asset_size_invalid() -> None:
|
||||||
|
"""A declared size that is not a size. Its own code because it says
|
||||||
|
something different about the document than `asset_too_large` does."""
|
||||||
|
from llm_ingestion_okf import assets
|
||||||
|
|
||||||
|
with pytest.raises(ExtractionError) as excinfo:
|
||||||
|
assets.check_size(-1, 40_000_000_000, name="negative")
|
||||||
|
assert excinfo.value.code == "asset_size_invalid"
|
||||||
|
|
||||||
|
|
||||||
def test_asset_remote() -> None:
|
def test_asset_remote() -> None:
|
||||||
from llm_ingestion_okf.extract import extract_document
|
from llm_ingestion_okf.extract import extract_document
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -6,6 +6,7 @@ py.typed marker mypy degrades every imported symbol to Any.
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
@ -106,3 +107,71 @@ def test_operational_tooling_stays_out_of_the_wheel() -> None:
|
||||||
packages = pyproject["tool"]["hatch"]["build"]["targets"]["wheel"]["packages"]
|
packages = pyproject["tool"]["hatch"]["build"]["targets"]["wheel"]["packages"]
|
||||||
assert packages == ["src/llm_ingestion_okf"]
|
assert packages == ["src/llm_ingestion_okf"]
|
||||||
assert (PROJECT_ROOT / "tools" / "okf_watch.py").is_file(), "the test must have a subject"
|
assert (PROJECT_ROOT / "tools" / "okf_watch.py").is_file(), "the test must have a subject"
|
||||||
|
|
||||||
|
|
||||||
|
def test_every_place_that_publishes_a_version_names_the_packaged_one() -> None:
|
||||||
|
"""A tag is a promise about bytes, and nine places here repeat it.
|
||||||
|
|
||||||
|
`test_the_declared_version_agrees_with_the_packaged_one` binds two of
|
||||||
|
them. An independent review of 0.10.1 found the other seven unbound and
|
||||||
|
all of them stale: four README install lines telling a consumer to install
|
||||||
|
`@v0.10.0`, two prose lines about what that tag declares, the "current tag"
|
||||||
|
entry, and a CHANGELOG whose 0.10.1 content sat under `[Unreleased]`. A
|
||||||
|
`v0.10.1` tag cut from that tree would have shipped a package reporting
|
||||||
|
0.10.0 and a README installing the release before it -- and the suite was
|
||||||
|
green, because nothing looked.
|
||||||
|
|
||||||
|
The guard tag is bound the same way and for the same reason: the README
|
||||||
|
tells a plain-pip user to install a specific guard tag first, and that
|
||||||
|
instruction is wrong the moment `[tool.uv.sources]` moves without it.
|
||||||
|
"""
|
||||||
|
tomllib = pytest.importorskip("tomllib")
|
||||||
|
pyproject = tomllib.loads((PROJECT_ROOT / "pyproject.toml").read_text(encoding="utf-8"))
|
||||||
|
version = llm_ingestion_okf.__version__
|
||||||
|
guard = pyproject["tool"]["uv"]["sources"]["llm-ingestion-guard"]["tag"]
|
||||||
|
readme = (PROJECT_ROOT / "README.md").read_text(encoding="utf-8")
|
||||||
|
|
||||||
|
install = re.findall(r"llm-ingestion-okf\.git@(v[0-9][^\"\s]*)", readme)
|
||||||
|
assert install, "the test must have a subject"
|
||||||
|
assert set(install) == {f"v{version}"}, f"install lines name {sorted(set(install))}"
|
||||||
|
|
||||||
|
guard_lines = re.findall(r"llm-ingestion-pipeline-security\.git@(v[0-9][^\"\s]*)", readme)
|
||||||
|
assert guard_lines, "the test must have a subject"
|
||||||
|
assert set(guard_lines) == {guard}, f"guard install lines name {sorted(set(guard_lines))}"
|
||||||
|
|
||||||
|
current = re.search(r"^- `(v[^`]+)` — the current tag", readme, re.MULTILINE)
|
||||||
|
assert current is not None, "the test must have a subject"
|
||||||
|
assert current.group(1) == f"v{version}"
|
||||||
|
|
||||||
|
# The prose between "## Install in detail" and the history list explains
|
||||||
|
# what THIS tag declares and which guard tag it is paired to. A stale
|
||||||
|
# number there is an instruction that fails, not a historical note.
|
||||||
|
detail = readme.split("## Install in detail", 1)[1].split("### Earlier tags, as history", 1)[0]
|
||||||
|
named = set(re.findall(r"`(v\d+\.\d+\.\d+[^`]*)`", detail))
|
||||||
|
assert named, "the test must have a subject"
|
||||||
|
assert named <= {f"v{version}", guard}, f"stale tags in the install prose: {sorted(named)}"
|
||||||
|
|
||||||
|
# The tenth place, which uv rewrites on its own and which is therefore the
|
||||||
|
# easiest of all to commit stale.
|
||||||
|
lock = (PROJECT_ROOT / "uv.lock").read_text(encoding="utf-8")
|
||||||
|
locked = re.search(r'name = "llm-ingestion-okf"\nversion = "([^"]+)"', lock)
|
||||||
|
assert locked is not None, "the test must have a subject"
|
||||||
|
assert locked.group(1) == version
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_changelog_heads_with_the_packaged_version() -> None:
|
||||||
|
"""The release notes for the version being shipped are not `[Unreleased]`.
|
||||||
|
|
||||||
|
`[Unreleased]` is the right place for work in flight and the wrong place
|
||||||
|
for the content of a tag someone is about to cut: a reader arriving at
|
||||||
|
`v0.10.1` would find its own entry under a heading saying it had not been
|
||||||
|
released. Found by an independent review of 0.10.1.
|
||||||
|
"""
|
||||||
|
changelog = (PROJECT_ROOT / "CHANGELOG.md").read_text(encoding="utf-8")
|
||||||
|
heading = re.search(r"^## \[([^\]]+)\](?: — (\d{4}-\d{2}-\d{2}))?$", changelog, re.MULTILINE)
|
||||||
|
assert heading is not None, "the test must have a subject"
|
||||||
|
assert heading.group(1) == llm_ingestion_okf.__version__, (
|
||||||
|
f"the changelog heads with [{heading.group(1)}], the package is "
|
||||||
|
f"{llm_ingestion_okf.__version__}"
|
||||||
|
)
|
||||||
|
assert heading.group(2), "a released section carries its date"
|
||||||
|
|
|
||||||
|
|
@ -395,6 +395,7 @@ REJECTION_CODES: frozenset[str] = frozenset(
|
||||||
"asset_pdf_unsupported",
|
"asset_pdf_unsupported",
|
||||||
"asset_remote",
|
"asset_remote",
|
||||||
"asset_samples_invalid",
|
"asset_samples_invalid",
|
||||||
|
"asset_size_invalid",
|
||||||
"asset_too_large",
|
"asset_too_large",
|
||||||
"asset_type_unknown",
|
"asset_type_unknown",
|
||||||
"asset_unresolved",
|
"asset_unresolved",
|
||||||
|
|
|
||||||
2
uv.lock
generated
2
uv.lock
generated
|
|
@ -547,7 +547,7 @@ source = { git = "https://git.fromaitochitta.com/open/llm-ingestion-pipeline-sec
|
||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "llm-ingestion-okf"
|
name = "llm-ingestion-okf"
|
||||||
version = "0.10.0"
|
version = "0.10.1"
|
||||||
source = { editable = "." }
|
source = { editable = "." }
|
||||||
dependencies = [
|
dependencies = [
|
||||||
{ name = "llm-ingestion-guard" },
|
{ name = "llm-ingestion-guard" },
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue