[
https://issues.apache.org/jira/browse/TIKA-4937?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18120334#comment-18120334
]
ASF GitHub Bot commented on TIKA-4937:
--------------------------------------
Copilot commented on code in PR #3266:
URL: https://github.com/apache/tika/pull/3266#discussion_r4126263272
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/main/java/org/apache/tika/parser/image/ICOParser.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tika.parser.image;
+
+import java.io.IOException;
+import java.util.Set;
+
+import org.apache.commons.io.IOUtils;
+import org.xml.sax.ContentHandler;
+import org.xml.sax.SAXException;
+
+import org.apache.tika.annotation.TikaComponent;
+import org.apache.tika.exception.TikaException;
+import org.apache.tika.extractor.EmbeddedDocumentUtil;
+import org.apache.tika.io.BoundedInputStream;
+import org.apache.tika.io.EndianUtils;
+import org.apache.tika.io.TikaInputStream;
+import org.apache.tika.metadata.HttpHeaders;
+import org.apache.tika.metadata.Icon;
+import org.apache.tika.metadata.Metadata;
+import org.apache.tika.metadata.TIFF;
+import org.apache.tika.mime.MediaType;
+import org.apache.tika.parser.ParseContext;
+import org.apache.tika.parser.Parser;
+import org.apache.tika.sax.XHTMLContentHandler;
+
+/**
+ * Parser for Windows icon (ICO) and cursor (CUR) files. Reads the ICONDIR
+ * and the header of every image to report the dimensions and colour depth of
+ * the largest image, the list of all images and, for cursors, the hotspot.
+ * The images themselves are not decoded.
+ * <p>
+ * The directory's own width, height and colour fields are unreliable (256 px
+ * is stored as 0, many tools leave the bit count empty), so the values come
+ * from each image's PNG IHDR or BITMAPINFOHEADER and the directory is only
+ * the fallback.
+ */
+@TikaComponent
+public class ICOParser implements Parser {
+
+ private static final long serialVersionUID = 4212837190215395123L;
+
+ static final MediaType ICO_TYPE = MediaType.image("vnd.microsoft.icon");
+ static final MediaType CUR_TYPE = MediaType.image("x-win-bitmap");
+
+ private static final Set<MediaType> SUPPORTED_TYPES = Set.of(ICO_TYPE,
CUR_TYPE);
+
+ private static final int TYPE_ICON = 1;
+ private static final int TYPE_CURSOR = 2;
+ private static final int HEADER_SIZE = 6;
+ private static final int ENTRY_SIZE = 16;
+ private static final int BITMAP_INFO_HEADER_SIZE = 40;
+ private static final byte[] PNG_SIGNATURE =
+ {(byte) 0x89, 'P', 'N', 'G', '\r', '\n', 0x1a, '\n'};
+ private static final int PNG_IHDR_SIZE = 8 + 8 + 13;
+ // Icons are small; anything bigger is read only this far
+ private static final int MAX_FILE_SIZE = 16 * 1024 * 1024;
+
+ @Override
+ public Set<MediaType> getSupportedTypes(ParseContext context) {
+ return SUPPORTED_TYPES;
+ }
+
+ @Override
+ public void parse(TikaInputStream tis, ContentHandler handler, Metadata
metadata,
+ ParseContext context) throws IOException, SAXException,
TikaException {
+ byte[] file = IOUtils.toByteArray(new
BoundedInputStream(MAX_FILE_SIZE, tis));
+ if (file.length < HEADER_SIZE || EndianUtils.getUShortLE(file, 0) !=
0) {
+ throw new TikaException("Not an ICO or CUR file");
+ }
+ int type = EndianUtils.getUShortLE(file, 2);
+ if (type != TYPE_ICON && type != TYPE_CURSOR) {
+ throw new TikaException("Not an ICO or CUR file: type " + type);
+ }
+ metadata.set(HttpHeaders.CONTENT_TYPE,
+ (type == TYPE_CURSOR ? CUR_TYPE : ICO_TYPE).toString());
+
+ int count = EndianUtils.getUShortLE(file, 4);
+ metadata.set(Icon.IMAGE_COUNT, count);
+ Image largest = null;
+ int unreadable = 0;
+ for (int i = 0; i < count; i++) {
+ int entry = HEADER_SIZE + i * ENTRY_SIZE;
+ if (entry + ENTRY_SIZE > file.length) {
+ unreadable += count - i;
+ break;
+ }
+ Image image = readImage(file, entry);
+ if (image == null) {
+ unreadable++;
+ continue;
+ }
+ metadata.add(Icon.IMAGES, image.describe());
+ if (largest == null || image.outranks(largest)) {
+ largest = image;
+ }
+ }
+ if (largest != null) {
+ metadata.set(TIFF.IMAGE_WIDTH, largest.width);
+ metadata.set(TIFF.IMAGE_LENGTH, largest.height);
+ metadata.set(TIFF.BITS_PER_SAMPLE,
Integer.toString(largest.bitsPerPixel));
+ if (type == TYPE_CURSOR) {
+ metadata.set(Icon.HOTSPOT_X, largest.hotspotX);
+ metadata.set(Icon.HOTSPOT_Y, largest.hotspotY);
+ }
+ }
Review Comment:
`TIFF.BITS_PER_SAMPLE` is being set to *bits-per-pixel* (e.g., 32 for RGBA),
not bits-per-sample/channel (typically 8 for 8-bit RGBA). This makes the
metadata semantically inaccurate for consumers that interpret TIFF fields
literally. Consider either (a) setting `TIFF.BITS_PER_SAMPLE` to the
per-channel bit depth (e.g., PNG IHDR bit depth; for BMP derive a per-channel
depth when possible), or (b) moving bits-per-pixel to an icon-specific key
(e.g., a new `icon:bits-per-pixel` or similar) while keeping TIFF fields
standards-aligned.
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/main/java/org/apache/tika/parser/image/ICOParser.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tika.parser.image;
+
+import java.io.IOException;
+import java.util.Set;
+
+import org.apache.commons.io.IOUtils;
+import org.xml.sax.ContentHandler;
+import org.xml.sax.SAXException;
+
+import org.apache.tika.annotation.TikaComponent;
+import org.apache.tika.exception.TikaException;
+import org.apache.tika.extractor.EmbeddedDocumentUtil;
+import org.apache.tika.io.BoundedInputStream;
+import org.apache.tika.io.EndianUtils;
+import org.apache.tika.io.TikaInputStream;
+import org.apache.tika.metadata.HttpHeaders;
+import org.apache.tika.metadata.Icon;
+import org.apache.tika.metadata.Metadata;
+import org.apache.tika.metadata.TIFF;
+import org.apache.tika.mime.MediaType;
+import org.apache.tika.parser.ParseContext;
+import org.apache.tika.parser.Parser;
+import org.apache.tika.sax.XHTMLContentHandler;
+
+/**
+ * Parser for Windows icon (ICO) and cursor (CUR) files. Reads the ICONDIR
+ * and the header of every image to report the dimensions and colour depth of
+ * the largest image, the list of all images and, for cursors, the hotspot.
+ * The images themselves are not decoded.
+ * <p>
+ * The directory's own width, height and colour fields are unreliable (256 px
+ * is stored as 0, many tools leave the bit count empty), so the values come
+ * from each image's PNG IHDR or BITMAPINFOHEADER and the directory is only
+ * the fallback.
+ */
+@TikaComponent
+public class ICOParser implements Parser {
+
+ private static final long serialVersionUID = 4212837190215395123L;
+
+ static final MediaType ICO_TYPE = MediaType.image("vnd.microsoft.icon");
+ static final MediaType CUR_TYPE = MediaType.image("x-win-bitmap");
+
+ private static final Set<MediaType> SUPPORTED_TYPES = Set.of(ICO_TYPE,
CUR_TYPE);
+
+ private static final int TYPE_ICON = 1;
+ private static final int TYPE_CURSOR = 2;
+ private static final int HEADER_SIZE = 6;
+ private static final int ENTRY_SIZE = 16;
+ private static final int BITMAP_INFO_HEADER_SIZE = 40;
+ private static final byte[] PNG_SIGNATURE =
+ {(byte) 0x89, 'P', 'N', 'G', '\r', '\n', 0x1a, '\n'};
+ private static final int PNG_IHDR_SIZE = 8 + 8 + 13;
+ // Icons are small; anything bigger is read only this far
+ private static final int MAX_FILE_SIZE = 16 * 1024 * 1024;
+
+ @Override
+ public Set<MediaType> getSupportedTypes(ParseContext context) {
+ return SUPPORTED_TYPES;
+ }
+
+ @Override
+ public void parse(TikaInputStream tis, ContentHandler handler, Metadata
metadata,
+ ParseContext context) throws IOException, SAXException,
TikaException {
+ byte[] file = IOUtils.toByteArray(new
BoundedInputStream(MAX_FILE_SIZE, tis));
+ if (file.length < HEADER_SIZE || EndianUtils.getUShortLE(file, 0) !=
0) {
+ throw new TikaException("Not an ICO or CUR file");
+ }
+ int type = EndianUtils.getUShortLE(file, 2);
+ if (type != TYPE_ICON && type != TYPE_CURSOR) {
+ throw new TikaException("Not an ICO or CUR file: type " + type);
+ }
+ metadata.set(HttpHeaders.CONTENT_TYPE,
+ (type == TYPE_CURSOR ? CUR_TYPE : ICO_TYPE).toString());
+
+ int count = EndianUtils.getUShortLE(file, 4);
+ metadata.set(Icon.IMAGE_COUNT, count);
+ Image largest = null;
+ int unreadable = 0;
+ for (int i = 0; i < count; i++) {
+ int entry = HEADER_SIZE + i * ENTRY_SIZE;
+ if (entry + ENTRY_SIZE > file.length) {
+ unreadable += count - i;
+ break;
+ }
+ Image image = readImage(file, entry);
+ if (image == null) {
+ unreadable++;
+ continue;
+ }
+ metadata.add(Icon.IMAGES, image.describe());
+ if (largest == null || image.outranks(largest)) {
+ largest = image;
+ }
+ }
+ if (largest != null) {
+ metadata.set(TIFF.IMAGE_WIDTH, largest.width);
+ metadata.set(TIFF.IMAGE_LENGTH, largest.height);
+ metadata.set(TIFF.BITS_PER_SAMPLE,
Integer.toString(largest.bitsPerPixel));
+ if (type == TYPE_CURSOR) {
+ metadata.set(Icon.HOTSPOT_X, largest.hotspotX);
+ metadata.set(Icon.HOTSPOT_Y, largest.hotspotY);
+ }
+ }
+ if (unreadable > 0) {
+ EmbeddedDocumentUtil.recordException(new TikaException(
+ unreadable + " of " + count + " images lie outside the
file or have no" +
+ " readable header"), metadata, context);
+ }
+
+ XHTMLContentHandler xhtml = new XHTMLContentHandler(handler, metadata,
context);
+ xhtml.startDocument();
+ xhtml.endDocument();
+ }
+
+ /**
+ * Reads one ICONDIRENTRY and the header of the image it points to.
+ *
+ * @return the image, or null if its data lies outside the file
+ */
+ private static Image readImage(byte[] file, int entry) {
+ Image image = new Image();
+ // 0 in the directory means 256
+ image.width = file[entry] == 0 ? 256 : file[entry] & 0xff;
+ image.height = file[entry + 1] == 0 ? 256 : file[entry + 1] & 0xff;
+ // planes and bit count for icons, hotspot for cursors
+ image.hotspotX = EndianUtils.getUShortLE(file, entry + 4);
+ image.hotspotY = EndianUtils.getUShortLE(file, entry + 6);
+ image.bitsPerPixel = image.hotspotY;
Review Comment:
`readImage()` always assigns `bitsPerPixel` from the directory field at
`entry + 6`. That field is the icon bit count for ICO entries, but it is the
cursor hotspot Y for CUR entries, so CURs with unreadable/unknown image headers
will report a bogus bpp (and `Icon.IMAGES` strings will show `@<hotspotY>bpp`).
Make `readImage` type-aware (e.g., pass a boolean/enum for ICO vs CUR): for
ICO, keep the directory bitcount fallback; for CUR, populate hotspot fields
from the directory but leave `bitsPerPixel` unset/unknown unless derived from
PNG/BMP headers (and consider updating `describe()` to avoid printing
misleading bpp when unknown).
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/main/java/org/apache/tika/parser/image/ICOParser.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tika.parser.image;
+
+import java.io.IOException;
+import java.util.Set;
+
+import org.apache.commons.io.IOUtils;
+import org.xml.sax.ContentHandler;
+import org.xml.sax.SAXException;
+
+import org.apache.tika.annotation.TikaComponent;
+import org.apache.tika.exception.TikaException;
+import org.apache.tika.extractor.EmbeddedDocumentUtil;
+import org.apache.tika.io.BoundedInputStream;
+import org.apache.tika.io.EndianUtils;
+import org.apache.tika.io.TikaInputStream;
+import org.apache.tika.metadata.HttpHeaders;
+import org.apache.tika.metadata.Icon;
+import org.apache.tika.metadata.Metadata;
+import org.apache.tika.metadata.TIFF;
+import org.apache.tika.mime.MediaType;
+import org.apache.tika.parser.ParseContext;
+import org.apache.tika.parser.Parser;
+import org.apache.tika.sax.XHTMLContentHandler;
+
+/**
+ * Parser for Windows icon (ICO) and cursor (CUR) files. Reads the ICONDIR
+ * and the header of every image to report the dimensions and colour depth of
+ * the largest image, the list of all images and, for cursors, the hotspot.
+ * The images themselves are not decoded.
+ * <p>
+ * The directory's own width, height and colour fields are unreliable (256 px
+ * is stored as 0, many tools leave the bit count empty), so the values come
+ * from each image's PNG IHDR or BITMAPINFOHEADER and the directory is only
+ * the fallback.
+ */
+@TikaComponent
+public class ICOParser implements Parser {
+
+ private static final long serialVersionUID = 4212837190215395123L;
+
+ static final MediaType ICO_TYPE = MediaType.image("vnd.microsoft.icon");
+ static final MediaType CUR_TYPE = MediaType.image("x-win-bitmap");
+
+ private static final Set<MediaType> SUPPORTED_TYPES = Set.of(ICO_TYPE,
CUR_TYPE);
Review Comment:
This parser does not support the common alias `image/x-icon`, but
`ImageParser` was changed to no longer claim `image/x-icon`. If any detection
path still yields `image/x-icon` (from client-provided `Content-Type` headers,
older mime rules, or upstream systems), those inputs may no longer be parsed by
either parser. To avoid a regression, either add `MediaType.image("x-icon")` to
`SUPPORTED_TYPES` (treating it as an alias of `vnd.microsoft.icon`) or ensure
Tika’s mime mapping normalizes `image/x-icon` to `image/vnd.microsoft.icon`
before parser dispatch.
> 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)
