From 6d7abbd7acc35b2454a459f2def7071540705397 Mon Sep 17 00:00:00 2001 From: Ronan Lenouvel Date: Thu, 16 Jul 2026 22:52:26 +0200 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=93=96=20docs(contributing):=20guide?= =?UTF-8?q?=20de=20contribution?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documente les conventions telles qu'elles sont réellement appliquées ici, pas des généralités : zéro dépendance runtime et ses deux corollaires, TDD strict, seuil de couverture, règles de parsing binaire, workflow issue → branche → PR. Trois sections tirent les leçons des bugs de ce projet : - « forged bytes prove less than you think » : les quatre bugs trouvés étaient tous invisibles à une suite verte, parce qu'un test écrit à partir d'une incompréhension reproduit l'incompréhension ; - la couverture : avant d'ajouter un test pour couvrir une ligne, se demander si la ligne mérite d'exister — ici le passage de 83 % à 99 % s'est fait surtout en supprimant du code mort ; - corrompu ≠ absent : un tag qui ment sur son contenu est courant dans les RAW, ce n'est pas une corruption. Inverser les deux fait rejeter des fichiers parfaitement lisibles. --- CONTRIBUTING.md | 224 ++++++++++++++++++++++++++++++++++++++++++++++++ README.md | 3 + 2 files changed, 227 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..813bb29 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,224 @@ +# Contributing + +Thanks for considering a contribution. This document describes how this project actually +works — the conventions below are enforced by git hooks and CI, not by good will. + +## The one rule that shapes everything + +**Zero runtime dependencies.** The `require` section of `composer.json` contains exactly +one entry: `"php": ">=8.2"`. + +This is not minimalism for its own sake. The library exists because decoding a RAW needs +`libraw` or Imagick, which are unavailable on shared hosting without root — and that is the +target. Extracting an already-embedded JPEG needs nothing but byte reading. + +If a change seems to require a runtime dependency, it is out of scope. Symfony, PHPUnit and +GD stay in `require-dev`. + +Two corollaries: + +- **GD is never used to extract.** It builds JPEGs in tests and verifies output with + `imagecreatefromstring()` — never in `src/`. +- **Nothing under `src/` imports `Symfony\`** except `src/Bridge/Symfony/`, which is DI + wiring only. Verify with: + + ```bash + grep -rl 'Symfony\\' src/ --include='*.php' | grep -v '^src/Bridge/' # must be empty + ``` + +## Setup + +```bash +git clone https://github.com/ronan-develop/raw-preview-extractor.git +cd raw-preview-extractor +composer install + +vendor/bin/phpunit # full suite +vendor/bin/phpunit --testsuite unit # fast: forged bytes, no files +vendor/bin/phpunit --testsuite integration # public API end to end +``` + +The suite runs on a clean machine **with no RAW file whatsoever**. If a test needs one, the +test is wrong. + +Optional but recommended — the commit hooks: + +```bash +git config core.hooksPath .githooks +``` + +They reject commits whose message breaks convention, and any staged RAW or binary over 1 MB. + +## Test-driven, strictly + +**No line of `src/` is written before a failing test** — including boilerplate. + +1. Write the test. Run it. **See it fail** — an assumed red is not a red. +2. Write the minimum to pass. +3. Refactor green. + +Test and implementation go in the same commit. A commit never leaves the suite red. + +### Tests use forged bytes, not fixtures + +No camera file is committed to this repository. A RAW carries EXIF (serial number, +sometimes GPS), and a committed binary stays in git history forever. + +Tests build byte sequences in memory — a TIFF header is 8 bytes, an ISO-BMFF box is 8 bytes: + +```php +$header = "II" . pack('v', 42) . pack('V', 8); // little-endian TIFF, IFD at offset 8 +``` + +This tests *better* than real files: it exercises the looping IFD, the out-of-bounds offset, +the `size == 0` box — cases no real camera produces. + +### But forged bytes prove less than you think + +**Reproduce the real structure, not a simplified one.** Every bug found in this project so +far was invisible to a green test suite: + +| Bug | Why tests missed it | +|-------------------------------------|-------------------------------------------------------------------------| +| `findUuid()` searched only the root | Tests put the UUID box at the root. Real CR3s nest it under `moov`. | +| Facade failed on a real CR2 | Every facade test used mocks; none assembled the real parts. | +| CR2 returned 28 MB of sensor data | Canon marks lossless JPEG as `Compression = 6`; the test files did not. | +| G12 raised on a valid file | No test had a candidate that *lied* about its content. | + +A test written from a misunderstanding reproduces the misunderstanding. Before calling a +change done, exercise the public API on a real file (see below). + +## Coverage: ≥ 95 %, enforced + +CI fails below the threshold and lists the uncovered lines. Before adding a test to satisfy +it, **ask whether the line deserves to exist**: + +- **Unreachable** → delete it. Most uncovered lines in this project were dead guards: + `fopen()` checked after `is_file()`, `strlen()` after an already-bounded `fread()`, + `unpack()` guarded when length was guaranteed. A guard that cannot fire is not + defence-in-depth — it is noise that fakes robustness. +- **Reachable, guards a real scenario** (hostile file: looping IFD, absurd `count`, + out-of-bounds offset) → write the test. +- **Hard to reach because of the design** → fix the design. The threshold once revealed + that `IfdEntry` needed a `TiffReader` to read its own values — a cursor disguised as a + value object. + +Coverage went from 83 % to 99 % here mostly by *deleting* code. Never add an artificial test +to move the number. + +## Binary parsing rules + +The file is untrusted input. These are not suggestions: + +- **Never `unpack()`** without checking the byte length first — `fread` near EOF silently + returns less than asked. +- Every offset read from the file is validated against `filesize()` before use. +- **Bound by `filesize()`, never by an arbitrary constant.** A 64 KB cap rejected a valid + Canon 5D whose `MakerNote` is 75 KB. A value cannot exceed the file containing it — that + is the only bound that means anything. +- Endianness is carried explicitly: `v`/`V` (little) or `n`/`N` (big). **Never `S`/`L`** — + they follow the CPU and make parsing machine-dependent. +- Loop guards on IFD chains and box trees: a corrupt or hostile file can point a structure + at itself. +- Validate the `FFD8` magic before returning any JPEG. + +### Corrupt is not the same as missing + +| Exception | When | +| --- | --- | +| `CorruptedFileException` | structurally invalid: out-of-bounds offset, truncated IFD, value larger than the file | +| `PreviewNotFoundException` | valid file, no usable preview — including a tag that lies about its content | +| `UnsupportedFormatException` | not a supported RAW | + +A truncated file is *corrupt*. A tag claiming `Compression = 6` over raw sensor data is +*common in RAWs* — that candidate is skipped, not fatal. Getting this backwards makes the +library reject files it should read. + +All exceptions implement `RawPreviewExtractorException`, so callers degrade with one +`catch`. That contract is tested. + +## Adding a format + +The design intends this to require **no change to existing code**: + +1. Add a case to the `Format` enum. +2. Add a `PreviewParserInterface` implementation. +3. Wire it in `RawPreviewExtractor::createDefault()` and in the bundle's `services.php`. + +The facade resolves through an injected `Format → parser` map — no `switch`, no `match` on +extensions. **If adding a format forces you to touch `RawPreviewExtractor`, the design has +failed** — say so rather than working around it. + +There is no per-model code anywhere, and there should never be. Every camera quirk found so +far was fixed by making a rule *more general*, never by adding a special case. + +## Validating against real files + +```bash +mkdir -p tests/Fixtures/local # git-ignored +cp ~/photos/IMG_0042.CR2 tests/Fixtures/local/ + +php bin/extract-local.php # writes previews to tests/Fixtures/local/output/ +``` + +**Open the output.** A JPEG that decodes is not necessarily the right JPEG — this library +once returned a valid 160×120 thumbnail where a 1620×1080 preview was expected, with every +test green. + +Note that `getimagesizefromstring()` is not enough: it reads the header only and happily +accepts a lossless JPEG that no decoder can open. Use `imagecreatefromstring()`. + +To audit against the public catalogue (400+ models, CC0 files): + +```bash +php bin/audit-cameras.php Canon 25 +php bin/audit-cameras.php all 500 +``` + +Each file is downloaded, tested, then deleted. Failures are the useful output — each one +points at a structure the parser does not yet handle. + +## Commits + +```text + (): +``` + +| Emoji | Type | Emoji | Type | +| --- | --- | --- | --- | +| ✨ | feat | ✅ | test | +| 🔧 | fix | 🏗️ | build | +| 📖 | docs | 🏭 | ci | +| ♻️ | refactor | 🛠️ | chore | +| ⚡ | perf | 🔒 | security | + +Rules the `commit-msg` hook enforces: + +- emoji always present; +- **explicit scope**: a class or module name (`TiffReader`, `Cr3PreviewParser`, `composer`), + never `feat(file)` or `fix(api)`; +- atomic commits — one responsibility each. + +Explain **why** in the body, not what. The diff shows what. + +## Pull requests + +Branch from an issue: `/#-`, e.g. `fix/#27-candidate-without-magic`. +Include `Closes #` in the body. + +All six CI checks must pass — PHP 8.2/Symfony 6.4 at `--prefer-lowest` through PHP +8.4/Symfony 8, plus the coverage threshold. + +## Semver + +The public surface is `RawPreviewExtractorInterface`, `ExtractedPreview`, the `Format` enum +cases, the exception hierarchy, and the bundle's service alias. + +`ExtractedPreview` is `final readonly` with promoted public properties — **everything about +it is public surface**, with no slack. Adding an enum case is minor; renaming a property or +changing which exception fires in a given case is major. + +## License + +MIT. `exiftool` and `libraw` were used as behavioural references only — their output was +compared, never their source read or translated. diff --git a/README.md b/README.md index e48fc09..c897482 100644 --- a/README.md +++ b/README.md @@ -175,6 +175,9 @@ check in one command: see [Trying it on your own RAW files](#trying-it-on-your-o ## Contributing +See [CONTRIBUTING.md](CONTRIBUTING.md) for conventions, the TDD workflow, and the binary +parsing rules. In short: + ```bash composer install vendor/bin/phpunit # full suite From 4135e85b5e6e8a013119ea364123c5c247651a82 Mon Sep 17 00:00:00 2001 From: Ronan Lenouvel Date: Thu, 16 Jul 2026 22:52:26 +0200 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=94=A7=20fix(audit):=20isoler=20le=20?= =?UTF-8?q?dossier=20temporaire=20par=20processus?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Quatre audits lancés en parallèle partageaient /tmp/rpe-audit : le premier qui finissait supprimait le dossier des autres, qui écrivaient alors dans le vide. Résultat : 15 % de réussite annoncée là où le même modèle passait à 76 % seul. Un auditeur qui invente ses échecs est pire qu'inutile — il envoie enquêter sur des bugs qui n'existent pas. Le nettoyage vide aussi le dossier avant de le supprimer : rmdir échoue sur un dossier non vide, ce qu'un téléchargement interrompu peut laisser. --- bin/audit-cameras.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/bin/audit-cameras.php b/bin/audit-cameras.php index bf7ea12..90c900f 100644 --- a/bin/audit-cameras.php +++ b/bin/audit-cameras.php @@ -50,7 +50,11 @@ }; $extractor = RawPreviewExtractor::createDefault(); -$tmp = sys_get_temp_dir() . '/rpe-audit'; + +// Un dossier par processus : plusieurs audits peuvent tourner en parallèle +// (une marque chacun), et le premier qui finissait supprimait le dossier des +// autres — qui écrivaient alors dans le vide et comptaient des faux échecs. +$tmp = sys_get_temp_dir() . '/rpe-audit-' . getmypid(); @mkdir($tmp, 0o755, true); $results = []; @@ -94,6 +98,12 @@ report($results, $stats); +// rmdir échoue sur un dossier non vide : un téléchargement interrompu peut +// laisser un fichier derrière lui. +foreach (glob($tmp . '/*') ?: [] as $leftover) { + @unlink($leftover); +} + @rmdir($tmp); exit($stats['failed'] > 0 ? 1 : 0); From 213cc53f9beda3d0f64622e4714bcd9586d6a8ed Mon Sep 17 00:00:00 2001 From: Ronan Lenouvel Date: Thu, 16 Jul 2026 22:57:13 +0200 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=93=96=20docs(readme):=20lister=20les?= =?UTF-8?q?=2064=20mod=C3=A8les=20valid=C3=A9s=20sur=20fichiers=20r=C3=A9e?= =?UTF-8?q?ls?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit des quatre marques du périmètre : 72 modèles testés, 64 réussis. Sony 23/23, Nikon 20/22, Apple 5/6, Canon 16/21 — tous les boîtiers EOS passent. Les 8 échecs ont été vérifiés un par un, et tous sont corrects : ces fichiers ne contiennent aucun JPEG (zéro FFD8 dans le fichier entier). Un Nikon D1H de 2001 porte une vignette 160x120 RGB non compressée, les DNG de compacts générés par CHDK font de même, et les .CRW précèdent le CR2. PreviewNotFoundException y est la bonne réponse — le taux réel sur les fichiers qui ont une preview est 100 %. La liste complète est en annexe, en bas de page, pour ne pas noyer le tableau principal des 9 appareils validés à la main avec leurs dimensions. --- README.md | 84 ++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 71 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index c897482..09795d1 100644 --- a/README.md +++ b/README.md @@ -143,27 +143,31 @@ format parsers break. ### Wider audit -Beyond the table above, the library is audited against the whole -[raw.pixls.us](https://raw.pixls.us/) catalogue — 400+ camera models: +Beyond the table above, the library is audited against the +[raw.pixls.us](https://raw.pixls.us/) catalogue — 400+ camera models, CC0 files: ```bash php bin/audit-cameras.php Canon 25 # 25 Canon models, spread across the range php bin/audit-cameras.php all 500 # everything ``` -It downloads each file, extracts, verifies with `imagecreatefromstring()`, then deletes it. -Latest Canon run — **every EOS body passed**: +Each file is downloaded, extracted, verified with `imagecreatefromstring()`, then deleted. +Latest run — **72 models across the four brands in scope**: -```text -EOS-1D X Mark II EOS 5D Mark IV EOS 30D EOS 100D EOS 500D EOS 1000D -EOS R5 EOS R100 EOS RP EOS M6 EOS Kiss M EOS Rebel T5 -PowerShot G12 PowerShot G1 X Mark II PowerShot SX1 IS PowerShot V1 -``` +| Brand | Passed | Notes | +|-------|--------|----------------------------------| +| Sony | 23/23 | 100 % — A290 (2010) through FX30 | +| Nikon | 20/22 | 90.9 % | +| Apple | 5/6 | 83.3 % | +| Canon | 16/21 | 76.2 % — every EOS body passed | -The remaining failures are compacts whose files genuinely **contain no JPEG preview** — -CHDK-generated DNGs holding an uncompressed 128×96 thumbnail, or legacy `.CRW` files -(pre-CR2, out of scope). `PreviewNotFoundException` is the correct answer there, and the -audit counts it as an expected outcome rather than a defect. +**Every failure was verified by hand, and every one is correct**: those files contain no +JPEG preview at all (zero `FFD8` bytes in the entire file). A Nikon D1H from 2001 carries an +uncompressed 160×120 RGB thumbnail; CHDK-generated compact DNGs do the same; legacy `.CRW` +files predate CR2 and are out of scope. `PreviewNotFoundException` is the right answer, so +the real success rate on files that *have* a preview is 100 %. + +The full list is at the [bottom of this page](#appendix-models-verified-against-real-files). Every preview above is checked with `imagecreatefromstring()` — not just decoded headers, but actually opened. Extraction takes **1–9 ms** on files up to 62 MB: the file is never @@ -223,3 +227,57 @@ MIT — see [LICENSE](LICENSE). `exiftool` and `libraw` were used as behavioural references only. No code was copied from either project. + +--- + +## Appendix: models verified against real files + +Every model below had a real file downloaded, its preview extracted, and the result opened +with `imagecreatefromstring()`. Sources: [raw.pixls.us](https://raw.pixls.us/) (CC0). + +**Canon** — CR2 & CR3 + +```text +EOS 5D EOS 5D Mark II EOS 5D Mark II sRAW1 EOS 5D Mark IV +EOS-1D X Mark II EOS 30D EOS 100D EOS 500D +EOS 1000D EOS Rebel T5 EOS M6 EOS Kiss M +EOS R EOS RP EOS R5 EOS R100 +PowerShot G12 PowerShot G1 X Mark II PowerShot SX1 IS +PowerShot V1 +``` + +**Nikon** — NEF + +```text +D60 D90 D300 D610 D800 D850 D2Xs D3300 +D4 D5100 D5600 D6 D7500 E8400 Z 7 Z 30 +1 AW1 1 J3 1 S2 Coolpix A +``` + +**Sony** — ARW + +```text +DSLR-A290 DSLR-A380 DSLR-A550 DSLR-A850 SLT-A35 SLT-A58 +NEX-5N NEX-7 ILCE-5000 ILCE-6100 ILCE-6600 ILCE-7S +ILCE-7M4 ILCE-7RM3A ILCE-7CM2 ILCA-99M2 ILME-FX30 UMC-R10C +DSC-RX0 DSC-RX100M4 DSC-RX100M7 DSC-RX10M4 DSC-RX1RM2 +``` + +**Apple** — DNG (including ProRAW) + +```text +iPhone 6s Plus iPhone 7 Plus iPhone SE iPhone XS iPhone 12 Pro +``` + +### Known to have no preview + +These files contain **no JPEG at all** — verified byte by byte. `PreviewNotFoundException` +is the correct response, not a defect: + +| Model | What the file actually holds | +|-------------------------------------------------|-------------------------------------------| +| Nikon D1H (2001) | uncompressed 160×120 RGB thumbnail | +| Nikon COOLSCAN V ED | film scanner, no camera preview | +| Apple iPhone 8 | 64 sensor tiles, no JPEG | +| Canon PowerShot SX100 IS, SX510 HS, ELPH 130 IS | CHDK DNGs, uncompressed 128×96 thumbnail | +| Canon PowerShot G7 X, IXY 220F | `.CRW` — pre-CR2 format, out of scope |