[
https://issues.apache.org/jira/browse/TIKA-4937?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18124242#comment-18124242
]
ASF GitHub Bot commented on TIKA-4937:
--------------------------------------
dschmidt commented on PR #3303:
URL: https://github.com/apache/tika/pull/3303#issuecomment-6030005988
My 🤖 says:
The OS/2 part is good. The TGA part has one regression: with TGA down at
priority 50, the cursor magic (`00 00 02 00`, byte 5 zero) claims every type 2
TGA without an image id that the new TGA rule rejects. A plain 24 bpp TGA only
stays TGA through the clause-length tie-break.
Detection with the `tika-mimetypes.xml` of main and of cfe814a6fa, synthetic
headers followed by pixel data:
| Input (hex) | main | this PR |
|---|---|---|
| type 2, 15 bpp: `00 00 02 00 00 00 00 00 00 00 00 00 04 00 04 00 0F 00` |
`image/x-tga` | `image/x-win-bitmap`, also when named `x.tga` |
| the same with a 3 byte image id (`03 00 02 ...`) | `image/x-tga` |
`application/octet-stream` |
| type 2, 24 bpp, colour map entry size set but no map: `00 00 02 00 00 00
00 18 00 00 00 00 04 00 04 00 18 00` | `image/x-tga` | `image/x-win-bitmap`,
also when named `x.tga` |
| `41 01 01 99 99 99 99 20` plus anything | `application/octet-stream` |
`image/x-tga` |
Suggested changes in `tika-mimetypes.xml`. The first two were tried against
the samples above plus `testCUR.cur` and `testICO.ico`:
1. `image/x-tga`: add `<match value="\017" type="string" offset="16"/>` to
the type 2 and type 10 rules. Fixes rows 1 and 2.
2. `image/x-win-bitmap`: require at least one image by replacing the nested
`offset="5"` match with `<match value="[\\x01-\\xff]\\x00" type="regex"
offset="4"/>`. A TGA without a colour map has zeros there, so the overlap is
gone instead of resting on the tie-break. Row 3 is then
`application/octet-stream` by content and `image/x-tga` by name. Cursors with 1
to 255 images are unaffected.
3. Not tried: the colour-mapped rules (types 1 and 9) pin only three bytes
(row 4). Checking the pixel depth at 16 as well, as the XML comment says, would
tighten them.
`MimeDetectionTest`:
- The cursor and Lotus assertions in `testTgaDetection` pass on main too
(bytes 1 to 4 are `00 02 00 01` and `00 02 00 04`, which neither the old nor
the new rule matches), so nothing pins the priority change. Better: a 15 bpp
type 2 header is `image/x-tga`, and a cursor header with 256 images (`00 00 02
00 00 01`) is not.
- `header[5] = (byte) 256` is 0.
- Three comments describe the old rules ("used to match", "the old rules
accepted", "the old rule matched"), which the development skill asks to avoid.
Smaller points:
- `ICOParser` throws again on `BA` input when it is called directly, or for
an array with a broken tag that is named `.ico`. Fine if intended, but CHANGES
does not say so.
- `image/x-os2-graphics` is not in `InferenceLoader.NON_RASTER`, where the
icon type these files used to be detected as is.
- CHANGES: the RLE types 9, 10 and 11 are newly detected, which is worth a
word, and the lost content detection could carry a "Compat:" note.
> Extract image dimensions and entry list from ICO/CUR files
> ----------------------------------------------------------
>
> Key: TIKA-4937
> URL: https://issues.apache.org/jira/browse/TIKA-4937
> Project: Tika
> Issue Type: New Feature
> Reporter: Dominik Schmidt
> Priority: Major
>
> h3. Background
> Tika detects {{image/vnd.microsoft.icon}} (alias {{image/x-icon}}) and
> {{ImageParser}} claims the type, but ImageIO has no ICO reader on the
> classpath, so a {{.ico}} yields nothing beyond the content type: no
> dimensions, no bit depth, no image count.
> Since TIKA-4936 every Windows EXE/DLL emits its icon groups as embedded
> {{.ico}} documents (the first one typed {{THUMBNAIL}}), so an index now sees
> plenty of .ico entries without width/height.
> h3. Proposal
> Read the ICONDIR/ICONDIRENTRY structure of {{.ico}} and {{.cur}} files
> directly (no decoding, no new dependency) and set:
> * {{tiff:ImageWidth}} / {{tiff:ImageLength}} and {{tiff:BitsPerSample}} of
> the largest image. Entries of 256 px are stored as 0 in the directory, and
> the directory's width/height/bpp fields are unreliable in general, so take
> the values from each image's header instead: the PNG IHDR for PNG-encoded
> entries, the BITMAPINFOHEADER for BMP-encoded ones (height there is doubled
> to include the AND mask).
> * the number of images
> * one multi-valued property listing each entry as {{WxH@bpp}} plus its
> encoding (bmp/png), in directory order
> * for cursors ({{.cur}}, type 2 in the header): the hotspot of the largest
> image
> No text content. Malformed files (directory beyond EOF, entry offsets outside
> the file, count 0, unknown encoding) must not throw: emit what can be read
> and record a warning.
> Implementation: a dedicated small parser in {{tika-parser-image-module}} for
> {{image/vnd.microsoft.icon}} and {{image/x-win-bitmap}}, replacing the
> {{image/x-icon}} entry in {{ImageParser}}'s supported types (which currently
> finds no reader).
> h3. Out of scope
> * Decoding or rendering the images (the thumbnail presets keep passing the
> .ico through as stored)
> * Animated cursors ({{.ani}}), {{RT_GROUP_CURSOR}} extraction from PE files
> h3. Acceptance criteria
> * {{tika -m}} on a .ico with 16/32 px BMP entries and a 256 px PNG entry
> reports 256x256, 32 bpp, 3 images and the per-entry list.
> * The embedded icons of TIKA-4936's fixtures show width/height in {{/rmeta}}
> output.
> * A .cur reports type cursor and the hotspot.
> * Truncated or corrupt .ico files yield a warning, not an exception.
> * Fixtures: {{testWindows-icons-app.ico}} / {{-doc.ico}} from TIKA-4936 plus
> one generated .cur.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
