TIKA-4808 -- fix extension calculations for embedded files

Author: tballison

tika-core/src/main/java/org/apache/tika/extractor/EmbeddedDocumentUtil.java

@@ -148,41 +148,29 @@ public String getExtension(TikaInputStream is, Metadata metadata) {
         //use the buffered mimetypes as default
         MimeTypes localMimeTypes = getMimeTypes();
 
-        MimeType mimeType = null;
-        boolean detected = false;
-        if (mimeString != null) {
-            try {
-                mimeType = localMimeTypes.forName(mimeString);
-            } catch (MimeTypeException e) {
-                //swallow
-            }
-        }
-        if (mimeType == null) {
-            try {
-                MediaType mediaType = getDetector().detect(is, metadata, context);
-                mimeType = localMimeTypes.forName(mediaType.toString());
-                detected = true;
-                is.reset();
-            } catch (IOException | MimeTypeException e) {
-                //swallow
-            }
+        //a parseable declared type wins, even if we have no glob for it. Don't
+        //detect just because the registry lookup came back empty -- that would
+        //overwrite a type the calling parser set deliberately.
+        if (mimeString != null && MediaType.parse(mimeString) != null) {
+            return extensionOf(getRegisteredMimeType(localMimeTypes, mimeString));
         }
-        if (mimeType != null) {
-            if (detected) {
-                //set or correct the mime type
-                metadata.set(Metadata.CONTENT_TYPE, mimeType.toString());
-            }
-            return mimeType.getExtension();
+        try {
+            MediaType mediaType = getDetector().detect(is, metadata, context);
+            is.reset();
+            //set or correct the mime type. Record what was detected, not the
+            //registry match, which may have fallen back to the base type.
+            metadata.set(Metadata.CONTENT_TYPE, mediaType.toString());
+            return extensionOf(getRegisteredMimeType(localMimeTypes, mediaType.toString()));
+        } catch (IOException e) {
+            //swallow
         }
         return ".bin";
     }
 
-    /**
-     * Looks up the file extension for a given media type string.
-     *
-     * @param mediaType the media type string (e.g., "image/png")
-     * @return the extension including the dot (e.g., ".png"), or empty string if unknown
-     */
+    private static String extensionOf(MimeType mimeType) {
+        return mimeType == null ? "" : mimeType.getExtension();
+    }
+
     /**
      * Normalizes internal OCR routing media types (e.g., {@code image/ocr-png})
      * back to standard media types (e.g., {@code image/png}).
@@ -198,16 +186,36 @@ public static String normalizeMediaType(String mediaType) {
         return mediaType;
     }
 
+    /**
+     * Looks up the file extension for a given media type string.
+     *
+     * @param mediaType the media type string (e.g., "image/png"), parameters allowed
+     * @return the extension including the dot (e.g., ".png"), or empty string if unknown
+     */
     public static String getExtensionForMediaType(String mediaType) {
         if (mediaType == null) {
             return "";
         }
-        mediaType = normalizeMediaType(mediaType);
+        MimeType mimeType =
+                getRegisteredMimeType(MimeTypes.getDefaultMimeTypes(),
+                        normalizeMediaType(mediaType));
+        return mimeType == null ? "" : mimeType.getExtension();
+    }
+
+    /**
+     * Not {@link MimeTypes#forName(String)}: that registers a new, glob-less type for
+     * any name it doesn't recognize, so <code>text/plain; charset=UTF-8</code> would
+     * lose its extension and add a registry entry per charset seen. This prefers an
+     * exact parameterized match (<code>application/dita+xml;format=map</code> is real)
+     * and otherwise falls back to the base type.
+     *
+     * @return the registered type, or null if unknown or invalid
+     */
+    private static MimeType getRegisteredMimeType(MimeTypes mimeTypes, String name) {
         try {
-            MimeType mimeType = MimeTypes.getDefaultMimeTypes().forName(mediaType);
-            return mimeType.getExtension();
+            return mimeTypes.getRegisteredMimeType(name);
         } catch (MimeTypeException e) {
-            return "";
+            return null;
         }
     }
 

tika-core/src/test/java/org/apache/tika/extractor/EmbeddedDocumentUtilExtensionTest.java

@@ -0,0 +1,150 @@
+/*
+ * 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.extractor;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.junit.jupiter.api.Test;
+
+import org.apache.tika.io.TikaInputStream;
+import org.apache.tika.metadata.Metadata;
+import org.apache.tika.mime.MediaType;
+import org.apache.tika.mime.MimeTypes;
+import org.apache.tika.parser.ParseContext;
+
+/**
+ * TIKA-4808 -- a media type carrying parameters must still resolve to the
+ * extension of its base type.
+ */
+public class EmbeddedDocumentUtilExtensionTest {
+
+    @Test
+    public void testParametersDoNotSuppressExtension() {
+        //the corpus regression: Pkcs7Parser refines the coarse family label to the
+        //exact smime-type, which turned /embedded-2.p7s into /embedded-2
+        assertEquals(".p7m",
+                EmbeddedDocumentUtil.getExtensionForMediaType(
+                        "application/pkcs7-mime; smime-type=signed-data"));
+        assertEquals(".txt",
+                EmbeddedDocumentUtil.getExtensionForMediaType("text/plain; charset=UTF-8"));
+        assertEquals(".js",
+                EmbeddedDocumentUtil.getExtensionForMediaType(
+                        "text/javascript; charset=UTF-8"));
+        assertEquals(".css",
+                EmbeddedDocumentUtil.getExtensionForMediaType(
+                        "text/css; charset=ISO-2022-JP"));
+        assertEquals(".html",
+                EmbeddedDocumentUtil.getExtensionForMediaType(
+                        "text/html; charset=windows-1252"));
+    }
+
+    @Test
+    public void testUnparameterizedStillWorks() {
+        assertEquals(".p7s",
+                EmbeddedDocumentUtil.getExtensionForMediaType("application/pkcs7-signature"));
+        assertEquals(".png", EmbeddedDocumentUtil.getExtensionForMediaType("image/png"));
+        assertEquals(".txt", EmbeddedDocumentUtil.getExtensionForMediaType("text/plain"));
+    }
+
+    /**
+     * A registered type that genuinely has parameters must win over its base type.
+     */
+    @Test
+    public void testRegisteredParameterizedTypeWinsOverBaseType() {
+        assertEquals(".ditamap",
+                EmbeddedDocumentUtil.getExtensionForMediaType(
+                        "application/dita+xml;format=map"));
+        assertEquals(".dita",
+                EmbeddedDocumentUtil.getExtensionForMediaType(
+                        "application/dita+xml;format=topic"));
+    }
+
+    @Test
+    public void testUnknownAndNull() {
+        assertEquals("", EmbeddedDocumentUtil.getExtensionForMediaType(null));
+        assertEquals("",
+                EmbeddedDocumentUtil.getExtensionForMediaType("application/tika-bogus-xyz"));
+    }
+
+    @Test
+    public void testOcrRoutingTypeIsNormalized() {
+        assertEquals(".png", EmbeddedDocumentUtil.getExtensionForMediaType("image/ocr-png"));
+    }
+
+    /**
+     * The lookup must not register anything: forName() would add one glob-less entry
+     * per distinct parameter value seen, which on a large crawl grows without bound.
+     */
+    @Test
+    public void testLookupDoesNotPolluteRegistry() {
+        MimeTypes mimeTypes = MimeTypes.getDefaultMimeTypes();
+        int before = mimeTypes.getMediaTypeRegistry().getTypes().size();
+        for (int i = 0; i < 50; i++) {
+            EmbeddedDocumentUtil.getExtensionForMediaType("text/plain; charset=made-up-" + i);
+        }
+        assertEquals(before, mimeTypes.getMediaTypeRegistry().getTypes().size());
+        assertEquals(".txt",
+                EmbeddedDocumentUtil.getExtensionForMediaType("text/plain; charset=made-up-0"));
+    }
+
+    @Test
+    public void testGeneratedResourceNameKeepsExtension() {
+        assertEquals("embedded-2.p7m", EmbeddedDocumentUtil.generateResourceName(
+                EmbeddedDocumentUtil.EmbeddedResourcePrefix.EMBEDDED, 2,
+                "application/pkcs7-mime; smime-type=signed-data"));
+        assertEquals("image-0.png", EmbeddedDocumentUtil.generateResourceName(
+                EmbeddedDocumentUtil.EmbeddedResourcePrefix.IMAGE, 0, "image/png"));
+    }
+
+    /**
+     * A declared type that simply has no glob must not trigger detection -- that would
+     * overwrite a CONTENT_TYPE the calling parser set deliberately.
+     */
+    @Test
+    public void testDeclaredUnregisteredTypeIsNotOverwritten() throws Exception {
+        Metadata metadata = new Metadata();
+        metadata.set(Metadata.CONTENT_TYPE, "application/tika-bogus-xyz");
+        EmbeddedDocumentUtil util = new EmbeddedDocumentUtil(new ParseContext());
+        try (TikaInputStream tis = TikaInputStream.get("%PDF-1.4\n".getBytes(UTF_8))) {
+            assertEquals("", util.getExtension(tis, metadata));
+            assertEquals("application/tika-bogus-xyz", metadata.get(Metadata.CONTENT_TYPE));
+        }
+    }
+
+    @Test
+    public void testDeclaredParameterizedTypeResolvesAndIsPreserved() throws Exception {
+        Metadata metadata = new Metadata();
+        metadata.set(Metadata.CONTENT_TYPE, "text/plain; charset=UTF-8");
+        EmbeddedDocumentUtil util = new EmbeddedDocumentUtil(new ParseContext());
+        try (TikaInputStream tis = TikaInputStream.get("hello".getBytes(UTF_8))) {
+            assertEquals(".txt", util.getExtension(tis, metadata));
+            assertEquals("text/plain; charset=UTF-8", metadata.get(Metadata.CONTENT_TYPE));
+        }
+    }
+
+    /**
+     * Guard the assumption the fix rests on: normalize() deliberately preserves
+     * parameters, which is why forName() misses the registry for parameterized names.
+     */
+    @Test
+    public void testNormalizePreservesParameters() {
+        MediaType withParams = MediaType.parse("text/plain; charset=UTF-8");
+        assertEquals(withParams,
+                MimeTypes.getDefaultMimeTypes().getMediaTypeRegistry().normalize(withParams));
+    }
+}

tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-mail-module/src/test/java/org/apache/tika/parser/mail/RFC822ParserTest.java

@@ -404,7 +404,7 @@ public void testExtractAttachments() throws Exception {
         assertEquals(null, metadataList.get(1).get(Metadata.CONTENT_DISPOSITION));
         assertEquals("attachment; filename=\"testPNG.png\"",
                 metadataList.get(2).get(Metadata.CONTENT_DISPOSITION));
-        assertEquals("/Test Attachment Email.eml/embedded-1",
+        assertEquals("/Test Attachment Email.eml/embedded-1.txt",
                 metadataList.get(1).get(TikaCoreProperties.FINAL_EMBEDDED_RESOURCE_PATH));
         assertEquals("/Test Attachment Email.eml/testPNG.png",
                 metadataList.get(2).get(TikaCoreProperties.FINAL_EMBEDDED_RESOURCE_PATH));