[
https://issues.apache.org/jira/browse/TIKA-4824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18106042#comment-18106042
]
ASF GitHub Bot commented on TIKA-4824:
--------------------------------------
tballison commented on PR #3037:
URL: https://github.com/apache/tika/pull/3037#issuecomment-5347083098
This is is really nice.
Claude has a few nits:
```
1. MEDIUM (compat/docs) — CHANGES undersells the behavior change every
upgrader inherits. With sub-class-of image/tiff added, a named
NEF/ARW/PEF/DNG that used to detect as image/tiff (TiffParser, metadata
only) now detects as image/x-raw-* and emits thumbnail-N.jpg
attachments in /rmeta and /unpack; CR2 keeps its detection but gains
attachments. Downstream consumers counting attachments or keying on
container Content-Type see changes CHANGES doesn't mention. One added
sentence fixes it.
2. MEDIUM (DoS lever, cheap fix) — byte-at-a-time unbuffered I/O. Every
value read bottoms out in single-byte RandomAccessFile.read() with
per-entry re-seeks (RawTiffParser.java:258-287, 408-427). A ~25KB
crafted file can force ~2.7×10⁸ syscalls (32 IFDs × 1024 entries ×
1024-count LONG8 values) — bounded but tens of seconds of CPU per tiny
file under concurrent load. Fix: read each IFD's entry table (≤20KB,
already bounds-checked) into one byte[] and parse from memory.
3. MEDIUM (design, maintainer call) — config ignored in ParseContext.
parse() reads only the constructor-bound config (:142,:297), never
context.get(RawTiffParserConfig.class, defaultConfig) — deviating from
the PDFParser convention. Trade-off is real: the security reviewer
counted "clients can't flip it per-request" as a hardening positive;
the correctness reviewer flags it as an API-freeze inconsistency a
ParseContext user silently loses to. Decide which contract you want
before 4.0 freezes it.
4. LOW/MEDIUM (scope-creep compat) — new II\x2b\x00 magic on image/tiff
routes plain little-endian BigTIFF .tif files to TiffParser, which
can't read BigTIFF → new parse exception where there was silence.
Extends a pre-existing gap (BE BigTIFF magic was already there), but
it's an unflagged detection change beyond the raw-file scope and absent
from CHANGES.
5. LOW — dedup is offset-only and untested (two reviewers independently).
A FFD8 FFD8... run defeats it via shifted offsets; real bound is
MAX_IFDS=32, so worst case ≈ 32× fileLength of downstream JPEG work —
tolerable, but the commit's "bound extraction work" claim overstates
it. An aggregate byte budget would make it true; either way, deleting
the dedup set today fails no test — add the two-IFD fixture test.
6. LOW — raw files silently lose the whole-file OCR branch.
RawTiffParser.parse fully replaces AbstractImageParser.parse, dropping
its OCR fallback for these types. Arguably better (previews get OCR'd
as JPEGs through the embedded pipeline), but undocumented and untested.
7. LOW — oversized previews are skipped with no signal (:296-297) — no
warning metadata, unlike structural failures which are recorded. Also:
CHANGES documents extractPreviews but not maxPreviewLengthBytes; the
"tens of MB is already generous" comment sits on a 100MB constant.
8. Test gaps: no hostile/truncated classic-TIFF test (the caps, cycle
guard, and exception-recording path at :153-157 are all unexercised);
no big-endian BigTIFF preview-extraction test (only LE fixture + BE
empty-header test). Nits: bare type codes 3/4/13/16/18 vs the file's
own named-constant convention; the
```
> Extract embedded JPEG previews from TIFF-based raw images (NEF, ARW, PEF,
> DNG, CR2)
> -----------------------------------------------------------------------------------
>
> Key: TIKA-4824
> URL: https://issues.apache.org/jira/browse/TIKA-4824
> Project: Tika
> Issue Type: New Feature
> Reporter: Dominik Schmidt
> Priority: Major
>
> TIFF-based camera raw files embed camera-generated JPEG previews, often at
> full resolution. Tika currently gives no access to them: the x-raw-* types
> are glob-only (not sub-classes of image/tiff), no parser claims them, and
> nothing extracts the preview bytes. For consumers that cannot decode raw
> sensor data, the embedded JPEG is the only practical way to render a raw file
> (analogous to TIKA-4801 for audio cover art).
> The previews are stored in two ways:
> - Nikon NEF/NRW, Sony ARW/SRF/SR2, Pentax PEF/PTX:
> JPEGInterchangeFormat/-Length tags in the IFD chain or in SubIFDs
> - Adobe DNG, Canon CR2: a single JPEG-compressed strip (Compression 6/7)
> Proposal:
> - New RawTiffParser in tika-parser-image-module claiming
> image/x-raw-{nikon,sony,pentax,adobe} and image/x-canon-cr2: TIFF/EXIF/XMP
> metadata via TiffParser, walks the IFD chain plus SubIFDs and emits each
> preview through the EmbeddedDocumentExtractor (image/jpeg, THUMBNAIL);
> configurable via "raw-tiff-parser": {"extractPreviews": false}
> - Strip candidates need a safety rule: raw sensor data in CR2/DNG is lossless
> JPEG and also starts with an SOI marker. Strips are only accepted for
> displayable images (PhotometricInterpretation RGB/YCbCr, or 8 bits per sample
> when it is absent, as in CR2's preview IFD)
> - Make image/x-raw-{nikon,sony,pentax,adobe} sub-classes of image/tiff so
> name+data detection resolves them (they have no reliable magic; data-only
> detection stays image/tiff)
> - Synthetic ~2KB test fixtures mirroring the real IFD layouts, unit and
> detection tests
> Evaluated and excluded: Epson ERF (preview lives in the MakerNote), Olympus
> ORF and Panasonic RW2 (non-standard TIFF magic), Fuji RAF (not TIFF), Canon
> CR3 (ISO-BMFF). Verified against CC0 samples from raw.pixls.us (NEX-6,
> DSC-R1, K-7, EOS 7D, K-x and GR DNGs) plus real Nikon D80/D3000 files.
> PR to follow.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
