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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
