You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tika.apache.org by GitBox <gi...@apache.org> on 2020/11/20 01:29:55 UTC

[GitHub] [tika] PeterAlfredLee commented on a change in pull request #382: Simplify some code in OPCPackageDetector#detect

PeterAlfredLee commented on a change in pull request #382:
URL: https://github.com/apache/tika/pull/382#discussion_r527336143



##########
File path: tika-parser-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/detect/microsoft/ooxml/OPCPackageDetector.java
##########
@@ -159,32 +159,32 @@
 
     @Override
     public MediaType detect(ZipFile zipFile, TikaInputStream stream) throws IOException {
-        //as of 4.x, POI throws an exception for non-POI OPC file types
-        //unless we change POI, we can't rely on POI for non-POI files
+        // as of 4.x, POI throws an exception for non-POI OPC file types
+        // unless we change POI, we can't rely on POI for non-POI files
         ZipEntrySource zipEntrySource = new ZipFileZipEntrySource(zipFile);
 
         // Use POI to open and investigate it for us
-        //Unfortunately, POI can throw a RuntimeException...so we
-        //have to catch that.
-        OPCPackage pkg = null;
-        MediaType type = null;
+        // Unfortunately, POI can throw a RuntimeException...so we have to catch that.
         try {
-            pkg = OPCPackage.open(zipEntrySource);
-            type = detectOfficeOpenXML(pkg);
+            OPCPackage pkg = OPCPackage.open(zipEntrySource);
+            MediaType type = detectOfficeOpenXML(pkg);
+
+            // only set the open container if we made it here
+            stream.setOpenContainer(pkg);
+            return type;
 
-        } catch (SecurityException e) {
-            closeQuietly(zipEntrySource);
-            closeQuietly(zipFile);
-            //TIKA-2571
-            throw e;
         } catch (InvalidFormatException | RuntimeException e) {
             closeQuietly(zipEntrySource);
             closeQuietly(zipFile);
-            return null;
+
+            if (e instanceof SecurityException) {

Review comment:
       Ah, I was just thinking to simplify the two catch clause as they look similar. Haven't think about the performance here. :)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org